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 "<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)

diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index 2bd7b1c07eea36e879698ce1b78a51cec64cd734..9e6077c7e1feab672f5814b1a5e9ed1229fd1f50 100644 (file)
--- 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 560df952874ea21c7d8340b3aadb906f2e1ff185..8f9a7ac5464028840abacbe0ea3c8e3e0e33ed2d 100644 (file)
--- 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 d2bfd20fd43af518eef4b878c882eab37692dee0..d1f74a8fd7e190fa6dd156386e566f567ef1a071 100644 (file)
--- 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 ab0db25687826388d8e133ddbb6af1465dbcce48..fb6d501ece4e7673515d6a87861473d0b5f48983 100644 (file)
--- 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 9c94113239ee0e2b67a821a2f1c5d8ec3bb55570..380ce7fac8c08ceda1c19016017aca29483b4ad7 100644 (file)
--- 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, ...)             \