cred: Fix a race in the FreeBSD-14-compatible setgroups(2)

[olce]

Summary

The freebsd14_setgroups() function would try to modify the effective GID
on the current process' credentials without holding the process lock,
allowing races with other threads concurrently modifying the process
credentials. In the worst case, freebsd14_setgroups() could be
manipulating a 'struct ucred' already freed by another thread (in the
very small window after reading 'p_ucred' without lock but before
modifying its field). Concurrent uses of freebsd14_setgroups() or
setcred() could also lead to non-atomic credentials modifications.

Fix this by making kern_setgroups() take a new boolean indicating
whether the passed array includes the effective GID in its first slot.
When this boolean is true, it internally keeps the effective GID in
a separate variable, pretends that the groups[] array that was passed
actually starts at 'groups + 1', do the usual steps to set the
supplementary groups and the extra ones to set the effective GID along,
without releasing the process lock in between.

Reported by: markj

[markj]

sys/kern/kern_prot.c

1237

Don't we want to pass groups instead of groups + 1 now?

sys/sys/syscallsubr.h

361

Can we make this function private to kern_prot.c? I don't see a reason to export it, and most other credential-switching system calls don't have anything analogous.

[olce]

sys/kern/kern_prot.c

1237

Of course...

sys/sys/syscallsubr.h

361

I was going to say "yes", but after a lot of reflection, and although this declaration in syscallsubr.h is not functionally necessary right now, I'm reluctant to remove it. It's true that the 32-bit compatibility system call does not need to access kern_setgroups(), as its arguments and return types in 32-bit mode can be handled generically (and probably the same for CheriBSD and 64-bit mode) so sys_setgroups() can be called directly. However, we should most probably be using it in the Linux compatibility instead of the duplicated code we have there (see linux_setgroups(). Typically, I see that that code lacks the exec_block_pass() that was added to kern_setgroups(), which itself looks slightly suspicious (I'll have a look at this new machinery; BTW, is it intended that D57497 is still not publicly accessible although it apparently was committed end of June?).