When pondering whether we should start linking to some external library in systemd, I usually spend some time looking at the library's sources, to understand the quality of the code. While coding style differences are fine, there are certain red flags that make libraries unsuitable for use in systemd, or that indicate questionable quality of the code.
Red flags like this are for example absence of OOM guards on malloc(), absence of reasonable error propagation,
placement of too much state in global variables or TLS (instead of maintaining clean context objects), obvious TTOCTTOU races around file use, open coded structures, absence of symbol prefixing and so on.
But there's one red flag that I want to write about today, that's just so annoying, because it's not widely understood to be the devil's work: use of ELF constructurs/destructors.
In ELF/gcc you can mark functions as ELF constructurs or destructors. Functions marked like that…
… will be automatically called whenever the shared library they are placed in is loaded into a process (which means at process startup, before main(), or at dlopen() time) or when they are unloaded from a process (which means at exit() time, or dlclose() time).
At first, from the outside the concept seems kinda OK: it has this C++'ish RAII feeling to it to: whenever a library is initialized in can initialize some fields, and whenever deinitializes it can release them again.
But of course (I guess like various C++ feaures), it's also terrible if people actually start using this IRL. In particular in systemd this is has brought a repeating theme of breakage to us.
The systemd service manager (aka "PID 1") runs very early at boot, it is after all typically the first userspace process invoked on a system. This means it initially runs in a very minimal execution context, for example /proc/ and /sys/ are typically not mounted yet, there's no /tmp/, and nothing else.
When systemd's main() function is started it then goes through a careful process of setting things up, i.e. establishing mounts, making things writable, loading security policies, initializing certain devices, RNGs, and so on, loading configuration and many many other things. This is carefully scheduled to be done in the right order, since after all we cannot assume that things are already set up for us, we have to do the setup *ourselves* after all.
But of course ELF constructors fuck that all up, because they get started before main() does its first step even, after all. It takes any control away we have in scheduling precisely when to do what.
And people do terrible things in ELF constructors: load configuration, check if kernel APIs are available (which typically means looking at /proc/, /sys/ or some other API VFS), pre-opening some fds, and so on. All these are things you really don't want to do before things are properly mounted, …
…, labelled, initialized and so on. One prominent library which we do link against that used to do horrible shit like that, is libselinux btw. They fixed much of it, but still use ELF constructors/destructors these days, and they really shouldn't.
Now you might say that not all projects are systemd, that we are a special case, but there are many other problems with it:
There's no ordering defined in which constructors/destructors are called. Or at least not a useful one: it's "topoligical".
Which means that constructors are invoked in the order the shared libraries are loaded in, and if shared libraries have dependencies, they are first loaded "down the tree". But that sucks hard, because libraries tend to have interdependencies, non obvious ones at that, and cyclic ones too! And that means you might end up calling functions from libs whose constructors haven't run yet, or whose destructors already ran.
Then, various libraries (including systemd's) use "-z nodelete", …
… or are loaded via RTLD_NODELETE (for various reasons). The effect of that: the ELF destructors they provide will never be called, and what's worse, the destructors of any depending lib will never be called either, ever).
What's worse: it's not clear which thread invokes the constructors/destructors even. It might be the main thread, or some other thread, because dlopen() is used from it, directly or indirectly. And other code might run in parallel with your constructor/destructor hence.
Relevant C projects nowadays tend to have CI systems that run everything in their codebase through valgrind and memory sanitizers provided by the various compiler suites. This is an additional headache when ELF destructors are used: whether memory is leaked or not cannot be determined correctly from inside a process if you basically have to exit first to get the resources are actually released by the ELF destructor.
And then, various projects tend to open/close fds in their ELF…
…constructors. But that sucks hard, because in so many cases system level C code must be able to ensure that all fds are closed (for example, in systemd we require this when we load MAC policy, so that all fds are definitely labelled properly, and for many other reasons) at some time, but if "hidden" fds are kept open, that cannot be reasonably closed unless a library is unloaded then things are just unworkable.
Anyway, there are other reasons ELF constructors/destructors are shit, …
… but let's stop this for now (there's more, really, for example sandboxing, and ELF const/dest really make an awful combo; or blocking syscalls in them will make dlopen() hang, or fricking locking in them is a desaster), and summarize:
ELF constructors/destructors run at the worst times, are not reasonably schedulable, run from arbitrary contexts, in abitrary threads, too early, and too late, maybe not at all, and not intstrumentable reasonably for analyzers. Hence, fuck them.
But enough of the expletives, let's talk a bit of what to do instead?
I think good C library design is always built around some kind of opaque context object (possibly multiple), that all relevant functions provided by the library get passed in as first argument. This context object should be allocated by one library function and destroyed by another, but key really is that the library is "dormant" until that constructor is explicitly called by its user, and "dormant" again…
… when the destroy function for the relevant contexts is called again.
And that's really all I'd ask for. This is great, because it means consuming code can carefully schedule when and in which context it wants to start making use of the library, and thus when it shall be initialized, and when it doesn't need it anymore, and when it shall be deinitialized again.
@pid_eins How do all these people (unit) test their libraries without a context object?
@GabrielKerneis they probably just don't.
libselinux is a really bad offender not just regarding this red flag. I guess the reason they get away with it, is because they come from a time where CIs/test suites were not a thing yet, and they are so low level in our dep tree, that everyone just accepts their fate and ignores the red flags.
@jmorris @GabrielKerneis I guess libselinux could find ways to avoid ELF constructros/destructors. For example initialize what they want to initialize lazily (i.e. when the first function that needs it is called), and then provide explicit functions that initialize/destroy what they want to destroy at the end. For systemd's use that would be enough.
I mean, i generally think libraries that have a lot of global state are a bit icky, but i guess that ship has sailed for libselinux…