Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 8 Mar 2020 19:53:06 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Mark Millard <marklmi@yahoo.com>
Cc:        FreeBSD Hackers <freebsd-hackers@freebsd.org>, jhb@freebsd.org
Subject:   Re: bugzilla 213937, syscall(int, ...), struct ktr_syscall's short ktr_code, struct syscall_args's u_int code: should the bug be closed?
Message-ID:  <20200308175306.GM98340@kib.kiev.ua>
In-Reply-To: <5AEC7760-EF38-4738-AEBD-9E563CB0E62B@yahoo.com>
References:  <5AEC7760-EF38-4738-AEBD-9E563CB0E62B.ref@yahoo.com> <5AEC7760-EF38-4738-AEBD-9E563CB0E62B@yahoo.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Mar 07, 2020 at 09:42:29PM -0800, Mark Millard via freebsd-hackers wrote:
> Question: Should bugzilla 213937 be left as-is to suggest
> changing things so that more bad syscall "code/number"
> values are preserved and reported correctly if/when they
> happen?
> 
> Background leading to the question . . .
> 
> I've been reviewing some of my old bugzilla reports that
> are still in the new state, closing or adding notes to a
> bunch of them. The status for what contributes to
> bugzilla 213937 has not changed over the years since the
> submittal.
> 
> Bugzilla 213937 was a report of ktrace misreporting the 24
> bit field value in the armv7 svclt instruction. (I had a bad
> file that had such an instruction with a large 24-bit field
> value that lead to seeing the problems. It was rather
> confusing until I figured out the information reported was
> incorrect relative to the instruction encoding.)
> 
> Part of the problem was that it looked like the lower 16
> bits had been used but sign extended to produce the 
> reported value, a negative number as reported. (It took
> a while to notice that.)
> 
> Oleksandr Tymoshenko eventually reported in comment #2
> that there is the odd mix of types in use overall
> (I'm using the "code" terminology below):
> 
> syscall(int code,...)
> struct ktr_syscall's short ktr_code
> struct syscall_args's u_int code
> 
> Sure enough, using ktr_code could generate a sign-extension
> of a 16-bit value. But syscall and syscall_args are a
> little odd as well (signed vs. unsigned at least).
> 
> It looks like "short ktr_code" has been around for long
> before I ever ran into it and might well be expected by
> folks with more historical background than I had. But it
> can not preserve all int or u_int values for FreeBSD's
> normal definitions of those types.
> 
> Is this something that should be fixed? Does it have a
> reason for being as it is?

I do not think that we want to change the layout of KTR_SYSCALL message.
We can re-define the message number and extend the ktr_code member to
full u_int, but the general hope is that we are very far from having 32K
syscalls defined, and will be for quite some time.

It might be that some relief for invalid printing can be provided by
changing the ktr_code type to unsigned short.  It does not fix truncation,
but would avoid sign-extension bug.  Or even this.

diff --git a/usr.bin/kdump/kdump.c b/usr.bin/kdump/kdump.c
index 3b597533359..522877ccb2d 100644
--- a/usr.bin/kdump/kdump.c
+++ b/usr.bin/kdump/kdump.c
@@ -796,7 +796,7 @@ ktrsyscall(struct ktr_syscall *ktr, u_int sv_flags)
 	intmax_t arg;
 	int quad_align, quad_slots;
 
-	syscallname(ktr->ktr_code, sv_flags);
+	syscallname((unsigned short)ktr->ktr_code, sv_flags);
 	ip = first = &ktr->ktr_args[0];
 	if (narg) {
 		char c = '(';
@@ -1534,7 +1534,7 @@ ktrsysret(struct ktr_sysret *ktr, u_int sv_flags)
 	register_t ret = ktr->ktr_retval;
 	int error = ktr->ktr_error;
 
-	syscallname(ktr->ktr_code, sv_flags);
+	syscallname((unsigned short)ktr->ktr_code, sv_flags);
 	printf(" ");
 
 	if (error == 0) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200308175306.GM98340>