bugzilla 213937, syscall(int, ...), struct ktr_syscall's short ktr_code, struct syscall_args's u_int code: should the bug be closed?

[Mark Millard]

On 2020-Mar-8, at 10:53, Konstantin Belousov <kostikbel at gmail.com> wrote:

> 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) {

Exploring what results with truncation present,
not worrying about sign-extension . . .

A example odd report by kdump for
syscall(0x654321,0x123456,0x214365,0x342156)
is:

 61018 a.out    CALL  [17185]
 61018 a.out    RET   [17185] -1 errno 78 Function not implemented

(17185 is decimal for 0x4321.)

An example of how bad truncation can be by
comparison:

syscall(0x87650021,0x123456,0x214365,0x342156)
shows:

 60996 a.out    CALL  access
 60996 a.out    RET   access -1 errno 78 Function not implemented

So: no report of any of the bad-code/number's
bits but a report that the value was rejected.

(This was not from using the patch. Just -r538510
behavior tested.)

I'll note that the alternate of using truss has
no truncation problems or missed bits from the
number:

-- UNKNOWN FreeBSD SYSCALL -2023423967 --
#-2023423967()                                   ERR#78 'Function not implemented'

-2023423967 is decimal for 0x87650021 (2's complement, 32-bit).

For truss, the only thing that might be considered
slightly odd is the use of 2's complement. But int
vs. u_int shows up elsewhere anyway.

That truss behaves this way might cover some
for the oddities of ktrace/kdump's handling.

[I've updated https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=213937
with comment 5 for such material and for corrections to my 2016
misinterpretation of where the code/number came from for the example
from back then.]

===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)