PTRACE_POKETEXT and PTRACE_POKEDATA use FOLL_FORCE to forcibly write
into a tracee's memory. That includes executable read-only pages. If we
restrict execution to trusted binaries PID 1 must enforce that it's
impossible to abuse this to get untrusted code execution. Note, that
usermodehelpers are not subject to PID 1's seccomp filters.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Since 4c0d8d9673, most dlopen notes are
set at the beginning of the executables. Let's manage all dlopen notes
there, rather than setting them where dlopen is called.
Note that the only exceptions are the LIBBPF_NOTE for networkd and
nsresource. Since dlopen_bpf() is wrapped in an `#if` guard, the notes
are instead set within the corresponding functionality.
As a result, the DLOPEN_FOO() wrapper macros are no longer needed and
can be dropped completely.
When compiler optimization or LTO (Link Time Optimization) is enabled,
dlopen helper functions (such as dlopen_libfoo()) can be aggressively
inlined into their callers or cloned for specific call sites.
While this is beneficial for production builds, it alters or completely
removes the original function symbols from the final executable.
Consequently, this breaks the test (to be added in this series) that
checks whether the dlopen_libfoo() function corresponding to a dlopen
ELF note is actually called.
To resolve this, introduce the `_dlopen_loader_` macro. Under developer
builds, it applies the `_noclone_` and `_noinline_` attributes to guarantee
that helper symbols remain intact and discoverable by the test suite. In
production builds, the macro evaluates to nothing, allowing full compiler
optimization to proceed unimpeded.
This also switches all these notes to be defined via SD_ELF_NOTE_DLOPEN_ANCHORED().
Consequently, any notes added within unreachable or unused functions will be
automatically garbage-collected by the linker (--gc-sections) instead of
bloating the final binary.
E.g. unused p11-kit library dependency is now dropped from
systemd-repart.standalone binary.
Before:
```
$ systemd-analyze dlopen-metadata build/systemd-repart.standalone
FEATURE DESCRIPTION SONAME PRIORITY
cryptsetup Support for disk encryption, integrity, and authentication libcryptsetup.so.12 recommended
blkid Support for block device identification libblkid.so.1 required
libcrypto Support for cryptographic operations libcrypto.so.4 libcrypto.so.3 recommended
mount Support for mount enumeration libmount.so.1 required
fdisk Support for reading and writing partition tables libfdisk.so.1 required
blkid Support for block device identification libblkid.so.1 recommended
libcrypto Support for cryptographic operations libcrypto.so.4 libcrypto.so.3 suggested
cryptsetup Support for disk encryption, integrity, and authentication libcryptsetup.so.12 suggested
fdisk Support for reading and writing partition tables libfdisk.so.1 suggested
idn Support for internationalized domain names libidn2.so.0 suggested
mount Support for mount enumeration libmount.so.1 recommended
selinux Support for SELinux libselinux.so.1 recommended
tpm Support for TPM libtss2-esys.so.0 suggested
tpm Support for TPM libtss2-rc.so.0 suggested
tpm Support for TPM libtss2-mu.so.0 suggested
tpm Support for TPM libtss2-tcti-device.so.0 suggested
p11-kit Support for PKCS11 hardware tokens libp11-kit.so.0 suggested
```
After:
```
$ systemd-analyze dlopen-metadata build/systemd-repart.standalone
FEATURE DESCRIPTION SONAME PRIORITY
cryptsetup Support for disk encryption, integrity, and authentication libcryptsetup.so.12 recommended
blkid Support for block device identification libblkid.so.1 required
libcrypto Support for cryptographic operations libcrypto.so.4 libcrypto.so.3 recommended
mount Support for mount enumeration libmount.so.1 required
fdisk Support for reading and writing partition tables libfdisk.so.1 required
blkid Support for block device identification libblkid.so.1 recommended
libcrypto Support for cryptographic operations libcrypto.so.4 libcrypto.so.3 suggested
cryptsetup Support for disk encryption, integrity, and authentication libcryptsetup.so.12 suggested
fdisk Support for reading and writing partition tables libfdisk.so.1 suggested
mount Support for mount enumeration libmount.so.1 recommended
selinux Support for SELinux libselinux.so.1 recommended
tpm Support for TPM libtss2-esys.so.0 suggested
tpm Support for TPM libtss2-rc.so.0 suggested
tpm Support for TPM libtss2-mu.so.0 suggested
tpm Support for TPM libtss2-tcti-device.so.0 suggested
```
This makes each `foo_cflags` dependency define a `SYSTEMD_CFLAGS_MARKER_FOO`
macro, and checks if the macro is set when headers provided by external
libraries are included.
With this, we can fail fast at compile time if necessary `_cflags`
dependencies are omitted in meson.build. Missing dependencies found by this
mechanism have been added across the tree.
This macro emits an SD_ELF_NOTE_DLOPEN note so that calling sites,
and not just libsystemd-shared, get the ELF stamped with the notes
documenting the dependency. Currently most of the dlopen notes end
up in libsystemd-shared, and executables have no way to document
the optional dependencies they need. With this new macro, this
issue can be fixed.
Co-developed-by: Claude Opus 4.8 <noreply@anthropic.com>
Every dlopen_xxx() helper now takes an int log_level argument. It is
passed through to dlopen_many_sym_or_warn() (which in turn propagates it
to dlopen_verbose() for the library-not-installed case), and is used by
the fallback stub when support for the library is not compiled in to
emit a "<lib> support is not compiled in." message at the caller's level.
Callers pass LOG_DEBUG when gracefully degrading, or a higher level when
the failure should surface, and no longer need to log redundantly at the
call site.
As part of this, dlopen_bpf_full() (which already took a log_level) is
merged into dlopen_bpf() rather than keeping both.
The static inline fallbacks used to live in the headers, which required
pulling log.h in from every header that declared a dlopen_xxx(). Move
them into the .c files instead: the declaration is always outside the
#if HAVE_XXX block, the impl sits outside the outer #if HAVE_XXX wrap
with its own internal #if HAVE_XXX/#else/#endif, and apparmor-util.c,
idn-util.c, libmount-util.c and pam-util.c are now always compiled so
they can host their stubs.
Major distributions already have libseccomp 2.5.x or newer.
Let's bump to the required minimum version to 2.4.0, which provides
SCMP_ACT_KILL_PROCESS, SCMP_ACT_LOG, SCMP_ARCH_PARISC, and
SCMP_ARCH_PARISC64.
Note, libseccomp 2.4.0 was released on 2019-03-15.
See also #38608.
Let's introduce these for the following two reasons:
- Using them makes sure the parameter names match between declaration
and definition (if both use the corresponding macros).
- They make sure developers can't forget the _const_ and _pure_
attributes for the declarations.
This commit also includes some include sorting fixes
Let's not leak details from src/shared and src/libsystemd into
src/basic, even though you can't actually do anything useful with
just forward declarations from src/shared.
The sd-forward.h header is put in src/libsystemd/sd-common as we
don't have a directory for shared internal headers for libsystemd
yet.
Let's also rename forward.h to basic-forward.h to keep things
self-explanatory.
Let's remove some HAVE_SECCOMP ifdeffery by simply defining the funcion
in question (seccomp_parse_errno_or_action() + related calls) into
common code that is also compiled if HAVE_SECCOMP is off.
This is generally the better approach anyway, since we want as much as
possible and easily feasible parsers work even if the code implementing
them is disabled. THis is easy to achieve here, hence do.
So far we asked via a comment that @default should stay the first group
and @known the last group in the list. Let's enforce that statically, in
code, too.
Let's group these 4 syscalls, as they offer similar things and I guess
might be used in conjunction quite often, as they offer unprivileged
sandboxing.
Fixes: #26913
- execute.c: bpf functions were in the middle of an #if HAVE_SECCOMP
block for no reason
- test-fd-util.c: make seccomp-util.h includable without depending on
<seccomp.h>, and make is_seccomp_available() hardcoded to returning
false in this case.
Also fix a stray DEFINED() -- HAVE_SECCOMP is defined as 0, so normal
#if should be used like everywhere else.
This is supposed to be used by package/image builders such as mkosi to
speed up building, since it allows us to suppress sync() inside a
container.
This does what Debian's eatmydata tool does, but for a container, and
via seccomp (instead of LD_PRELOAD).
If the cleanup function returns the appropriate type, use that to reset the
variable. For other functions (usually the foreign ones which return void), add
an explicit value to reset to.
This causes a bit of code churn, but I think it might be worth it. In a
following patch static destructors will be called from a fuzzer, and this
change allows them to be called multiple times. But I think such a change might
help with detecting unitialized code reuse too. We hit various bugs like this,
and things are more obvious when a pointer has been set to NULL.
I was worried whether this change increases text size, but it doesn't seem to:
-Dbuildtype=debug:
before "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 4494520 Feb 16 15:06 build/systemd*
after "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 4494576 Feb 16 15:10 build/systemd*
now:
-rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 4494640 Feb 16 15:15 build/systemd*
-Dbuildtype=release:
before "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:09 build-rawhide/systemd*
after "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:10 build-rawhide/systemd*
now:
-rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:16 build-rawhide/systemd*
I would expect that the compiler would be able to elide the setting of a
variable if the variable is never used again. And this seems to be the case:
in optimized builds there is no change in size whatsoever. And the change in
size in unoptimized build is negligible.
Something strange is happening with size of libsystemd: it's bigger in
optimized builds. Something to figure out, but unrelated to this patch.
When seccomp_restrict_archs is called, architectures that are blocked
are replaced by the SECCOMP_LOCAL_ARCH_BLOCKED marker so that they are
not disabled again and filters are not installed for them.
This can make some service that use SystemCallArchitecture= and
SystemCallFilter= start faster.
Define explicit action "kill" for SystemCallErrorNumber=.
In addition to errno code, allow specifying "kill" as action for
SystemCallFilter=.
---
v7: seccomp_parse_errno_or_action() returns -EINVAL if !HAVE_SECCOMP
v6: use streq_ptr(), let errno_to_name() handle bad values, kill processes,
init syscall_errno
v5: actually use seccomp_errno_or_action_to_string(), don't fail bus unit
parsing without seccomp
v4: fix build without seccomp
v3: drop log action
v2: action -> number
https://tools.ietf.org/html/draft-knodel-terminology-02https://lwn.net/Articles/823224/
This gets rid of most but not occasions of these loaded terms:
1. scsi_id and friends are something that is supposed to be removed from
our tree (see #7594)
2. The test suite defines an API used by the ubuntu CI. We can remove
this too later, but this needs to be done in sync with the ubuntu CI.
3. In some cases the terms are part of APIs we call or where we expose
concepts the kernel names the way it names them. (In particular all
remaining uses of the word "slave" in our codebase are like this,
it's used by the POSIX PTY layer, by the network subsystem, the mount
API and the block device subsystem). Getting rid of the term in these
contexts would mean doing some major fixes of the kernel ABI first.
Regarding the replacements: when whitelist/blacklist is used as noun we
replace with with allow list/deny list, and when used as verb with
allow-list/deny-list.
Patch contains a coccinelle script, but it only works in some cases. Many
parts were converted by hand.
Note: I did not fix errors in return value handing. This will be done separate
to keep the patch comprehensible. No functional change is intended in this
patch.
We noticed in our tests that occasionally SystemCallFilter= would
fail to set and the service would run with no syscall filtering.
Most of the time the same tests would apply the filter and fail
the service as expected. While it's not totally clear why this happens,
we noticed seccomp_load() in the systemd code base would fail open for
all errors except EPERM and EACCES.
ENOMEM, EINVAL, and EFAULT seem like reasonable values to add to the
error set based on what I gather from libseccomp code and man pages:
-ENOMEM: out of memory, failed to allocate space for a libseccomp structure, or would exceed a defined constant
-EINVAL: kernel isn't configured to support the operations, args are invalid (to seccomp_load(), seccomp(), or prctl())
-EFAULT: addresses passed as args are invalid
Building on previous commit, let's pass the unit name when parsing
dbus message or builtin whitelist, which is better than nothing.
seccomp_parse_syscall_filter() is not needed anymore, so it is removed,
and seccomp_parse_syscall_filter_full() is renamed to take its place.
Let services use a private UTS namespace. In addition, a seccomp filter is
installed on set{host,domain}name and a ro bind mounts on
/proc/sys/kernel/{host,domain}name.
Our logs are full of:
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldstat() / -10037, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call get_thread_area() / -10076, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call set_thread_area() / -10079, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldfstat() / -10034, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldolduname() / -10036, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldlstat() / -10035, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call waitpid() / -10073, ignoring: Numerical argument out of domain
...
This is pointless and makes debug logs hard to read. Let's keep the logs
in test code, but disable it in nspawn and pid1. This is done through a function
parameter because those functions operate recursively and it's not possible to
make the caller to log meaningfully.
There should be no functional change, except the skipped debug logs.
Currently we employ mostly system call blacklisting for our system
services. Let's add a new system call filter group @system-service that
helps turning this around into a whitelist by default.
The new group is very similar to nspawn's default filter list, but in
some ways more restricted (as sethostname() and suchlike shouldn't be
available to most system services just like that) and in others more
relaxed (for example @keyring is blocked in nspawn since it's not
properly virtualized yet in the kernel, but is fine for regular system
services).
These lines are generally out-of-date, incomplete and unnecessary. With
SPDX and git repository much more accurate and fine grained information
about licensing and authorship is available, hence let's drop the
per-file copyright notice. Of course, removing copyright lines of others
is problematic, hence this commit only removes my own lines and leaves
all others untouched. It might be nicer if sooner or later those could
go away too, making git the only and accurate source of authorship
information.
This part of the copyright blurb stems from the GPL use recommendations:
https://www.gnu.org/licenses/gpl-howto.en.html
The concept appears to originate in times where version control was per
file, instead of per tree, and was a way to glue the files together.
Ultimately, we nowadays don't live in that world anymore, and this
information is entirely useless anyway, as people are very welcome to
copy these files into any projects they like, and they shouldn't have to
change bits that are part of our copyright header for that.
hence, let's just get rid of this old cruft, and shorten our codebase a
bit.
Let's always write "1 << 0", "1 << 1" and so on, except where we need
more than 31 flag bits, where we write "UINT64(1) << 0", and so on to force
64bit values.
Files which are installed as-is (any .service and other unit files, .conf
files, .policy files, etc), are left as is. My assumption is that SPDX
identifiers are not yet that well known, so it's better to retain the
extended header to avoid any doubt.
I also kept any copyright lines. We can probably remove them, but it'd nice to
obtain explicit acks from all involved authors before doing that.
This reworks system call filter parsing, and replaces a couple of "bool"
function arguments by a single flags parameter.
This shouldn't change behaviour, except for one case: when we
recursively call our parsing function on our own syscall list, then
we'll lower the log level to LOG_DEBUG from LOG_WARNING, because at that
point things are just a problem in our own code rather than in the user
configuration we are parsing, and we shouldn't hence generate confusing
warnings about syntax errors.
Fixes: #8261
If multiple SystemCallFilter= settings, some of them are whitelist
and the others are blacklist, are sent to bus, then the parse
result was corrupted.
This fixes the parse logic, now it is the same as one used in
load-fragment.c
Christian Brauner
Yu Watanabe
Luca Boccassi
Daan De Meyer
Daan De Meyer
Lennart Poettering
Zbigniew Jędrzejewski-Szmek
Dominique Martinet
Greg Depoire--Ferrer
Topi Miettinen
Kevin Kuehler
Anita Zhang