SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

core: limit number of LogExtraFields (#41044)

Browse Source 
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:
```console
$ 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 "<string>", line 1, in <module>
    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)
```
This commit is contained in:
Luca Boccassi
2026-03-11 19:40:43 +00:00
committed by GitHub
parent 4b551035b4 18031b17f8
commit 415663a13a
6 changed files with 72 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/dbus-execute.c
+3
View File
@@ -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;
src/core/execute-serialize.c
+6
View File
@@ -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();
src/core/load-fragment.c
+5
View File
@@ -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();
src/core/unit.c
+14 -23
View File
@@ -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)
@@ -5843,8 +5837,10 @@ static int unit_export_log_extra_fields(Unit *u, const ExecContext *c) {
if (c->n_log_extra_fields <= 0)
return 0;
sizes = newa(le64_t, c->n_log_extra_fields);
iovec = newa(struct iovec, c->n_log_extra_fields * 2);
assert(c->n_log_extra_fields <= LOG_EXTRA_FIELDS_MAX);
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);
@@ -5853,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;
}
@@ -5882,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);
@@ -5895,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)
@@ -5909,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);
@@ -5922,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)
src/core/unit.h
+6
View File
@@ -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, ...) \
test/units/TEST-07-PID1.issue-40916.sh
Executable
+38
View File
@@ -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" <<EOF
[Service]
ExecStart=true
EOF
for i in {1..2000}; do
echo "LogExtraFields=FIELD_$i=$i"
done >>/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