initgroups(3): Add a pre-FreeBSD-15-compatible version

[olce]

Summary

After commit 9da2fe96ff2e ("kern: fix setgroups(2) and getgroups(2) to
match other platforms"), setgroups() does not set the effective GID
anymore and uses all passed groups as the supplementary group list.
This effectively breaks backwards compatibility with programs/libraries
compiled on a FreeBSD 14 or earlier system.

Restore compatibility by creating a new version of the 'initgroups'
symbol that designates the current implementation and providing
a pre-FreeBSD-15-compatible version under the symbol's previously
exported version. The new version calls the new setgroups(2) system
call, while the compatible one calls the original one (called
freebsd14_setgroups()).

Update the manual page with some history and comparison with other
current open-source systems. Add a "SECURITY CONSIDERATIONS" section
highlighting some security properties of this approach and the reasons
we adopt it. While here, revamp the manual page, in particular to use
the exact POSIX terminology where possible.

Fixes: 9da2fe96ff2e ("kern: fix setgroups(2) and getgroups(2) to match other platforms")
Sponsored by: The FreeBSD Foundation

[kevans]

This was an intentional decision, not an oversight. On every other platform it does exactly what we had previously documented, leaving the egid alone and ensuring the passed-in basegid is added to the supplementary group list.

[olce]

A priori, we have to decide between two alternatives for initgroups(3):

1. We restore the exact old behavior, which was modified by the setgroups(2)/getgroups(2) changes: The additional group passed to initgroups() is in fact meant to become the effective GID, and those retrieved from the groups database the supplementary groups.
2. Or we agree with the change of behavior, but then we have to add a new symbol version for initgroups(3) and provide the new implementation in this review as the backwards compatible implementation.

In any case, I don't think the status quo is acceptable because of the backwards compatibility requirement.

Going to take a little bit more time to ponder the impacts of point 2 in existing callers. Point 2 means that the primary group is also included in the supplementary groups set, which has the impact that unprivileged setgid programs cannot get rid of it.

I'm pretty sure we have documentation somewhere saying that if admins want that, they explicitly have to list a user as a member of its groups in the groups database (if they don't, the effective GID is not part of the supplementary groups set). I'm slightly reluctant to give up that degree of liberty. On the other hand, I understand that other systems already do point 2; I've checked only glibc so far; in its getgrouplist() implementation, it still treats the first GID separately (it is the one passed as an argument) and only checks for duplication for the others, but in the end initgroups() passes everything to setgroups() anyway (this is a bit silly, but I gather the reason is most probably getgrouplist() backwards compatibility; and anyway this doesn't really help answer the initial question). Our getgrouplist() does the same. I'll quickly second check NetBSD, OpenBSD and illumos tomorrow. Also, the degree of liberty evoked above might actually be seen as a drawback.

This was a summary of my current, moving thoughts. I'll sleep on them, and hopefully things will be clearer tomorrow (for now, I'm inclining towards 2, but after checking the callers and the doc I mentioned). Ideas/arguments welcome.

[kevans]

In D52282#1195193, @olce wrote:

A priori, we have to decide between two alternatives for initgroups(3):

1. We restore the exact old behavior, which was modified by the setgroups(2)/getgroups(2) changes: The additional group passed to initgroups() is in fact meant to become the effective GID, and those retrieved from the groups database the supplementary groups.
2. Or we agree with the change of behavior, but then we have to add a new symbol version for initgroups(3) and provide the new implementation in this review as the backwards compatible implementation.

Right, sorry- the failure to provide a new version was an oversight. I have a slight preference for implementing the compat symbol in terms of the compat setgroups, though, for provenance purposes when glancing over syscall traces/audit logs -- it's nice to be able to glance over and realize I'm dealing with something that hasn't linked against new setgroups/initgroups versions.

In any case, I don't think the status quo is acceptable because of the backwards compatibility requirement.

Going to take a little bit more time to ponder the impacts of point 2 in existing callers. Point 2 means that the primary group is also included in the supplementary groups set, which has the impact that unprivileged setgid programs cannot get rid of it.

I'm pretty sure we have documentation somewhere saying that if admins want that, they explicitly have to list a user as a member of its groups in the groups database (if they don't, the effective GID is not part of the supplementary groups set). I'm slightly reluctant to give up that degree of liberty. On the other hand, I understand that other systems already do point 2; I've checked only glibc so far; in its getgrouplist() implementation, it still treats the first GID separately (it is the one passed as an argument) and only checks for duplication for the others, but in the end initgroups() passes everything to setgroups() anyway (this is a bit silly, but Igather the reason is most probably getgrouplist() backwards compatibility; and anyway this doesn't really help answer the initial question). Our getgrouplist() does the same. I'll quickly second check NetBSD, OpenBSD and illumos tomorrow. Also, the degree of liberty evoked above might actually be seen as a drawback.

This was a summary of my current, moving thoughts. I'll sleep on them, and hopefully things will be clearer tomorrow (for now, I'm inclining towards 2, but after checking the callers and the doc I mentioned). Ideas/arguments welcome.

[kevans]

My dilemma here is that, assuming the docs you recall for admins does exist (I don't recall either way), we have two competing notions of how this works:

• initgroups(3) has advertised *to developers* for many years that it sets up 'the group access list' and ensures that basegid is included in it. It does note that it's "with setgroups(2)", but we have to remember that that means different things for different developers. We can't really know how many people developing for (but not actively using) FreeBSD would look at this manpage and decide that describes the same behavior as on any other system.
• On the other hand, we're conditioning sysadmins to as noted, which is a somewhat contradictory stance but probably also one that we've held for many years.

This isn't really a fun position to be in.

[olce]

TL;DR: Please jump to "Finally going back to the dilemma" below.

In D52282#1195248, @kevans wrote:

This isn't really a fun position to be in.

Yes, that's exactly why I am hesitating, and I'll develop a bit more with some findings.

In D52282#1195248, @kevans wrote:

(...) assuming the docs you recall for admins does exist (I don't recall either way), we have two competing notions of how this works:

I've found back the passage I was thinking of. It is the following paragraph in setgroups.2, which has just been removed in commit 9da2fe96ff2e ("kern: fix setgroups(2) and getgroups(2) to match other platforms"):

The first entry of the group array (gidset[0]) is used as the effective
group-ID for the process.  This entry is over-written when a setgid
program is run.  To avoid losing access to the privileges of the
gidset[0] entry, it should be duplicated later in the group array.  By
convention, this happens because the group value indicated in the
password file also appears in /etc/group.  The group value in the
password file is placed in gidset[0] and that value then gets added a
second time when the /etc/group file is scanned to create the group set.

This paragraph was introduced with the not-that-old commit of interest 8557409f20b2 ("In the C library, the setting up of the group array..."), in which @mckusick says:

There is some confusion about the setgroups system call because in BSD it has (always) set the entire group including the egid group (in group[0]). However, in Linux, it skips over group[0] and starts setting from group[1].

which is partly true, but seems in part exaggerated (depending on what you call "BSD"). setgroups(2) indeed has changed the entire effective groups set (effective GID + supplementary groups) in 4.4BSD and FreeBSD (up to our recent changes). It has not in NetBSD except in the very early days, nor in OpenBSD which inherited NetBSD's behavior at fork. And, if POSIX XRAT is correct (see reference just below), this wasn't the case either in 4.2BSD nor 4.3BSD (nor System V Release 4).

As a parenthesis, another source of confusion is POSIX itself. POSIX.1-1990 was inconsistent in its meaning of "supplementary group ID" (it allowed the effective GID to be considered a supplementary group but at the same time mandated, e.g., in setgid(), that no supplementary groups would be changed where the effective GID was supposed to be changed). This fortunately was fixed later; the effective GID is said to be independent of the supplementary groups set. There is an interesting paragraph in POSIX XRAT, called "Supplementary Group ID" which traces the thinking of the standard makers, whose conclusion I've just summarized. However, while the terminology has been fixed, POSIX still allows getgroups() to return the effective GID in addition to the supplementary groups set. Because getgroups() is the only portable way to have a view of the process' supplementary groups set, this looks like a bad design (but still done to accomodate different systems). Anyway, it seems a camp has been chosen in practice, at least by the open source world: The effective GID should not be returned by getgroups() (neither Linux, nor NetBSD, nor OpenBSD, nor illumos return it; I hear Darwin does, but have not checked), and this is what we are now embracing, giving even more weight to it. And same for the unstandardized setgroups(), which is always used in conjunction with setegid() or setgid() in all portable code I've seen so far. So far, so good.

Going back to the passage quoted above, it says that the group associated to some user (in the password database; it is to become the effective GID) is supposed to also be added to the supplementary groups set, thanks to the "convention" that this group must appear *both* in the password and group database. This "convention" is however neither enforced by pw user nor by adduser (which does nothing special and relies on pw(8)).

On the runtime front, neither login(1) nor su(1) were doing it, as they use setusercontext(LOGIN_SETGROUP) which in the end calls setgid() and initgroups(). With the recent change of setgroups() and thus initgroups(), they are now actually enforcing that the effective GID is also one of the supplementary groups at runtime, a behavior which is no more tunable in contrast with the "convention" of how to update the password and group databases. This is the main change in behavior I'm not very comfortable with.

• initgroups(3) has advertised *to developers* for many years that it sets up 'the group access list' and ensures that basegid is included in it. It does note that it's "with setgroups(2)", but we have to remember that that means different things for different developers.

Yes. "group access list" isn't clear (depending on the context, it means only the supplementary groups or these plus the effective GID) and doesn't refer to any standard wording. In other recent reviews, I'm proposing to use "effective groups list/set" (which isn't standard either, but at least uses the standard word "effective" and seems clearer) to designate the effective GID and the supplementary groups, and just "supplementary groups (list/set)" when not including the effective GID.

It seems that what happened is that initgroups() implemented in GNU libc just worked by using getgrouplist() and setgroups(), possibly based on BSD/SunOS precedent. While getgrouplist() was implemented with mostly the same assumptions as that in BSD (and still is today; i.e., the passed GID is also returned back as the first slot in the result; funnily, though, that GID is specifically removed from the rest of the array that comes from the group database), setgroups() in Linux from the start (introduced in Linux 0.12, 1992/01/15; before, Linux didn't support multiple groups per process) only changed the supplementary groups. Maybe nobody really noticed at that time, but I don't really know (didn't stumble on anything conclusive).

NetBSD folks did notice for sure. They first changed initgroups() to also call setgid() and then setgroups() on the groups array returned by getgrouplist() with the first element skipped, but then backed that out to be "compatible with
SunOS and SysV" (see mycroft's commit dated 1995/06/03), effectively passing also the effective GID to setgroups() (only setting the supplementary groups).

• On the other hand, we're conditioning sysadmins to as noted, which is a somewhat contradictory stance but probably also one that we've held for many years.

Yes, as detailed above: The text in setgroups(2), the fact that pw(8) and adduser(8) do not by default add the effective GID into the groups database, the fact that login(1) and su(1) do not either at runtime. All these point towards a practice where the effective GID has never been added into the supplementary groups automatically in FreeBSD.

Finally going back to the dilemma: Should initgroups(3) include the effective GID automatically in the supplementary groups?

• For
  • Full compatibility with the following open source systems: Linux+glibc, NetBSD, OpenBSD, illumos.
  • As the initial effective GID always stays in the supplementary groups, it can reliably be used as a tag to authorize/deny accesses regardless of effective GID change.
  • No need for the administrator to manually insert each user in its group in the groups database, risking discrepancies between users.
• Against
  • Breaks with FreeBSD's existing practice from the start.
  • Probably incompatible with Darwin/MacOS.
  • Less fine-grained control on the supplementary groups.
  • We could instead change pw(1) and adduser(1) to add by default some user in its group in the groups database, gaining some of the benefits of the other approach while retaining finer-grained control.
  • setgid executables, or privileged processes executing setgid()/setegid() only do not get rid of the initial user's group (it stays in the supplementary set). Thus, the resulting processes can still access files belonging to that user's group.

If it was not for compatibility, I would have chosen "Against" without hesitation as it leaves more leeway to administrators and we can alleviate inconsistencies issues between users by changing pw(1) and adduser(1). I'm personally not (yet?) convinced that full initgroups(3) compatibility is really compelling.

So, I'm at the moment more inclined to stay with "Against". In practice, I would modify initgroups() to just call setgroups() but omitting the first group returned by getgrouplist() (the one explicitly passed to initgroups()). A possible refinement would be to change pw(8) and adduser(8) to automatically include the user's group from the password database in the groups database, possibly based on some flag/config (command-line, pw.conf, adduser.conf).
Alternatives:

• Instead, just call setgroups(2) on the whole list returned by getgrouplist(3), that's the "For" alternative above.
• In addition, call setegid(2) to change the effective GID, this is equivalent to the current patch (which falls under "Against" as well for the dilemma above), basically restoring the previous behavior (before the setgroups(2)/getgroups(2) changes). However, other implementations never set the effective GID. "Portable" programs usually support our idiosyncrasy, but there are bugs sometimes (kevans@ uncovered one recently in OpenSSH).

What do people think?

[kevans]

In D52282#1195513, @olce wrote:

TL;DR: Please jump to "Finally going back to the dilemma" below.

In D52282#1195248, @kevans wrote:

This isn't really a fun position to be in.

Yes, that's exactly why I am hesitating, and I'll develop a bit more with some findings.

In D52282#1195248, @kevans wrote:

• On the other hand, we're conditioning sysadmins to as noted, which is a somewhat contradictory stance but probably also one that we've held for many years.

Yes, as detailed above: The text in setgroups(2), the fact that pw(8) and adduser(8) do not by default add the effective GID into the groups database, the fact that login(1) and su(1) do not either at runtime. All these point towards a practice where the effective GID has never been added into the supplementary groups automatically in FreeBSD.

Finally going back to the dilemma: Should initgroups(3) include the effective GID automatically in the supplementary groups?

• For
  • Full compatibility with the following open source systems: Linux+glibc, NetBSD, OpenBSD, illumos.
  • As the initial effective GID always stays in the supplementary groups, it can reliably be used as a tag to authorize/deny accesses regardless of effective GID change.
  • No need for the administrator to manually insert each user in its group in the groups database, risking discrepancies between users.
• Against
  • Breaks with FreeBSD's existing practice from the start.
  • Probably incompatible with Darwin/MacOS.
  • Less fine-grained control on the supplementary groups.
  • We could instead change pw(1) and adduser(1) to add by default some user in its group in the groups database, gaining some of the benefits of the other approach while retaining finer-grained control.
  • setgid executables, or privileged processes executing setgid()/setegid() only do not get rid of the initial user's group (it stays in the supplementary set). Thus, the resulting processes can still access files belonging to that user's group.

If it was not for compatibility, I would have chosen "Against" without hesitation as it leaves more leeway to administrators and we can alleviate inconsistencies issues between users by changing pw(1) and adduser(1). I'm personally not (yet?) convinced that full initgroups(3) compatibility is really compelling.

So, I'm at the moment more inclined to stay with "Against". In practice, I would modify initgroups() to just call setgroups() but omitting the first group returned by getgrouplist() (the one explicitly passed to initgroups()). A possible refinement would be to change pw(8) and adduser(8) to automatically include the user's group from the password database in the groups database, possibly based on some flag/config (command-line, pw.conf, adduser.conf).
Alternatives:

• Instead, just call setgroups(2) on the whole list returned by getgrouplist(3), that's the "For" alternative above.
• In addition, call setegid(2) to change the effective GID, this is equivalent to the current patch (which falls under "Against" as well for the dilemma above), basically restoring the previous behavior (before the setgroups(2)/getgroups(2) changes). However, other implementations never set the effective GID. "Portable" programs usually support our idiosyncrasy, but there are bugs sometimes (kevans@ uncovered one recently in OpenSSH).

What do people think?

I don't think macOS is a particularly compelling reason to retain setting the egid, personally, though they are an active downstream for a solid chunk of base -- they can patch it back in to retain previous compatibility, but most of the software coming *in* to FreeBSD isn't typically coming from macOS. They'll already have to evaluate their own usage of initgroups() with the same kinds of problems in mind that we've raised in the course of this work, and initgroups usage in base is really not that widespread.

Breaking existing practice is a reasonable argument, though we'd already decided there's a good reason to break equally-longstanding precedence by making the setgroups/getgroups changes to begin with.

I think there's a solid argument for skipping the basegid-derived first element. I had wondered if it makes sense to make that contingent on whether it matches the current egid or not, but it seems like there's a pretty mixed bag of initgroups + setgid vs. setgid + initgroups ordering where we'd probably still do something weird or maybe wrong half of the time.

[olce]

Pondering some more on this issue, I'm finally leaning towards full compatibility with other systems.

Supplementary groups are always retained on functions changing the "primary" groups (setgid() and co.), so a setgid application that is not run as root cannot get rid of the initial user's supplementary groups. Such an application can only switch out of the effective GID, which is an outlier. That inconsistency is even more peculiar in a frame where each user has its own specific base group, since the user will stay the same but without being associated to his group. If the application does not switch all its primary groups, it can still access the initial effective group's files by switching GIDs, so putting the effective GID in the supplementary groups does not give more access in this case (except that it does give access without switching GIDs; but the user is anyway not changed, so this is not that surprising). If it does switch its primary groups, all bets are off as to whether it can still access its original GID's files depending on the system it is running on, and it is likely that open-source applications have catered to Linux+glibc behavior for a long time (i.e., it can still access). It's arguable that's more a POSIX's lack of standardization problem (maybe for good reasons, e.g., because of some other implementations) than everything else.

Also, I've stumbled by chance on the fact that, from the 4.4BSD Lite import to 2007, our getgrouplist() would actually duplicate the base group *also* in the supplementary groups in the returned array (so it was present in the first slot, as the effective GID to set, and *also* in the second slot). Commit a59d6a872459 ("Implementing 'fallback' nsswitch source. ...") removed that without a word. So, except for setting the effective GID, our initgroups() had been doing the same as other system for the supplementary groups for years after the project started.

Amending this revision in this light.

[olce]

Change of paradigm (see previous comment).

[kib]

Can we avoid the macros please? Provide something like

static int
initgroups_impl(const char *uname, gid_t agroup, int (*setgroupsp)(int, gid_t *))
{
}

and pass corresponding function as an argument.

[olce]

• Switch to a common function
• Slight rework of the man page

[kib]

lib/libc/gen/initgroups.c

56

Is this comment about errno stil relevant?

68

Talking about errno, do we need to save/restore it around the free() call? (I never know what a malloc implementation of the day is required to do).

70

76

[kevans]

lib/libc/gen/initgroups.c

68

free() is mandated to preserve errno as long as it's passed NULL or a valid heap pointer, iirc. CC @des

[olce]

lib/libc/gen/initgroups.c

56

Of course. We leave setting errno on failure to setgroups().

Speaking of which, the return (ENOMEM) below is wrong, going to fix it.

68

jemalloc doesn't seem to do that. While it's true that the C standard explicitly allows functions for which it does not specify a specific behavior to set errno as they please, POSIX seems much more restrictive: Functions that cannot return a value are not supposed to set errno (well, it's not exactly what they say, which rather is that you shouldn't read errno if the function called didn't return a value saying that errno is valid; setting errno would just be silly in this case, but is not explicitly forbidden, and this may be seen as a license not to care about some sub-routine setting errno; see XSH, General Information, Error Numbers for the paragraph I'm speaking about).

I think that if one day we have a free() that does that, we'll have either to audit/fix a large amount of code, or more plausibly we'll just wrap it inside a function saving and restoring errno.

Hence I don't think that's specifically necessary here. But if you insist I'll add the save/restore.

68

free() is mandated to preserve errno as long as it's passed NULL or a valid heap pointer, iirc. CC @des

I don't think that's the case, see response to kib@ above (had written it before but apparently forgot to send it).

[kevans]

lib/libc/gen/initgroups.c

68

Latest POSIX:

Austin Group Defect 385 is applied, adding a requirement that free() does not modify errno when passed a pointer to an object than can be freed.

[olce]

lib/libc/gen/initgroups.c

68

Oh, great. Then that conforts me in not saving errno around free() here.

[kevans]

lib/libc/gen/initgroups.c

68

Granted, I don't think we've audited the jemalloc onion. If it doesn't preserve it, IMO we should consider that a fix for jemalloc because that's really handy behavior to assume.

[olce]

lib/libc/gen/initgroups.c

68

I skimmed through its code this afternoon, and am pretty convinced it doesn't set errno on free().

And I completely agree that if it did, or if the next malloc we import does, we should just fix it (as said above in a response to kib@).

[olce]

• Cater to comments
• Update on top of D52580

[olce]

Right diff

[kib]

lib/libc/gen/initgroups.c

68

Not setting errno is not enough. I am sure that sometimes free() calls munmap(2) or madvise(2). Is errno preserved around these calls?

[des]

lib/libc/gen/initgroups.c

68

free() is mandated to preserve errno as long as it's passed NULL or a valid heap pointer, iirc. CC @des

Correct, but beware that this requirement is a recent addition and not all implementations conform to it. I've been told that jemalloc does, but I haven't checked.

[olce]

lib/libc/gen/initgroups.c

68

Not setting errno is not enough. I am sure that sometimes free() calls munmap(2) or madvise(2). Is errno preserved around these calls?

I've spent more than an hour discovering and browsing jemalloc's code, so this should be taken with a grain of salt, but:

1. There is nothing that prevents errno to be overwritten in all code paths leading to munmap()/madvise()/mprotect().
2. But these are never supposed to fail unless there is a bug in jemalloc, or something else messed up with jemalloc's mappings.
3. An old commit from 2012 ( 6e6164ae159d9c3b upstream) gives the impression that indeed the expectation, or perhaps more realistically, the behavior at that time was not to fiddle with errno on free(), and that FreeBSD depended on it (how precisely is not mentioned).

So, in practice, errno should never be overwritten on free(). Should...

[kib]

lib/libc/gen/initgroups.c

68

Indeed the use of syscalls might assume that they never fail, but I do not think this is a good assumption.

Also, I mentioned that free() might be not the jemalloc free() - users can legitimately LD_PRELOAD another implementation, or link with it. We do support interposing malloc.

So overall, assuming the sane behavior from free() looks too optimistic for me.

[olce]

lib/libc/gen/initgroups.c

68

Maybe, but then what do we do?

Reviewing all uses of free() and making sure to save/restore errno around it seems error-prone and perhaps more importantly may just be a waste of time in a world where we have made this assumption for a long time (to which extent, I don't know) and where POSIX now is sanctifying that behavior.

I have a trivial patch for je_free() saving and restoring errno, but that is (obviously) for jemalloc only.

In which ways do we allow "interposing" another malloc? By just LD_PRELOAD, or some other means? It would indeed be problematic if users substitute jemalloc with an allocator where free() can crush errno. Is is unreasonable to simply require that behavior for free()? Does some mechanism exist that allows to still intercept a free() even on LD_PRELOAD, in which case we could have a simple wrapper saving and restoring errno around any free() implementation?

[des]

lib/libc/gen/initgroups.c

68

I have a trivial patch for je_free() saving and restoring errno, but that is (obviously) for jemalloc only.

Since errno is a thread-local variable, actually doing this without pessimizing the common case is far from trivial. Making free() even slightly slower across the board adds up fast. If you want to do this properly, you'll have to identify and patch, as narrowly as possible, only those code paths that actually risk modifying errno.

[kib]

lib/libc/gen/initgroups.c

68

Maybe, but then what do we do?

Reviewing all uses of free() and making sure to save/restore errno around it seems error-prone and perhaps more importantly may just be a waste of time in a world where we have made this assumption for a long time (to which extent, I don't know) and where POSIX now is sanctifying that behavior.

When I added the first comment, I intended that author add errno preservation into the initgroups_impl() code around the call to free(), and be done with it.

I have a trivial patch for je_free() saving and restoring errno, but that is (obviously) for jemalloc only.

In which ways do we allow "interposing" another malloc? By just LD_PRELOAD, or some other means? It would indeed be problematic if users substitute jemalloc with an allocator where free() can crush errno.

E.g. by linking with an alternative malloc implementation explicitly, butting the dso with the malloc/free/etc symbols before -lc in the link (actually DT_NEEDED) order.

Is is unreasonable to simply require that behavior for free()? Does some mechanism exist that allows to still intercept a free() even on LD_PRELOAD, in which case we could have a simple wrapper saving and restoring errno around any free() implementation?

I think that there are not too many places where free() vs. errno matters, at least in libc. IMO it is better to be protective and add errno preservation around calls where it matters as the calls are discovered.

[olce]

lib/libc/gen/initgroups.c

68

When I added the first comment, I intended that author add errno preservation into the initgroups_impl() code around the call to free(), and be done with it.

I understood it as that, but just wanted to explore the feasibility of something more generic first.

E.g. by linking with an alternative malloc implementation explicitly, butting the dso with the malloc/free/etc symbols before -lc in the link (actually DT_NEEDED) order.

Ok, this is what I assumed. I'm not aware of any mechanism adding back some wrapper or some pre-/post- conditions to overridden symbols in the linker or rtld, so interception on symbol override in general seems impossible.

I think that there are not too many places where free() vs. errno matters, at least in libc. IMO it is better to be protective and add errno preservation around calls where it matters as the calls are discovered.

Ok, then chasing them is the way to go (in the absence of any generic mechanism to protect us from allocators, be it our jemalloc or third-party ones, not obeying this rule).

68

Since errno is a thread-local variable, actually doing this without pessimizing the common case is far from trivial. Making free() even slightly slower across the board adds up fast.

True.

If you want to do this properly, you'll have to identify and patch, as narrowly as possible, only those code paths that actually risk modifying errno.

Not necessarily.

jemalloc may be helping us here, as je_free() is basically just:

if (!free_fastpath(ptr, 0, false)) {
        free_default(ptr);
}

and it may be that we can just bracket free_default() with saving/restoring errno without incurring more performance penalty then patching exactly the paths calling our system calls.

Actually, saving/restoring errno as narrowly as possible may in fact be a performance pessimization when our system calls in the end are called multiple times, as this bracketing would occur multiple times wastefully.

This is indeed not a trivial project, it requires a thorough analysis and some benchmarking. I don't intend to undertake it in the short term.

[kib]

lib/libc/gen/initgroups.c

68

E.g. by linking with an alternative malloc implementation explicitly, butting the dso with the malloc/free/etc symbols before -lc in the link (actually DT_NEEDED) order.

Ok, this is what I assumed. I'm not aware of any mechanism adding back some wrapper or some pre-/post- conditions to overridden symbols in the linker or rtld, so interception on symbol override in general seems impossible.

There are e.g. --wrap option for ld.bfd, also apparently supported by ld.lld. But this is more a user option than something that would be applicable to build of the system libraries.

I think that there are not too many places where free() vs. errno matters, at least in libc. IMO it is better to be protective and add errno preservation around calls where it matters as the calls are discovered.

Ok, then chasing them is the way to go (in the absence of any generic mechanism to protect us from allocators, be it our jemalloc or third-party ones, not obeying this rule).

Besides wrapping the free() call with errno saving, you could switch to alloca(). I do not understand why people demonize it.