NetBSD Problem Report #57918

From www@netbsd.org  Fri Feb  9 21:57:00 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 087571A9238
	for <gnats-bugs@gnats.NetBSD.org>; Fri,  9 Feb 2024 21:57:00 +0000 (UTC)
Message-Id: <20240209215659.054271A9239@mollari.NetBSD.org>
Date: Fri,  9 Feb 2024 21:56:58 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: kdump ioctl decoder doesn't understand sign extension
X-Send-Pr-Version: www-1.0

>Number:         57918
>Category:       bin
>Synopsis:       kdump ioctl decoder doesn't understand sign extension
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Feb 09 22:00:00 +0000 2024
>Closed-Date:    Tue Mar 12 10:27:25 +0000 2024
>Last-Modified:  Tue Mar 12 10:27:25 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current
>Organization:
The NetBSkdump Foundation
>Environment:
LP64
>Description:
When kdump(8) tries to decode ioctls nicely with _IOW(...), it incorrectly interprets a `negative' input (i.e., a bunch of leading zeros), leading it to print an _IOW(...) that doesn't produce the same result:

kdump:
 21174  21174 fido2-cred CALL  ioctl(4,_IOW('h',0x2,0x4),0x7f7fff1d8d1c)
kdump -n:
 21174  21174 fido2-cred CALL  ioctl(4,0xffffffff80046802,0x7f7fff1d8d1c)

Note that the true value of _IOW('h',0x2,0x4) is also spelled USB_HID_SET_RAW, but isn't sign-extended from 32-bit to 64-bit:

kdump:
 13581  13581 fido2-cred CALL  ioctl(4,USB_HID_SET_RAW,0x7f7fff228c0c)
kdump -n:
 13581  13581 fido2-cred CALL  ioctl(4,0x80046802,0x7f7fff228c0c)

>How-To-Repeat:
ktrace a program that does ioctl(fd, (int)USB_HID_SET_RAW, ...), kdump the result, get confused why _IOW('h',0x2,0x4) is not being decoded as USB_HID_SET_RAW
>Fix:
Yes, please!

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57918 CVS commit: src/usr.bin/kdump
Date: Sat, 10 Feb 2024 07:53:37 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Sat Feb 10 12:53:36 UTC 2024

 Modified Files:
 	src/usr.bin/kdump: kdump.c

 Log Message:
 PR/57918: Taylor R Campbell: Don't try to print symbolically ioctls that
 have bits set on the high 32 bits of a 64 bit long.


 To generate a diff of this commit:
 cvs rdiff -u -r1.140 -r1.141 src/usr.bin/kdump/kdump.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57918 CVS commit: src/usr.bin/kdump
Date: Sun, 11 Feb 2024 01:08:57 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Sun Feb 11 01:08:57 UTC 2024

 Modified Files:
 	src/usr.bin/kdump: kdump.c

 Log Message:
 PR bin/57918

 Redo the previous correctly, the test should be whether or not only the
 bottom 32 bits are set, rather than whether any of bits 32..63 are set.
 This makes no difference if u_long (or unsigned long long) is 64 bits,
 it is the same test in that case, but if u_long were 128 bits there is
 a notable difference (though for the particular issue observed in the PR,
 it is likely to work either way).


 To generate a diff of this commit:
 cvs rdiff -u -r1.141 -r1.142 src/usr.bin/kdump/kdump.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Taylor R Campbell <riastradh@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org, kre@NetBSD.org,
	christos@NetBSD.org
Subject: Re: PR/57918 CVS commit: src/usr.bin/kdump
Date: Sun, 11 Feb 2024 14:43:13 +0000

 Cool, thanks, christos and kre!

 Would be nice if we had automatic tests for this too, and we should
 get this pulled up.

From: Robert Elz <kre@munnari.OZ.AU>
To: Taylor R Campbell <riastradh@NetBSD.org>
Cc: gnats-bugs@NetBSD.org, gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org,
        christos@NetBSD.org
Subject: Re: PR/57918 CVS commit: src/usr.bin/kdump
Date: Mon, 12 Feb 2024 04:31:57 +0700

     Date:        Sun, 11 Feb 2024 14:43:13 +0000
     From:        Taylor R Campbell <riastradh@NetBSD.org>
     Message-ID:  <20240211144313.439526086B@jupiter.mumble.net>

   | Would be nice if we had automatic tests for this too,

 I'm not a big fan of adding tests for bugs that have been fixed,
 except where there's a reason to believe the bug might reappear
 (as in sources that we update from somewhere upstream, or perhaps
 something that is tricky and not obvious - neither of which applies
 here).

 Still, if there were kdump tests already, as a model, I could be
 persuaded to add one I guess, but without a foundation, I don't
 really have any thoughts on how to test kdump in any meaningful
 way.

   | and we should get this pulled up.

 I don't really think this is a serious enough (as in affects enough
 people) to bother the -10 release cycle with a pullup request right
 now, but after -10 is released, a pullup for 10.0_STABLE (and so to
 be in 10.1) is probably warranted.   I'm less convinced this is worth
 any effort at all for -9, and I'm convinced it isn't for -8.

 But if you disagree, feel free to send in pullup requests yourself,
 those don't need to come from the developer who made the changes.

 Alternatively change the PR status to "needs pullups" and assign it
 to me if you like.

 kre

State-Changed-From-To: open->pending-pullups
State-Changed-By: kre@NetBSD.org
State-Changed-When: Tue, 12 Mar 2024 09:33:20 +0000
State-Changed-Why:
[pullup-10 #635]


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57918 CVS commit: [netbsd-10] src/usr.bin/kdump
Date: Tue, 12 Mar 2024 10:13:05 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Tue Mar 12 10:13:05 UTC 2024

 Modified Files:
 	src/usr.bin/kdump [netbsd-10]: kdump.c

 Log Message:
 Pull up following revision(s) (requested by kre in ticket #635):

 	usr.bin/kdump/kdump.c: revision 1.141
 	usr.bin/kdump/kdump.c: revision 1.142
 	usr.bin/kdump/kdump.c: revision 1.143
 	usr.bin/kdump/kdump.c: revision 1.144

 PR/57918: Taylor R Campbell: Don't try to print symbolically ioctls that
 have bits set on the high 32 bits of a 64 bit long.

 PR bin/57918

 Redo the previous correctly, the test should be whether or not only the
 bottom 32 bits are set, rather than whether any of bits 32..63 are set.

 This makes no difference if u_long (or unsigned long long) is 64 bits,
 it is the same test in that case, but if u_long were 128 bits there is
 a notable difference (though for the particular issue observed in the PR,
 it is likely to work either way).
 decode some {g,s}etsockopt*.

 kdump: keep the comment with its corresponding code
 No binary change.


 To generate a diff of this commit:
 cvs rdiff -u -r1.140 -r1.140.2.1 src/usr.bin/kdump/kdump.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Tue, 12 Mar 2024 10:27:25 +0000
State-Changed-Why:
Pullups complete.


>Unformatted:

Home	PR Database Search