From: Frantisek Sumsal <frantisek@sumsal.cz>
Date: Fri, 27 Oct 2023 16:36:35 +0000 (+0200)
Subject: core: don't insert an extra space before each SocketBind{Allow,Deny}= item
X-Git-Tag: v255-rc1~115^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b0bb3be130e241178646df0b5c5f02ed661651d4;p=thirdparty%2Fsystemd.git

core: don't insert an extra space before each SocketBind{Allow,Deny}= item

The extra space was actually screwing up deserialization:

~# systemd-run --wait --pipe -p SocketBindAllow=any true
Running as unit: run-u167.service
Finished with result: exit-code
Main processes terminated with: code=exited/status=234
Service runtime: 1ms
CPU time consumed: 0
~# journalctl -b -p err
...
Oct 27 16:39:15 arch systemd-executor[5983]: Failed to deserialize: Invalid argument

Let's not do that by default and introduce a simple wrapper which
inserts the space after each item only when necessary.
---

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 15dfc56fb77..535f457fa83 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -698,16 +698,14 @@ void cgroup_context_dump(Unit *u, FILE* f, const char *prefix) {
                         prefix, bpf_cgroup_attach_type_to_string(p->attach_type), p->bpffs_path);
 
         if (c->socket_bind_allow) {
-                fprintf(f, "%sSocketBindAllow:", prefix);
-                LIST_FOREACH(socket_bind_items, bi, c->socket_bind_allow)
-                        cgroup_context_dump_socket_bind_item(bi, f);
+                fprintf(f, "%sSocketBindAllow: ", prefix);
+                cgroup_context_dump_socket_bind_items(c->socket_bind_allow, f);
                 fputc('\n', f);
         }
 
         if (c->socket_bind_deny) {
-                fprintf(f, "%sSocketBindDeny:", prefix);
-                LIST_FOREACH(socket_bind_items, bi, c->socket_bind_deny)
-                        cgroup_context_dump_socket_bind_item(bi, f);
+                fprintf(f, "%sSocketBindDeny: ", prefix);
+                cgroup_context_dump_socket_bind_items(c->socket_bind_deny, f);
                 fputc('\n', f);
         }
 
@@ -734,16 +732,29 @@ void cgroup_context_dump_socket_bind_item(const CGroupSocketBindItem *item, FILE
         }
 
         if (item->nr_ports == 0)
-                fprintf(f, " %s%s%s%sany", family, colon1, protocol, colon2);
+                fprintf(f, "%s%s%s%sany", family, colon1, protocol, colon2);
         else if (item->nr_ports == 1)
-                fprintf(f, " %s%s%s%s%" PRIu16, family, colon1, protocol, colon2, item->port_min);
+                fprintf(f, "%s%s%s%s%" PRIu16, family, colon1, protocol, colon2, item->port_min);
         else {
                 uint16_t port_max = item->port_min + item->nr_ports - 1;
-                fprintf(f, " %s%s%s%s%" PRIu16 "-%" PRIu16, family, colon1, protocol, colon2,
+                fprintf(f, "%s%s%s%s%" PRIu16 "-%" PRIu16, family, colon1, protocol, colon2,
                         item->port_min, port_max);
         }
 }
 
+void cgroup_context_dump_socket_bind_items(const CGroupSocketBindItem *items, FILE *f) {
+        bool first = true;
+
+        LIST_FOREACH(socket_bind_items, bi, items) {
+                if (first)
+                        first = false;
+                else
+                        fputc(' ', f);
+
+                cgroup_context_dump_socket_bind_item(bi, f);
+        }
+}
+
 int cgroup_context_add_device_allow(CGroupContext *c, const char *dev, CGroupDevicePermissions p) {
         _cleanup_free_ CGroupDeviceAllow *a = NULL;
         _cleanup_free_ char *d = NULL;
diff --git a/src/core/cgroup.h b/src/core/cgroup.h
index 9816966dbb2..d7cc842835b 100644
--- a/src/core/cgroup.h
+++ b/src/core/cgroup.h
@@ -273,6 +273,7 @@ void cgroup_context_init(CGroupContext *c);
 void cgroup_context_done(CGroupContext *c);
 void cgroup_context_dump(Unit *u, FILE* f, const char *prefix);
 void cgroup_context_dump_socket_bind_item(const CGroupSocketBindItem *item, FILE *f);
+void cgroup_context_dump_socket_bind_items(const CGroupSocketBindItem *items, FILE *f);
 
 void cgroup_context_free_device_allow(CGroupContext *c, CGroupDeviceAllow *a);
 void cgroup_context_free_io_device_weight(CGroupContext *c, CGroupIODeviceWeight *w);
diff --git a/src/test/test-socket-bind.c b/src/test/test-socket-bind.c
index 9372f208ae9..84a897801a5 100644
--- a/src/test/test-socket-bind.c
+++ b/src/test/test-socket-bind.c
@@ -49,9 +49,8 @@ static int test_socket_bind(
                         return log_unit_error_errno(u, r, "Failed to parse SocketBindAllow: %m");
         }
 
-        fprintf(stderr, "SocketBindAllow:");
-        LIST_FOREACH(socket_bind_items, bi, cc->socket_bind_allow)
-                cgroup_context_dump_socket_bind_item(bi, stderr);
+        fprintf(stderr, "SocketBindAllow: ");
+        cgroup_context_dump_socket_bind_items(cc->socket_bind_allow, stderr);
         fputc('\n', stderr);
 
         STRV_FOREACH(rule, deny_rules) {
@@ -62,9 +61,8 @@ static int test_socket_bind(
                         return log_unit_error_errno(u, r, "Failed to parse SocketBindDeny: %m");
         }
 
-        fprintf(stderr, "SocketBindDeny:");
-        LIST_FOREACH(socket_bind_items, bi, cc->socket_bind_deny)
-                cgroup_context_dump_socket_bind_item(bi, stderr);
+        fprintf(stderr, "SocketBindDeny: ");
+        cgroup_context_dump_socket_bind_items(cc->socket_bind_deny, stderr);
         fputc('\n', stderr);
 
         exec_start = strjoin("-timeout --preserve-status -sSIGTERM 1s ", netcat_path, " -l ", port, " -vv");