libc: fix the initgroups(3) compat path

[kevans]

Summary

setgroups@FBSD_1.0 comes from libsys and freebsd14_setgroups was
intended to be an alias of that, but this doesn't actually work as
written. In reality, the reference in initgroups(3) compat version is
instead resolved to some other symbol (accept(2) in my testing), which
then breaks things.

Remove the include/compat.h bits because they're misleading, and instead
provide an alias for setgroups(2) now that we have a user of that. I
think there's an implication that other compat symbols in
include/compat.h might be broken in similar subtle ways since the libsys
split, but I haven't looked at any of that.

Fixes: 9dc1ac86919 ("initgroups(3): Add a pre-FreeBSD-15-compat [...]")
Fixes: 9da2fe96ff2 ("kern: fix setgroups(2) and getgroups(2) [...]")

[olce]

Admittedly, I didn't test the compat' version of initgroups() as the code was the same as the latest version. However, out of curiosity, I had looked at the assembly code produced in libc, and had noticed something strange that I probably should have taken more seriously. objdump on libc.so reported that the call to the compat' version of setgroups() in the source code had been substituted with a call to uuidgen()... I then looked at other calls to compat' system calls and noticed the exact same phenomenon. Looking at the symbol table, I saw that all these compatibility symbols for some reason had the same address as that of uuidgen(). At this point, I just assumed that, since other compat' calls were not known to be broken, this was how compat' symbols were supposed to work, and that somehow these would be resolved correctly when libc.so would pull libsys.so.

Back to the code here, for now I do not understand these changes at all, see inline comments.

lib/libc/gen/gen-compat.h

55–56

This should not be necessary either, at least no more than for other compat' system calls. objdump on libsys.so lists freebsd14_setgroups, so I don't see why we would add to add this annotation. Maybe freebsd14_setgroups from libsys.so is not exported correctly to libc.so?

lib/libc/include/compat.h

72–73

I added these as compat.h is included in the object files produced by each system call, and I thought these were necessary so that the linker knows that freebsd14_setgroups, which is automatically produced by our syscall scripts from syscalls.master, is in fact setgroups@FBSD_1.0 when linking libsys.so. Without these, where does the mapping between freebsd14_setgroups and setgroups@FBSD_1.0 come from?

[kevans]

lib/libc/include/compat.h

72–73

Oh, fair point- I didn't realize we had exported the freebsd11_ symbols in libsys/FBSDprivate_1.0. If that works, I'll just commit that as a Fixes: to the earlier commit that bumped setgroups/getgroups.

[kevans]

lib/libc/include/compat.h

72–73

Exporting freebsd14_setgroups@FBSDprivate_1.0 in libsys works, but only if I also hack up libsys/Makefile.sys to filter freebsd14_setgroups* out of MIASM to avoid creating conflicting weak refs in the libc build.

[kib]

lib/libc/include/compat.h

72–73

I think that this is in fact first time when we directly call the previous version of the syscall directly in libc. From this PoV it might be better to use syscall(2) to avoid depending on the specific compat symbol exported from some lib.

[kib]

lib/libc/include/compat.h

72–73

Without syscall(2) usage, the following seems to be good enough.

diff --git a/lib/libc/gen/initgroups.c b/lib/libc/gen/initgroups.c
index a1a7d92250e2..828df4f11cfc 100644
--- a/lib/libc/gen/initgroups.c
+++ b/lib/libc/gen/initgroups.c
@@ -76,10 +76,13 @@ initgroups(const char *uname, gid_t agroup)
 	return (initgroups_impl(uname, agroup, setgroups));
 }
 
+static int xfreebsd14_setgroups(int, const gid_t *)
+    __attribute__((__weakref__("setgroups@FBSD_1.0")));
+
 int
 freebsd14_initgroups(const char *uname, gid_t agroup)
 {
-	return (initgroups_impl(uname, agroup, freebsd14_setgroups));
+	return (initgroups_impl(uname, agroup, xfreebsd14_setgroups));
 }
 
 __sym_compat(initgroups, freebsd14_initgroups, FBSD_1.0);

[kevans]

OBE