If we try to deserialize only a pidfd that points to a process that
has been reaped, creating the pidref object will fail, which means that
we'll try to create a pidref object from the serialized pid that comes
next. If the pid has already been reused, this will succeed and we'll
now have a pidref that points to a different process.
Let's avoid this issue by serializing both the pidfd and the pid and
creating the pidref object directly from both. This means we'll reuse
the deserialized pidfd instead of opening a new one. We'll then immediately
notice the pidfd is dead and do the appropriate follow up depending on
the unit type.
Currently, when downgrading from a version with pidfd support to a
version without pidfd support, all information about running processes
is lost as the newer systemd will serialized pidfds which are not recognized
by the older systemd when deserializing.
To improve the situation, let's serialize both the pid and the pidfd.
This is safe because existing versions will either replace the first
deserialized pidref with the second one or discard the second one in
favor of the first one depending on the unit and field. Older versions
that don't support pidfd's will silently discard any fields that contain
a pidfd as those will try to parse the field as a pid and since a pidfd
field will start with '@', those versions will debug error log and ignore
the value.
To make sure we reuse the existing pidfd as much as possible, the pidfd
is serialized first. Both for scopes and service main pids, if the same
pid is seen multiple times, the first pidref is kept. So by serializing
the pidfd first we make sure the original pidfd is used instead of the
new one which is opened when deserializing the first pid field.
For other control units, older versions with pidfd support will discard
the first pidfd and replace it with a new pidfd from the second pid field.
This is a slight regression on downgrades, but we make sure it doesn't
happen for future versions (and older versions when this commit is
backported) by modifying the logic to only use the first successfully
deserialized pidref so that the raw pid without pidfd is discarded instead
of it replacing the existing pidfd.
Currently, when we deserialize an fd we do a lot of manual work. Add a
common helper that makes this more robust and uniform.
Note that this sometimes changes behaviour slightly, but in ways that
shouldn't really matter: if we fail to deserialize an fd correctly we'll
unset (i.e. set to -EBADF) the fd in the deserialized data structure.
Previously, we'd leave the old value in place.
This should not change effective result (as in either case we'll be in a
bad state afterwards, just once we mix old/invalidated state with new
state, while now we'll reset the state explicitly to invalidated state
on failure). In particular as deserialization starts from an empty
structure generally, hence the old value should be unset anyway.
Another slight change is that if we fail to deserialize some object half
way, and we already have taken out one fd from the serialized fdset
we'll now just close it instead of returning it to/leaving it in the
fdset. Given that such "orphaned" fds are blanket closed after
deserialization finishes this also shouldn't change behaviour IRL.
Also, the idle_pipe was previously incorrectly serialized: we'd
serialize invalidated fds, which would fail, but because parsing errors
on this were ignored on the deserializatin noone noticed. This is fixed.
Use the flags MEMFD_EXEC or MEMFD_NOEXEC_SEAL as applicable.
These warnings instruct the kernel wether the memfd is executable or
not.
Without specifying those flags the kernel will emit the following
warning since version 6.3,
commit 105ff5339f49 ("mm/memfd: add MFD_NOEXEC_SEAL and MFD_EXEC"):
kernel: memfd_create() without MFD_EXEC nor MFD_NOEXEC_SEAL, pid=1 'systemd'
All callers of strv_env_replace() would free the argument on error.
So let's follow the same pattern as with strv_consume (and similar
naming) and unconditionally "use up" the argument.
Ideally, coccinelle would strip unnecessary braces too. But I do not see any
option in coccinelle for this, so instead, I edited the patch text using
search&replace to remove the braces. Unfortunately this is not fully automatic,
in particular it didn't deal well with if-else-if-else blocks and ifdefs, so
there is an increased likelikehood be some bugs in such spots.
I also removed part of the patch that coccinelle generated for udev, where we
returns -1 for failure. This should be fixed independently.
All users of the macro (except for one, in serialize.c), use the macro in
connection with read_line(), so they must include fileio.h. Let's not play
libc games and require multiple header file to be included for the most common
use of a function.
The removal of def.h includes is not exact. I mostly went over the commits that
switch over to use read_line() and add def.h at the same time and reverted the
addition of def.h in those files.
Let's be more careful with what we serialize: let's ensure we never
serialize strings that are longer than LONG_LINE_MAX, so that we know we
can read them back with read_line(…, LONG_LINE_MAX, …) safely.
In order to implement this all serialization functions are move to
serialize.[ch], and internally will do line size checks. We'd rather
skip a serialization line (with a loud warning) than write an overly
long line out. Of course, this is just a second level protection, after
all the data we serialize shouldn't be this long in the first place.
While we are at it also clean up logging: while serializing make sure to
always log about errors immediately. Also, (void)ify all calls we don't
expect errors in (or catch errors as part of the general
fflush_and_check() at the end.
Daan De Meyer
Frantisek Sumsal
Zbigniew Jędrzejewski-Szmek
Lennart Poettering
Luca Boccassi
Yu Watanabe
Thomas Weißschuh