Avoid two suword() calls per auxarg entry.

[brooks]

Summary

Instead, construct an auxargs array and copy it out all at once.

Use an array of Elf_Auxinfo rather than pairs of Elf_Addr * to represent
the array. This is the correct type where pairs of words just happend
to work. To reduce the size of the diff, AUXARGS_ENTRY is altered to act
on this array rather than introducing a new macro.

Return errors on copyout() and suword() failures and handle them in the caller.

Incidentally fixes AT_RANDOM and AT_EXECFN in 32-bit linux on amd64
which incorrectly used AUXARG_ENTRY instead of AUXARGS_ENTRY_32
(now removed).

[brooks]

This patch compiles (if you don't build linux emulation), but is incomplete. I've tested a variant on CheriBSD. In this review I'm looking for feedback on the approach before altering AUXARGS_ENTRY_32 and handling linux emulation. This is certainly more efficient and might be measurably so on i386 with 4/4.

[kib]

sys/kern/imgact_elf.c

1159

Why printing it out ? Other copyouts into the new process address space fail silently.

I think all code should be same in this regard, i.e. we should uprintf() the failure and return some error from execve(), so that the process terminates with a signal.

[brooks]

sys/kern/imgact_elf.c

1159

I put that in while I was debugging on CheriBSD. I'll update to return an error.

[brooks]

• Return error on copyout/suword failure in freebsd_fixup.

[kib]

sys/kern/imgact_elf.c

1101–1103

Merge this and previous line ?

1106–1108

() can be dropped. I am not sure about split of 2 into 1 and 1, but I do not object.

1157–1158

I think cast can be dropped.

sys/kern/kern_exec.c

696–699

() around == are not needed.

[emaste]

sys/kern/imgact_elf.c

1106–1108

To me two separate + 1s makes a more clear suggestion that there's one NULL terminator per list.

sys/sys/imgact_elf.h

41

(pos)++->a_un.a_val reads a bit awkwardly

[brooks]

• Cleanups suggested by kib.
• Break out the increment of pos.
• Extend to cover linux compat and eliminate AUXARGS_ENTRY_32.

[brooks]

Trivial testing done with amd64, freebsd32 on amd64, and linux64 on amd64.

[brooks]

• Fix build on i386.

[jhb]

sys/amd64/linux32/linux32_sysvec.c

210–212

Use the two + 1's here as well?

sys/kern/imgact_elf.c

1101–1103

My only question is if the stack usage is ok. On LP64 with our current AT_COUNT of 27 (which might very well grow), this uses 432 bytes. With CHERI 128 that is up 864 bytes (and CHERI 256 is almost 2K). Even in the LP64 case I think that's perhaps a big chunk of stack space.

[brooks]

sys/kern/imgact_elf.c

1101–1103

I was debating which was best and I agree 432 is large and AT_COUNT is at least 31 on CHERI where I've punted argc, argv, and envv (as well as envc because, why not) to auxargs. I'll make the switch. I also noticed while doing so that I'd failed to zero argarray so there was a large kernel stack leak.

[brooks]

• Allocate argarray with malloc() and zero to prevent leak.
• Use argc + 1 + envc + 1 idiom one more place

[jkim]

Unfortunately, this change broke 32-bit Linuxulator on amd64. Please see PR228790.

[jkim]

I found there's a copy-and-pasto and it should be fixed with r335020.