truss: improved support for decoding compat32 arguments

[arichardson]

Summary

Currently running truss -a -e does not decode any
argument values for freebsd32_* syscalls (open/readlink/etc.)

This change checks whether a syscall starts with freebsd{32,64}_ and if
so strips that prefix when looking up the syscall information. To ensure
that the truss logs include the real syscall name we create a copy of
the syscall information struct with the updated.

The other problem is that when reading string array values, truss
naively iterates over an array of char* and fetches the pointer value.
This will result in arguments not being loaded if the pointer is not
aligned to sizeof(void*), which can happens in the compat32 case. If it
happens to be aligned, we would end up printing every other value.
To fix this problem, this changes adds a pointer_size member to the
procabi struct and uses that to correctly read indirect arguments
as 64/32 bit addresses in the the compat32 case (and also compat64 on
CheriBSD).

The motivating use-case for this change is using truss for 64-bit
programs on a CHERI system, but most of the diff also applies to 32-bit
compat on a 64-bit system, so I'm upstreaming this instead of keeping it
as a local CheriBSD patch.

Output of truss -aef ldd32 /usr/bin/ldd32 before:
39113: freebsd32_mmap(0x0,0x1000,0x3,0x1002,0xffffffff,0x0,0x0) = 543440896 (0x20644000)
39113: freebsd32_ioctl(0x1,0x402c7413,0xffffd2a0) = 0 (0x0)
/usr/bin/ldd32:
39113: write(1,"/usr/bin/ldd32:\n",16) = 16 (0x10)
39113: fork() = 39114 (0x98ca)
39114: <new process>
39114: freebsd32_execve(0xffffd97e,0xffffd680,0x20634000) EJUSTRETURN
39114: freebsd32_mmap(0x0,0x20000,0x3,0x1002,0xffffffff,0x0,0x0) = 541237248 (0x2042a000)
39114: freebsd32_mprotect(0x20427000,0x1000,0x1) = 0 (0x0)
39114: issetugid() = 0 (0x0)
39114: openat(AT_FDCWD,"/etc/libmap32.conf",O_RDONLY|O_CLOEXEC,00) ERR#2 'No such file or directory'
39114: openat(AT_FDCWD,"/var/run/ld-elf32.so.hints",O_RDONLY|O_CLOEXEC,00) = 3 (0x3)
39114: read(3,"Ehnt\^A\0\0\0\M^@\0\0\0#\0\0\0\0"...,128) = 128 (0x80)
39114: freebsd32_fstat(0x3,0xffffbd98) = 0 (0x0)
39114: freebsd32_pread(0x3,0x2042f000,0x23,0x80,0x0) = 35 (0x23)
39114: close(3) = 0 (0x0)
39114: openat(AT_FDCWD,"/usr/lib32/libc.so.7",O_RDONLY|O_CLOEXEC|O_VERIFY,00) = 3 (0x3)
39114: freebsd32_fstat(0x3,0xffffc7d0) = 0 (0x0)
39114: freebsd32_mmap(0x0,0x1000,0x1,0x40002,0x3,0x0,0x0) = 541368320 (0x2044a000)

After:

783: freebsd32_mmap(0x0,4096,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANON|MAP_ALIGNED(12),-1,0x0) = 543543296 (0x2065d000)
783: freebsd32_ioctl(1,TIOCGETA,0xffffd7b0)    = 0 (0x0)

/usr/bin/ldd32:

783: write(1,"/usr/bin/ldd32:\n",16)           = 16 (0x10)
784: <new process>
783: fork()                                    = 784 (0x310)
784: freebsd32_execve("/usr/bin/ldd32",[ "(null)" ],[ "LD_32_TRACE_LOADED_OBJECTS_PROGNAME=/usr/bin/ldd32", "LD_TRACE_LOADED_OBJECTS_PROGNAME=/usr/bin/ldd32", "LD_32_TRACE_LOADED_OBJECTS=yes", "LD_TRACE_LOADED_OBJECTS=yes", "USER=root", "LOGNAME=root", "HOME=/root", "SHELL=/bin/csh", "BLOCKSIZE=K", "MAIL=/var/mail/root", "MM_CHARSET=UTF-8", "LANG=C.UTF-8", "PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/root/bin", "TERM=vt100", "HOSTTYPE=FreeBSD", "VENDOR=amd", "OSTYPE=FreeBSD", "MACHTYPE=x86_64", "SHLVL=1", "PWD=/root", "GROUP=wheel", "HOST=freebsd-amd64", "EDITOR=vi", "PAGER=less" ]) EJUSTRETURN
784: freebsd32_mmap(0x0,135168,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANON,-1,0x0) = 541212672 (0x20424000)
784: freebsd32_mprotect(0x20421000,4096,PROT_READ) = 0 (0x0)
784: issetugid()                               = 0 (0x0)
784: sigfastblock(0x1,0x204234fc)              = 0 (0x0)
784: open("/etc/libmap32.conf",O_RDONLY|O_CLOEXEC,00) ERR#2 'No such file or directory'
784: open("/var/run/ld-elf32.so.hints",O_RDONLY|O_CLOEXEC,00) = 3 (0x3)
784: read(3,"Ehnt\^A\0\0\0\M^@\0\0\0\v\0\0\0"...,128) = 128 (0x80)
784: freebsd32_fstat(3,{ mode=-r--r--r-- ,inode=18680,size=32768,blksize=0 }) = 0 (0x0)
784: freebsd32_pread(3,"/usr/lib32\0",11,0x80) = 11 (0xb)

[jrtc27]

usr.bin/truss/setup.c

93

This works just fine as void * even downstream in CheriBSD.

116

Meh, should be void * upstream and we can make it be Linux64 downstream.

136–143

Is more like we have downstream. It decouples the native ABI from what other ABIs you might know about so is easier to work out what it's doing at a glance about and easier to extend (otherwise you get a bunch of duplication once you add a third possible native ABI).

usr.bin/truss/syscalls.c

1071–1081

I still don't like this messing with stailq's and would be fine printing the non-prefixed syscall name (and we still have the ABI floating around if we wanted to reconstruct the prefixed name later).

1074–1075

freebsd64 should stay downstream

1830–1832

and keep strarray64 (which only makes sense alongside the __capability annotation for strarray) downstream

[arichardson]

usr.bin/truss/syscalls.c

1071–1081

I agree this is rather ugly. Let me have another look to see if I can simplify this while keeping the original names.

1074–1075

yeah will remove that before commiting. Missed it while cleaning up CHERI stuff.

1830–1832

I prefer int64_t here because a) it makes the diff to cheribsd a bit smaller and b) we only ever care about the address so using a pointer could suggest that it's dereferenceable.

[arichardson]

usr.bin/truss/setup.c

93

Well, sizeof(void* __capability) in order to support a hybrid truss, but I guess that's simpler.

[jrtc27]

usr.bin/truss/setup.c

93

Sure, though your proposed patch didn't actually do that :)

usr.bin/truss/syscalls.c

1830–1832

uintptr_t strarray then? (Which can be uintcap_t for us instead)

[arichardson]

usr.bin/truss/setup.c

93

It did (https://github.com/CTSRD-CHERI/cheribsd/pull/692/files), not sure why I changed it here. I think to match the linux case...

usr.bin/truss/syscalls.c

1830–1832

That would work although I think it the code below a bit less readable.
It would then need to be something like

			if (pointer_size == sizeof(u.strarray[0])) {
				if (u.strarray[i] == 0)
					break;
				straddr = u.strarray[i];
			} else if (pointer_size == 4) {
				if (u.strarray32[i] == 0)
					break;
				/* sign-extend 32-bit pointers */
				straddr = (intptr_t)u.strarray32[i];
			}

I think I prefer the explicit size version.

[jrtc27]

usr.bin/truss/syscalls.c

1830–1832

What's wrong with that? Though I'd make it more like the kernel COMPAT_FREEBSD32 code and add #ifs like:

#if __SIZEOF_POINTER__ > 4
			if (pointer_size == 4) {
				if (u.strarray32[i] == 0)
					break;
				/* sign-extend 32-bit pointers */
				straddr = (intptr_t)u.strarray32[i];
			} else
#endif
			{
				if (u.strarray[i] == 0)
					break;
				straddr = u.strarray[i];
			}

possibly with a pointer_size check and errx in the else branch if you want.

[arichardson]

• Rebase on top of D27636
• fix ubsan array bounds error

[arichardson]

rebase

[arichardson]

rebase again

[arichardson]

rebase after D27636 changes

[jrtc27]

Hm. Perhaps this should be word_size (or similar) and __SIZEOF_LONG__ rather than pointer_size and __SIZEOF_POINTER__ so there's less churn for CHERI?

[jhb]

I had long-standing todo list to add freebsd32, but I was going to do it manually. This is much nicer. I have a test program I use that exercises most syscalls (or least, different argument values to exercise decoding) that I typically run under truss and kdump when making changes and then compare the before after with kompare to ensure the changes are all desired changes. If you have a git branch these changes sit in I can run that and see how it compares before/after for freebsd32.

usr.bin/truss/syscalls.c

1030–1036

Yes, it's needed for all 32-bit FreeBSD ABIs. One way to handle this might be to have a function pointer in 'abi' for "syscallfixup" and use quad_fixup as that for the FreeBSD ABI (on ILP32) and the FreeBSD32 ABIs (on LP64). This would also permitting adding different behavior if needed for Linux32 in the future.

1081

Rather than hardcoding this here, let's add a new member to abi that is something like 'compat_prefix'. You would define it to "freebsd32_" over in main.c for the FreeBSD 32 compat ABIs. Then this code would be something like:

lookup_name = name;
if (strncmp(abi->compat_prefix, name, strlen(compat_prefix)) == 0)
    lookup_name += strlen(compat_prefix);

This would let you handle freebsd64_ easily in CheriBSD by just adding it to the FreeBSD64 ABI over in main.c. It would also permit handling Linux32 if someone really wanted to. It also avoids potential false positive matches.

1873–1888

Is what style(9) prefers for infinite loops.

[arichardson]

• Rebase and add the compat_prefix member

[arichardson]

In D27625#637995, @jhb wrote:

I had long-standing todo list to add freebsd32, but I was going to do it manually. This is much nicer. I have a test program I use that exercises most syscalls (or least, different argument values to exercise decoding) that I typically run under truss and kdump when making changes and then compare the before after with kompare to ensure the changes are all desired changes. If you have a git branch these changes sit in I can run that and see how it compares before/after for freebsd32.

I've pushed the changes to https://github.com/arichardson/freebsd/tree/truss-cleanups

usr.bin/truss/syscalls.c

1030–1036

I had a quick look at adding the function pointer, since that seems like the cleaner solution. However, that makes the changes more invasive since I need to move lots of stuff around. I think it makes more sense as a follow-up commit especially if all 32-bit ABIs (including Linux32) need it.

[arichardson]

rebase

[arichardson]

ping?

[jhb]

It's fine with me modulo the nits.

usr.bin/truss/syscalls.c

1071

This seems to be a duplicate line now?

1082

It's too bad there's no 'strstartswith()' or the like.