From 456a277d3f43a04d2c0fa8f7f480ce901e349e79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 11 Mar 2026 12:03:19 +0100 Subject: [PATCH 1/3] core: limit number of LogExtraFields We have two places where those fields can be set: config and the dbus interface. Let's clamp down on the number in both places. But in principle, we could also be upgrading (through serialization/deserialization) from an older systemd which didn't enforce this limit, so also check on deserialization. A user could have a unit with lots and lots of ExtraFields, but not enough to cause the issue in #40916. To handle this gracefully, ignore the extra fields, like we do in the parser. Where the field is used, assert that we are within the expected bounds. Fixes #40916. Reproducer: $ python3 -c 'from pydbus import SystemBus; from gi.repository import GLib; SystemBus().get("org.freedesktop.systemd1", "/org/freedesktop/systemd1").StartTransientUnit("crash.service", "fail", [("ExecStart", GLib.Variant("a(sasb)", [("/bin/true", ["/bin/true"], False)])), ("LogExtraFields", GLib.Variant("aay", [b"F%d=x" % i for i in range(140000)]))], [])' Traceback (most recent call last): File "", line 1, in from pydbus import SystemBus; from gi.repository import GLib; SystemBus().get("org.freedesktop.systemd1", "/org/freedesktop/systemd1").StartTransientUnit("crash.service", "fail", [("ExecStart", GLib.Variant("a(sasb)", [("/bin/true", ["/bin/true"], False)])), ("LogExtraFields", GLib.Variant("aay", [b"F%d=x" % i for i in range(140000)]))], []) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.14/site-packages/pydbus/proxy_method.py", line 102, in __call__ raise error File "/usr/lib/python3.14/site-packages/pydbus/proxy_method.py", line 97, in __call__ result = instance._bus.con.call_sync(*call_args) gi.repository.GLib.GError: g-dbus-error-quark: GDBus.Error:org.freedesktop.DBus.Error.InvalidArgs: Too many extra log fields. (16) --- src/core/dbus-execute.c | 3 +++ src/core/execute-serialize.c | 6 ++++++ src/core/load-fragment.c | 5 +++++ src/core/unit.c | 2 ++ src/core/unit.h | 6 ++++++ 5 files changed, 22 insertions(+) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 2bd7b1c07e..9e6077c7e1 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -2806,6 +2806,9 @@ int bus_exec_context_set_transient_property( return sd_bus_error_set(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Journal field is not valid UTF-8"); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { + if (c->n_log_extra_fields >= LOG_EXTRA_FIELDS_MAX) + return sd_bus_error_set(reterr_error, SD_BUS_ERROR_INVALID_ARGS, "Too many extra log fields."); + if (!GREEDY_REALLOC(c->log_extra_fields, c->n_log_extra_fields + 1)) return -ENOMEM; diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c index 560df95287..8f9a7ac546 100644 --- a/src/core/execute-serialize.c +++ b/src/core/execute-serialize.c @@ -30,6 +30,7 @@ #include "string-util.h" #include "strv.h" #include "time-util.h" +#include "unit.h" static int exec_cgroup_context_serialize(const CGroupContext *c, FILE *f) { _cleanup_free_ char *disable_controllers_str = NULL, *delegate_controllers_str = NULL, @@ -3104,6 +3105,11 @@ static int exec_context_deserialize(ExecContext *c, FILE *f) { if (r < 0) return r; } else if ((val = startswith(l, "exec-context-log-extra-fields="))) { + if (c->n_log_extra_fields >= LOG_EXTRA_FIELDS_MAX) { + log_warning("Too many extra log fields, ignoring."); + continue; + } + if (!GREEDY_REALLOC(c->log_extra_fields, c->n_log_extra_fields + 1)) return log_oom_debug(); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index d2bfd20fd4..d1f74a8fd7 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2941,6 +2941,11 @@ int config_parse_log_extra_fields( continue; } + if (c->n_log_extra_fields >= LOG_EXTRA_FIELDS_MAX) { + log_syntax(unit, LOG_WARNING, filename, line, 0, "Too many extra log fields, ignoring some."); + return 0; + } + if (!GREEDY_REALLOC(c->log_extra_fields, c->n_log_extra_fields + 1)) return log_oom(); diff --git a/src/core/unit.c b/src/core/unit.c index ab0db25687..fb6d501ece 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5843,6 +5843,8 @@ static int unit_export_log_extra_fields(Unit *u, const ExecContext *c) { if (c->n_log_extra_fields <= 0) return 0; + assert(c->n_log_extra_fields <= LOG_EXTRA_FIELDS_MAX); + sizes = newa(le64_t, c->n_log_extra_fields); iovec = newa(struct iovec, c->n_log_extra_fields * 2); diff --git a/src/core/unit.h b/src/core/unit.h index 9c94113239..380ce7fac8 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -10,6 +10,7 @@ #include "install.h" #include "iterator.h" #include "job.h" +#include "journal-importer.h" #include "list.h" #include "log.h" #include "log-context.h" @@ -1097,6 +1098,11 @@ int unit_queue_job_check_and_mangle_type(Unit *u, JobType *type, bool reload_if_ int parse_unit_marker(const char *marker, unsigned *settings, unsigned *mask); unsigned unit_normalize_markers(unsigned existing_markers, unsigned new_markers); +/* Trying to log with too many fields is going to fail. We need at least also MESSAGE=, + * but we generally log a few extra in most cases. So let's reserve 10. Anything + * above a few would be very unusual, but let's not be overly strict. */ +#define LOG_EXTRA_FIELDS_MAX (ENTRY_FIELD_COUNT_MAX - 10) + /* Macros which append UNIT= or USER_UNIT= to the message */ #define log_unit_full_errno_zerook(unit, level, error, ...) \ From 3105286f251286a4cc78cde04705e0c3b4013990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 11 Mar 2026 12:50:49 +0100 Subject: [PATCH 2/3] core/unit: shorten code I wanted to use _cleanup_(unlink_tempfilep), but the type doesn't match (char ** vs. char (*)[]), so the goto remains. --- src/core/unit.c | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index fb6d501ece..9af7fb5140 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5829,12 +5829,6 @@ static int unit_export_log_level_max(Unit *u, int log_level_max, bool overwrite) } static int unit_export_log_extra_fields(Unit *u, const ExecContext *c) { - _cleanup_close_ int fd = -EBADF; - struct iovec *iovec; - const char *p; - char *pattern; - le64_t *sizes; - ssize_t n; int r; if (u->exported_log_extra_fields) @@ -5845,8 +5839,8 @@ static int unit_export_log_extra_fields(Unit *u, const ExecContext *c) { assert(c->n_log_extra_fields <= LOG_EXTRA_FIELDS_MAX); - sizes = newa(le64_t, c->n_log_extra_fields); - iovec = newa(struct iovec, c->n_log_extra_fields * 2); + le64_t *sizes = newa(le64_t, c->n_log_extra_fields); + struct iovec *iovec = newa(struct iovec, c->n_log_extra_fields * 2); for (size_t i = 0; i < c->n_log_extra_fields; i++) { sizes[i] = htole64(c->log_extra_fields[i].iov_len); @@ -5855,15 +5849,14 @@ static int unit_export_log_extra_fields(Unit *u, const ExecContext *c) { iovec[i*2+1] = c->log_extra_fields[i]; } - p = strjoina("/run/systemd/units/log-extra-fields:", u->id); - pattern = strjoina(p, ".XXXXXX"); + const char *p = strjoina("/run/systemd/units/log-extra-fields:", u->id); + char *pattern = strjoina(p, ".XXXXXX"); - fd = mkostemp_safe(pattern); + _cleanup_close_ int fd = mkostemp_safe(pattern); if (fd < 0) return log_unit_debug_errno(u, fd, "Failed to create extra fields file %s: %m", p); - n = writev(fd, iovec, c->n_log_extra_fields*2); - if (n < 0) { + if (writev(fd, iovec, c->n_log_extra_fields * 2) < 0) { r = log_unit_debug_errno(u, errno, "Failed to write extra fields: %m"); goto fail; } @@ -5884,8 +5877,6 @@ fail: } static int unit_export_log_ratelimit_interval(Unit *u, const ExecContext *c) { - _cleanup_free_ char *buf = NULL; - const char *p; int r; assert(u); @@ -5897,10 +5888,10 @@ static int unit_export_log_ratelimit_interval(Unit *u, const ExecContext *c) { if (c->log_ratelimit.interval == 0) return 0; - p = strjoina("/run/systemd/units/log-rate-limit-interval:", u->id); + const char *p = strjoina("/run/systemd/units/log-rate-limit-interval:", u->id); - if (asprintf(&buf, "%" PRIu64, c->log_ratelimit.interval) < 0) - return log_oom(); + char buf[DECIMAL_STR_MAX(c->log_ratelimit.interval)]; + xsprintf(buf, "%" PRIu64, c->log_ratelimit.interval); r = symlink_atomic(buf, p); if (r < 0) @@ -5911,8 +5902,6 @@ static int unit_export_log_ratelimit_interval(Unit *u, const ExecContext *c) { } static int unit_export_log_ratelimit_burst(Unit *u, const ExecContext *c) { - _cleanup_free_ char *buf = NULL; - const char *p; int r; assert(u); @@ -5924,10 +5913,10 @@ static int unit_export_log_ratelimit_burst(Unit *u, const ExecContext *c) { if (c->log_ratelimit.burst == 0) return 0; - p = strjoina("/run/systemd/units/log-rate-limit-burst:", u->id); + const char *p = strjoina("/run/systemd/units/log-rate-limit-burst:", u->id); - if (asprintf(&buf, "%u", c->log_ratelimit.burst) < 0) - return log_oom(); + char buf[DECIMAL_STR_MAX(c->log_ratelimit.burst)]; + xsprintf(buf, "%u", c->log_ratelimit.burst); r = symlink_atomic(buf, p); if (r < 0) From 18031b17f8fa9bd7bd91cab3faab58f0299339dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 11 Mar 2026 13:15:57 +0100 Subject: [PATCH 3/3] TEST-07-PID1: add small test for LogExtraFields --- test/units/TEST-07-PID1.issue-40916.sh | 38 ++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100755 test/units/TEST-07-PID1.issue-40916.sh diff --git a/test/units/TEST-07-PID1.issue-40916.sh b/test/units/TEST-07-PID1.issue-40916.sh new file mode 100755 index 0000000000..c3f6dd6f59 --- /dev/null +++ b/test/units/TEST-07-PID1.issue-40916.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later + +set -eux +set -o pipefail + +# shellcheck source=test/units/util.sh +. "$(dirname "$0")"/util.sh + +# Units with excessive numbers of fields in LogExtraFields=. +# Issue: https://github.com/systemd/systemd/issues/40916 + +UNIT=test-07-pid1-issue-40916.service + +cleanup() { + rm -f /run/systemd/system/"$UNIT" + systemctl daemon-reload +} + +trap cleanup EXIT + +cat >/run/systemd/system/"$UNIT" <>/run/systemd/system/"$UNIT" + +systemctl start --wait "$UNIT" + +systemctl show -p LogExtraFields "$UNIT" | grep FIELD_1000 +(! systemctl show -p LogExtraFields "$UNIT" | grep FIELD_1500) + +# Now try setting the properties dynamically +(! systemd-run --wait -u test-07-pid1-issue-40916-1.service -pLogExtraFields=FD{1..2000}=1 true) +systemd-run --wait -u test-07-pid1-issue-40916-1.service -pLogExtraFields=FD{1..1000}=1 true