While a delegated scope waits for the async cgroup chown helper it sits
in SCOPE_START_CHOWN (UNIT_ACTIVATING). unit_start() forwards to
->start() while activating, so scope_start() could be re-entered in
that state and trip assert(s->state == SCOPE_DEAD), aborting PID 1.
Treat SCOPE_START_CHOWN as already-starting instead.
Follow-up for 03860190fe
get_user_creds would use getpwnam() and then returns strings pointing
into the static buffer. This seems very iffy. Let's duplicate the
strings properly
The param was input/output, which is unexpected and confusing and
actually made most callers much more complicated than they needed to be.
The function was playing fast and loose with the return value. In some
cases it was returning a static string, which would be completely fine.
But in other case it was returning a pointer into the getgrgid static
buffer, i.e. that return value could be overwritten. AFAICT, this didn't
matter in any of the callers, but we shouldn't do that anyway.
So use a separate output param with an allocated string that the caller
is responsible for.
It turns out that all callers pass NULL (outside of tests) and zero for
flags. So the function _could_ be simplified. But get_user_creds is
called with all the params actually used, and I think having the the
functions so different wouldn't be nice. I first wrote a commit to
drop the unused params, but then I discarded it.
The error message in exec-invoke.c is fixed.
As per https://github.com/systemd/systemd/pull/41986#pullrequestreview-4281939586
This also corrects the list of unit types a bit:
1. this removes the mount/automount unit type from the list, since for these types
we do not allow aliases/renaming anyway.
2. this adds socket + swap units to the list, since they can change
name, and for both of them we actually do fork off processes hence
track resources.
Follow-up for: #41986
$ sudo build/systemd-run --uid=asdf whoami
$ journalctl -e
(whoami)[1007784]: run-p1007782-i5200512.service: Failed to determine user credentials: No such process
(whoami)[1007784]: run-p1007782-i5200512.service: Failed at step USER spawning /usr/sbin/whoami: No such process
systemd[1]: run-p1007782-i5200512.service: Main process exited, code=exited, status=217/USER
systemd[1]: run-p1007782-i5200512.service: Failed with result 'exit-code'.
Now:
(whoami)[1013204]: run-p1013202-i5205932.service: Failed to determine credentials for user 'asdf': Unknown user
(whoami)[1013204]: run-p1013202-i5205932.service: Failed at step USER spawning /usr/sbin/whoami: Invalid argument
systemd[1]: run-p1013202-i5205932.service: Main process exited, code=exited, status=217/USER
systemd[1]: run-p1013202-i5205932.service: Failed with result 'exit-code'.
Follow-up for 495e75ed5c
The mentioned commit switched scope unit's "pids" deserialization
to call unit_watch_pid() already, meaning all later invocations
in scope_coldplug() are no-op. Remove the cruft altogether.
This allows some strings constants to be deduplicated.
libsystemd-core.so + systemd are about 2k smaller.
Also tweak some messages for consistency. We used 'exec' in one place and
'execute' in all other messages.
Let's move some more implementation logic into functions. We keep
the logic that requires the macro in the macro and move the rest into
functions.
While we're at it, let's also make the parameter declarations of
all the string table macros less clausthrophobic.
Currently there are various circular dependencies between headers
in core/. Let's get rid of these by making judicious use of forward
declarations and moving includes into implementation files instead of
having them in header files.
Getting rid of circular header includes simplifies the code and makes
various clang based tooling such as iwyu work much better on our code.
The most important change is getting rid of the manager.h include in
unit.h which is possible thanks to the previous commits. We also move
the OOMPolicy and StatusType enums to unit.h to remove the need for
other unit headers to include manager.h to get access to these enums.
These operations might require slow I/O, and thus might block PID1's main
loop for an undeterminated amount of time. Instead of performing them
inline, fork a worker process and stash away the D-Bus message, and reply
once we get a SIGCHILD indicating they have completed. That way we don't
break compatibility and callers can continue to rely on the fact that when
they get the method reply the operation either succeeded or failed.
To keep backward compatibility, unlike reload control processes, these
are ran inside init.scope and not the target cgroup. Unlike ExecReload,
this is under our control and is not defined by the unit. This is necessary
because previously the operation also wasn't ran from the target cgroup,
so suddenly forking a copy-on-write copy of pid1 into the target cgroup
will make memory usage spike, and if there is a MemoryMax= or MemoryHigh=
set and the cgroup is already close to the limit, it will cause an OOM
kill, where previously it would have worked fine.
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.
This refactors the Unit structure a bit: all cgroup-related state fields
are moved to a new structure CGroupRuntime, which is only allocated as
we realize a cgroup.
This is both a nice cleanup and should make unit structures considerably
smaller that have no cgroup associated, because never realized or
because they belong to a unit type that doesn#t have cgroups anyway.
This makes things nicely symmetric:
ExecContext → static user configuration about execution
ExecRuntime → dynamic user state of execution
CGroupContext → static user configuration about cgroups
CGroupRuntime → dynamic user state of cgroups
And each time the XyzContext is part of the unit type structures such as
Service or Slice that need it, but the runtime object is only allocated
when a unit is started.
This commit overhauls the way freeze/thaw works recursively:
First, it introduces new FreezerActions that are like the existing
FREEZE and THAW but indicate that the action was initiated by a parent
unit. We also refactored the code to pass these FreezerActions through
the whole call stack so that we can make use of them. FreezerState was
extended similarly, to be able to differentiate between a unit that's
frozen manually and a unit that's frozen because a parent is frozen.
Next, slices were changed to check recursively that all their child
units can be frozen before it attempts to freeze them. This is different
from the previous behavior, that would just check if the unit's type
supported freezing at all. This cleans up the code, and also ensures
that the behavior of slices corresponds to the unit's actual ability
to be frozen
Next, we make it so that if you FREEZE a slice, it'll PARENT_FREEZE
all of its children. Similarly, if you THAW a slice it will PARENT_THAW
its children.
Finally, we use the new states available to us to refactor the code
that actually does the cgroup freezing. The code now looks at the unit's
existing freezer state and the action being requested, and decides what
next state is most appropriate. Then it puts the unit in that state.
For instance, a RUNNING unit with a request to PARENT_FREEZE will
put the unit into the PARENT_FREEZING state. As another example, a
FROZEN unit who's parent is also FROZEN will transition to
PARENT_FROZEN in response to a request to THAW.
Fixes https://github.com/systemd/systemd/issues/30640
Fixes https://github.com/systemd/systemd/issues/15850
This makes sure unit_watch_pid() and unit_unwatch_pid() will track
processes by pidfd if supported. Also ports over some related code.
Should not really change behaviour.
Note that this does *not* add support waiting for POLLIN on the pidfds
as additional exit notification. This is left for a later commit (this
commit is already large enough), in particular as that would add new
logic and not just convert existing logic.
Let's not stop watching processes on every scope state change. This
corrects behaviour when a scope is being enqueued and hasn't started
yet, but has processes associated to it already. previously, if we'd doa
full PID 1 reload we'd stop watching those pids. With this change we'll
continue watching them in that case, and only stop watching them when
the scope unit really shuts down after first being up.
This ports over unit_main_pid() + unit_control_pid() to return PidRef*
pointers (which also means the underlying UnitVTable function pointers
are changed accordingly).
This then uses te functions to simplify the unit_kill() call, by
avoiding the kill() vtable indirection and instead just suing
unit_main_pid() and unit_control_pid() directly.
This adds a new structure UnitDefaults which embedds the various default
settings for units we maintain. We so far maintained two sets of
variables for this, one in main.c as static variables and one in the
Manager structure. This moves them into a common structure.
This is most just search/replace, i.e. very dumb refactoring.
The fact that we now use a common structure for this allows us further
refactorings later.
Inspired by the discussions on #27890
This is supposed to be a help for compilers to apply optimizations on
functions where they can't determine whether they are const/pure on
their own. For static, local functions the compiler can do this on its
own easily however, hence the decoration with pure/const is just noise.
Let's drop it, and let the compiler to its thing better.
(Use it for exported functions, since compilers can't 'reach-over' into
other modules to determine if they are pure, except if LTO is used)
As it says on the tin, configures the unit to survive a soft reboot.
Currently all the following options have to be set by hand:
Conflicts=reboot.target kexec.target poweroff.target halt.target
Before=reboot.target kexec.target poweroff.target halt.target
After=sysinit.target basic.target
DefaultDependencies=no
IgnoreOnIsolate=yes
This is not very user friendly. If new default dependencies are added,
or new shutdown/reboot types, they also have to be added manually.
The new option is much simpler, easy to find, and does the right thing
by default.
In scope_set_state(), the timer event source may be disabled depending
on the state. Currently, it will be disabled when the state is
SCOPE_RUNNING. This has the effect of new RuntimeMaxSec values being
ignored on coldplug.
Note that this issue is not currently present when scopes are started
because when scope_start() is called, scope_arm_timer() is called after
scope_set_state().
This function was moved from cgroup-util.c to cgroup-setup.c a while
back, but the prototype in the matching header files wasn't migrated.
Let's fix that.
This augments the existing KillUnit() + Kill() methods with
QueueSignalUnit() + QueueSignal(), which are what sigqueue() is to
kill().
This is useful for sending our new SIGRTMIN+18 control signals to system
services.
If we add a drop-in for init.scope (e.g.: to set some memory limit),
it will be loaded long after the cgroup has already been realized.
Do it again when creating the special unit.
Previously it was possible to set delegate property for scope, but you
were not able to allow unprivileged process to manage the scope's cgroup
hierarchy. This is useful when launching manager process that will run
unprivileged but is supposed to manage its own (scope) sub-hierarchy.
Fixes#21683
Luca Boccassi
Zbigniew Jędrzejewski-Szmek
Lennart Poettering
Michael Vogt
Yu Watanabe
Zbigniew Jędrzejewski-Szmek
Mike Yuan
Daan De Meyer
Luca Boccassi
Lennart Poettering
Adrian Vovk
Richard Phibel
Nick Rosbrook
Mark Laws
gtwang01
Michal Sekletar