Commit Graph
472 Commits
Author SHA1 Message Date
Zbigniew Jędrzejewski-Szmek 8ec2e177b0 coredump: fix 0-passed-as-pointer warning 2025-06-06 22:15:03 +02:00
Michal Sekletar 8f8148cb08 Revert "coredump: lock down EnterNamespace= mount even more"
This reverts commit 4c9c8b8d09.

Reverted change broke EnterNamespace= completely. For example, libdw
tries to access libc in /lib64 which points to usr/lib64 and that fails.
Similarly for binaries, we need to be able to resolve /bin to usr/bin
and /sbin to usr/sbin at the very least.
2025-06-04 19:21:07 +09:00
Adrian Vovk 554130faf3 Define uid range for greeter
In multi-seat scenarios, a display manager might need to start multiple
greeter sessions. But systemd allows at most one graphical session per
user. So, display managers now have a range of UIDs to dynamically
allocate users for their greeter sessions.
2025-06-02 11:19:12 +02:00
Lennart Poettering 1105f8d20f tree-wide: only use .si_pid field in siginfo_t, if .si_code indicates that's safe
Fixes: #37498
2025-05-31 14:48:37 +02:00
Zbigniew Jędrzejewski-Szmek 9ce8e3e449 Define helper to call PR_SET_DUMPABLE 2025-05-29 00:19:24 +02:00
Zbigniew Jędrzejewski-Szmek 76e0ab49c4 coredump: introduce an enum to wrap dumpable constants
Two constants are described in the man page, but are not defined by a header.
The third constant is described in the kernel docs. Use explicit values to
show that those are values are defined externally.
2025-05-29 00:12:10 +02:00
Zbigniew Jędrzejewski-Szmek e6a8687b93 coredump: when %F/pidfd is used, again allow forwarding to containers 2025-05-29 00:10:55 +02:00
Luca Boccassi 868d95577e coredump: add support for new %F PIDFD specifier
A new core_pattern specifier was added, %F, to provide a PIDFD
to the usermode helper process referring to the crashed process.
This removes all possible race conditions, ensuring only the
crashed process gets inspected by systemd-coredump.
2025-05-29 00:10:55 +02:00
Zbigniew Jędrzejewski-Szmek 13902e0253 coredump: get rid of a bogus assertion
The check looks plausible, but when I started checking whether it needs
to be lowered for the recent changes, I realized that it doesn't make
much sense.

context_parse_iovw() is called from a few places, e.g.:
- process_socket(), where the other side controls the contents of the
  message. We already do other checks on the correctness of the message
  and this assert is not needed.
- gather_pid_metadata_from_argv(), which is called after
  inserting MESSAGE_ID= and PRIORITY= into the array, so there is no
  direct relation between _META_ARGV_MAX and the number of args in the
  iovw.
- gather_pid_metadata_from_procfs(), where we insert a bazillion fields,
  but without any relation to _META_ARGV_MAX.

Since we already separately check if the required stuff was set, drop this
misleading check.
2025-05-29 00:10:55 +02:00
Zbigniew Jędrzejewski-Szmek 8fc7b2a211 coredump: also stop forwarding non-dumpable processes
See the comment in the patch for details.

Suggested-by: Qualys Security Advisory <qsa@qualys.com>
2025-05-29 00:10:55 +02:00
Zbigniew Jędrzejewski-Szmek 0c49e0049b coredump: use %d in kernel core pattern
The kernel provides %d which is documented as
"dump mode—same as value returned by prctl(2) PR_GET_DUMPABLE".

We already query /proc/pid/auxv for this information, but unfortunately this
check is subject to a race, because the crashed process may be replaced by an
attacker before we read this data, for example replacing a SUID process that
was killed by a signal with another process that is not SUID, tricking us into
making the coredump of the original process readable by the attacker.

With this patch, we effectively add one more check to the list of conditions
that need be satisfied if we are to make the coredump accessible to the user.

Reportedy-by: Qualys Security Advisory <qsa@qualys.com>

In principle, %d might return a value other than 0, 1, or 2 in the future.
Thus, we accept those, but emit a notice.
2025-05-29 00:10:08 +02:00
Zbigniew Jędrzejewski-Szmek 49f1f2d4a7 coredump: get rid of _META_MANDATORY_MAX
No functional change. This change is done in preparation for future changes.
Currently, the list of fields which are received on the command line is a
strict subset of the fields which are always expected to be received on a
socket. But when we add new kernel args in the future, we'll have two
non-overlapping sets and this approach will not work. Get rid of the variable
and enumerate the required fields. This set will never change, so this is
actually more maintainable.

The message with the hint where to add new fields is switched with
_META_ARGV_MAX. The new order is more correct.
2025-05-26 15:33:08 +02:00
Zbigniew Jędrzejewski-Szmek 61d78f1788 Restore compatibility for coredump --backtrace (#37559) 2025-05-22 13:40:29 +02:00
Zbigniew Jędrzejewski-Szmek c673f1f67a coredump: wrap long lines, fix grammar in comments 2025-05-21 23:51:31 +02:00
Zbigniew Jędrzejewski-Szmek ded0aac389 coredump: restore compatibility with older patterns
This was broken in f45b801551. Unfortunately
the review does not talk about backward compatibility at all. There are
two places where it matters:
- During upgrades, the replacement of kernel.core_pattern is asynchronous.
  For example, during rpm upgrades, it would be updated a post-transaction
  file trigger. In other scenarios, the update might only happen after
  reboot. We have a potentially long window where the old pattern is in
  place. We need to capture coredumps during upgrades too.
- With --backtrace. The interface of --backtrace, in hindsight, is not
  great. But there are users of --backtrace which were written to use
  a specific set of arguments, and we can't just break compatiblity.
  One example is systemd-coredump-python, but there are also reports of
  users using --backtrace to generate coredump logs.

Thus, we require the original set of args, and will use the additional args if
found.

A test is added to verify that --backtrace works with and without the optional
args.
2025-05-21 23:45:14 +02:00
Daan De Meyer 86b9a3e31d tree-wide: Clean up includes
This removes more unused includes from the components already cleaned
up in #37467 and #37457.

Split out of #37344.
2025-05-21 10:49:42 +02:00
Daan De Meyer a583b34416 meson: Extract more objects instead of compiling multiple times
Also, let's deflatten the lists of sources in preparation for the
next commit at the same time.

In systemctl, we split out systemctl-main.c to make sure the definition
of main() is in a separate object which allows us to extract the systemctl.c
object and link it in the fuzzer target without getting a multiple definition
error when linking.
2025-05-20 10:29:58 +02:00
Daan De Meyer b06adfb06c coredump: Clean up includes
Split out of #37344.
2025-05-15 00:02:00 +02:00
Daan De Meyer 6350d2dbd9 meson: Extract objects instead of creating intermediate static libraries
Currently, when we want to add unit tests for code that is compiled into
an executable, we either compile the code at least twice (once for the
executable, and once for each test that uses it) or we create a static
library which is then used by both the executable and all the tests.

Both of these options are not ideal, compiling source files more than
once slows down the build for no reason and creating the intermediate
static libraries takes a lot of boilerplate.

Instead, let's use the extract_objects() method that meson exposes on
build targets. This allows us to extract the objects corresponding to
specific source files and use them in other executables. Because we
define all executables upfront into a dictionary, we integrate this into
the dictionary approach by adding two new fields:

- 'extract' takes a list of files for which objects should be extracted.
  The extracted objects are stored in a dict keyed by the executable name
  from which they were extracted.
- 'objects' takes the name of an executable from which the extracted
  objects should be added to the current executable.

One side effect of this approach is that we can't build test executables
anymore without building the main executable, so we stop building test
executables unless we're also building the main executable. This allows
us to switch to using subdir_done() in all of these subdirectories to skip
parsing them if the corresponding component is disabled.

These changes get me down from 2439 => 2403 ninja targets on a full rebuild
from scratch.
2025-05-12 13:35:50 +02:00
Daan De Meyer 1cf40697e3 tree-wide: Sort includes
This was done by running a locally built clang-format with
https://github.com/llvm/llvm-project/pull/137617 and
https://github.com/llvm/llvm-project/pull/137840 applied on all .c
and .h files.
2025-04-30 09:30:51 +02:00
Daan De Meyer 8aa304d313 shared: Remove circular dependency between image-policy.h and dissect-image.h
ImagePolicy can just be a forward declaration in dissect-image.h.
2025-04-23 09:53:43 +02:00
Daan De Meyer 93a1f7921a basic: Stop including log.h in macro.h
Now that the necessary functions from log.h have been moved to macro.h,
we can stop including log.h in macro.h. This requires modifying source
files all over the tree to include log.h instead.
2025-04-18 14:19:15 +02:00
Yu Watanabe c96a5d9912 misc: hash_ops related cleanups (#37117) 2025-04-18 05:59:15 +09:00
Luca Boccassi ba6c955f21 coredump: verify pidfd after parsing data in usermode helper
Ensure the pidfd is still valid before continuing

Follow-up for 313537da6f
2025-04-13 10:18:58 +01:00
Yu Watanabe 855800aaec coredump: replace custom cleanup function with specific hash_ops with destructor 2025-04-13 10:00:57 +09:00
Lennart Poettering ed2e3a8404 coredump: do not disable removal of temporary coredump file 2025-03-26 11:37:15 -04:00
Lennart Poettering dc8e3118f9 coredump: make sure we initialize all return parameters on success of save_external_coredump() 2025-03-26 11:37:11 -04:00
Lennart Poettering 5125a0b8fa coredump: do not remove PID1/journal coredumps if Storage=journal is used
We always redirect PID1/journal coredumps directly onto disk instead of
the journal even if that's configured because that might cause a
deadlock because we are still pinning the old journal process while
processing the coredump. However, so far we then immediately deleted the
coredumps because of Storage=journal, which is very annoying, since
there's hence no copy kept whatsoever.

Let's hence exclude PID1+journal from the removal.

This in particulary brings the code in line with the log messages which
claim we kept the file around but we actually did not.
2025-03-26 11:37:08 -04:00
Lennart Poettering 2d0bcf1e02 coredump: rename fix_permissions() → fix_permissions_and_link()
The function doesn't just adjust permissions, it actually moves the
file into place. The function name should really make this fundamental
fact clear.
2025-03-26 11:33:38 -04:00
Yu Watanabe 3cf6a3a3d4 tree-wide: check more log message format in log_struct() and friends
This introduce LOG_ITEM() macro that checks arbitrary formats in
log_struct().
Then, drop _printf_ attribute from log_struct_internal(), as it does not
help so much, and compiler checked only the first format string.

Hopefully, this silences false-positive warnings by Coverity.
2025-03-19 01:56:48 +09:00
Lennart Poettering da65941c3e coredump,analyze: use read_full_file() for reading various top-level /proc/ files
Kernel API file systems typically use either "raw" or "seq_file" to
implement their various interface files. The former are really simple
(to point I'd call them broken), in that they have no understanding of
file offsets, and return their contents again and again on every read(),
and thus EOF is indicated by a short read, not by a zero read. The
latter otoh works like a typical file: you read until you get a
zero-sized read back.

We have read_virtual_file() to read the "raw" files, and can use regular
read_full_file() to read the "seq_file" ones.

Apparently all files in the top-level /proc/ directory use 'seq_file'.
but we accidentally used read_virtual_file() for them. Fix that.

Also clarify in a comment what the rules are.

Fixes: #36131
2025-03-13 14:18:46 +01:00
Yu Watanabe 5c9feb2d6e tree-wide: drop unnecessary break in default branch 2025-03-06 11:42:20 +01:00
Yu Watanabe ec32732043 basic: introduce our own sys/mount.h implementation
To resolve conflict with sys/mount.h and linux/mount.h or linux/fs.h.

The conflict between sys/mount.h and linux/mount.h is resolved in
glibc-2.37 (774058d72942249f71d74e7f2b639f77184160a6), but our baseline
is still glibc-2.31. Also, even with the version or newer, still
sys/mount.h conflicts with linux/fs.h, which is included by
linux/btrfs.h.

This introduces our own implementation of sys/mount.h, that can be
simultaneously included with linux/mount.h and linux/fs.h. This also
imports linux/fs.h, linux/mount.h, and several other dependent headers.
The introduced sys/mount.h header itself may not be enough simple, but
by using the header, we can drop most of workarounds in other source files.
2025-03-04 02:24:49 +09:00
Lennart Poettering d6267b9b18 process-util: port pid_from_same_root_fs() to pidref, and port three places over to it 2025-01-16 11:55:21 +01:00
Lennart Poettering 0b8b13324e namespace-util: port namespace_get_leader() to PidRef 2025-01-10 14:22:49 +01:00
Lennart Poettering 36812cb659 coredump: port to pidref_namespace_open() 2025-01-10 14:14:17 +01:00
Lennart Poettering 8703a50835 coredump: acquire further attributes via pidref rather than pid 2025-01-10 14:14:17 +01:00
Lennart Poettering 336acebc77 basic: port various pidfd/pidref helpers to PIDFD_GET_INFO and PIDFD_GET_*_NAMESPACE (#35242)
Supersedes #35308 (cherry-picked one commit and replaced the rest)

(I left a few comments that's folded by GitHub. Please make sure to
check them too.)
2025-01-06 11:23:08 +01:00
Mike Yuan 92b8e5e72f namespace-util: introduce pidref_in_same_namespace() 2025-01-04 17:08:01 +01:00
Mike Yuan 5734893f82 tree-wide: make sigactions static const 2025-01-04 16:40:53 +01:00
Yu Watanabe 77064620d7 Revert "coredumpctl: Don't treat no coredumps as failure"
This reverts commit dfe79b9ed2.
2024-12-11 11:14:37 +09:00
Daan De Meyer dfe79b9ed2 coredumpctl: Don't treat no coredumps as failure
Having to deal with a process that fails or doesn't fail depending on
whether there are coredumps or not is incredibly annoying for users.
2024-12-10 21:03:20 +01:00
Lennart Poettering 00f7398049 coredump: rename gather_pid_mount_tree_fd() → acquire_pid_mount_tree_fd()
From my understanding of the english language "gather" imples there are
multiple things to gather. But here there's only one, hence use
"acquire"
2024-11-01 10:07:55 +01:00
Lennart Poettering 4c9c8b8d09 coredump: lock down EnterNamespace= mount even more
Let's disable symlink following if we attach a container's mount tree to
our own mount namespace. We afte rall mount the tree to a different
location in the mount tree than where it was inside the container, hence
symlinks (if they exist) will all point to the wrong places (even if
relative, some might point to other places). And since symlink attacks
are a thing, and we let libdw operate on the tree, let's lock this down
as much as we can and simply disable symlink traversal entirely.
2024-11-01 10:05:53 +01:00
Lennart Poettering 098c3975ac coredump: make check that all argv[] meta data fields are passed strict
Otherwise, if some field is not supplied we might end up parsing a NULL
string later. Let's catch that early.
2024-10-31 23:09:14 +01:00
Lennart Poettering b1694040af coredump: use memory_startswith() when looking at a data blob 2024-10-31 23:08:11 +01:00
Lennart Poettering 2865561eaa coredump: move to _cleanup_ for destroying iovw object 2024-10-31 23:08:11 +01:00
Lennart Poettering 19455dd600 coredump: parse rlimit field at same place as other fields 2024-10-31 23:08:11 +01:00
Lennart Poettering 960b045875 coredump: parse signal number at the same time as parsing other fields 2024-10-31 23:08:11 +01:00
Lennart Poettering 32756e57ef coredump: rename save_context() → context_parse_iovw()
The function doesn't "save" anything, it just parses iovw into the
individual fields, hence name the function accordingly.
2024-10-31 23:08:11 +01:00