core/dbus-execute: fix memleak on Mount/ExtensionImages parse failure

Define mount_image_free_many() in our usual fashion for use in
CLEANUP_ARRAY and ensure proper cleanup on error paths.

diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index 685abc948a37e7610e64a9e765414c80a120057e..59514f688dfe18fab30669fc60ec7f186905d173 100644 (file)
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -4050,6 +4050,8 @@ int bus_exec_context_set_transient_property(
                 char *source, *destination;
                 int permissive;
 
+                CLEANUP_ARRAY(mount_images, n_mount_images, mount_image_free_many);
+
                 r = sd_bus_message_enter_container(message, 'a', "(ssba(ss))");
                 if (r < 0)
                         return r;
@@ -4122,15 +4124,21 @@ int bus_exec_context_set_transient_property(
 
                 if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
                         if (n_mount_images == 0) {
-                                c->mount_images = mount_image_free_many(c->mount_images, &c->n_mount_images);
+                                mount_image_free_many(c->mount_images, c->n_mount_images);
+                                c->mount_images = NULL;
+                                c->n_mount_images = 0;
 
                                 unit_write_settingf(u, flags, name, "%s=", name);
                         } else {
-                                for (size_t i = 0; i < n_mount_images; ++i) {
-                                        r = mount_image_add(&c->mount_images, &c->n_mount_images, &mount_images[i]);
-                                        if (r < 0)
-                                                return r;
-                                }
+                                if (!c->mount_images) {
+                                        c->mount_images = TAKE_PTR(mount_images);
+                                        c->n_mount_images = n_mount_images;
+                                } else
+                                        FOREACH_ARRAY(i, mount_images, n_mount_images) {
+                                                r = mount_image_add(&c->mount_images, &c->n_mount_images, i);
+                                                if (r < 0)
+                                                        return r;
+                                        }
 
                                 unit_write_settingf(u, flags|UNIT_ESCAPE_C|UNIT_ESCAPE_SPECIFIERS,
                                                     name,
@@ -4140,14 +4148,15 @@ int bus_exec_context_set_transient_property(
                         }
                 }
 
-                mount_images = mount_image_free_many(mount_images, &n_mount_images);
-
                 return 1;
+
         } else if (streq(name, "ExtensionImages")) {
                 _cleanup_free_ char *format_str = NULL;
                 MountImage *extension_images = NULL;
                 size_t n_extension_images = 0;
 
+                CLEANUP_ARRAY(extension_images, n_extension_images, mount_image_free_many);
+
                 r = sd_bus_message_enter_container(message, 'a', "(sba(ss))");
                 if (r < 0)
                         return r;
@@ -4211,15 +4220,21 @@ int bus_exec_context_set_transient_property(
 
                 if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
                         if (n_extension_images == 0) {
-                                c->extension_images = mount_image_free_many(c->extension_images, &c->n_extension_images);
+                                mount_image_free_many(c->extension_images, c->n_extension_images);
+                                c->extension_images = NULL;
+                                c->n_extension_images = 0;
 
                                 unit_write_settingf(u, flags, name, "%s=", name);
                         } else {
-                                for (size_t i = 0; i < n_extension_images; ++i) {
-                                        r = mount_image_add(&c->extension_images, &c->n_extension_images, &extension_images[i]);
-                                        if (r < 0)
-                                                return r;
-                                }
+                                if (!c->extension_images) {
+                                        c->extension_images = TAKE_PTR(extension_images);
+                                        c->n_extension_images = n_extension_images;
+                                } else
+                                        FOREACH_ARRAY(i, extension_images, n_extension_images) {
+                                                r = mount_image_add(&c->extension_images, &c->n_extension_images, i);
+                                                if (r < 0)
+                                                        return r;
+                                        }
 
                                 unit_write_settingf(u, flags|UNIT_ESCAPE_C|UNIT_ESCAPE_SPECIFIERS,
                                                     name,
@@ -4229,8 +4244,6 @@ int bus_exec_context_set_transient_property(
                         }
                 }
 
-                extension_images = mount_image_free_many(extension_images, &n_extension_images);
-
                 return 1;
 
         } else if (STR_IN_SET(name, "StateDirectorySymlink", "RuntimeDirectorySymlink", "CacheDirectorySymlink", "LogsDirectorySymlink")) {

diff --git a/src/core/execute.c b/src/core/execute.c
index 3602e5229d3d84f0ad6d261e254910c876da960f..6d2291cadd55cb68a8f37df7deb8cafb542056ef 100644 (file)
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -684,8 +684,6 @@ void exec_context_done(ExecContext *c) {
         iovec_done(&c->root_hash_sig);
         c->root_hash_sig_path = mfree(c->root_hash_sig_path);
         c->root_verity = mfree(c->root_verity);
-        c->extension_images = mount_image_free_many(c->extension_images, &c->n_extension_images);
-        c->extension_directories = strv_free(c->extension_directories);
         c->tty_path = mfree(c->tty_path);
         c->syslog_identifier = mfree(c->syslog_identifier);
         c->user = mfree(c->user);
@@ -705,10 +703,16 @@ void exec_context_done(ExecContext *c) {
         bind_mount_free_many(c->bind_mounts, c->n_bind_mounts);
         c->bind_mounts = NULL;
         c->n_bind_mounts = 0;
+        mount_image_free_many(c->mount_images, c->n_mount_images);
+        c->mount_images = NULL;
+        c->n_mount_images = 0;
+        mount_image_free_many(c->extension_images, c->n_extension_images);
+        c->extension_images = NULL;
+        c->n_extension_images = 0;
+        c->extension_directories = strv_free(c->extension_directories);
         temporary_filesystem_free_many(c->temporary_filesystems, c->n_temporary_filesystems);
         c->temporary_filesystems = NULL;
         c->n_temporary_filesystems = 0;
-        c->mount_images = mount_image_free_many(c->mount_images, &c->n_mount_images);
 
         cpu_set_done(&c->cpu_set);
         numa_policy_reset(&c->numa_policy);

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index decdd2eae306355474c23ec0e1c1b66b3d620d34..ce90b08e562a382aa8f53ab8956625faa6f7495d 100644 (file)
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -5175,7 +5175,9 @@ int config_parse_mount_images(
 
         if (isempty(rvalue)) {
                 /* Empty assignment resets the list */
-                c->mount_images = mount_image_free_many(c->mount_images, &c->n_mount_images);
+                mount_image_free_many(c->mount_images, c->n_mount_images);
+                c->mount_images = NULL;
+                c->n_mount_images = 0;
                 return 0;
         }
 
@@ -5323,7 +5325,9 @@ int config_parse_extension_images(
 
         if (isempty(rvalue)) {
                 /* Empty assignment resets the list */
-                c->extension_images = mount_image_free_many(c->extension_images, &c->n_extension_images);
+                mount_image_free_many(c->extension_images, c->n_extension_images);
+                c->extension_images = NULL;
+                c->n_extension_images = 0;
                 return 0;
         }
 

diff --git a/src/core/namespace.c b/src/core/namespace.c
index 065e291ccb05e0a56fe514a44edb3af008ef7344..1a969895123331a965c1761ac351204bd07b455b 100644 (file)
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -3098,19 +3098,16 @@ int bind_mount_add(BindMount **b, size_t *n, const BindMount *item) {
         return 0;
 }
 
-MountImage* mount_image_free_many(MountImage *m, size_t *n) {
-        assert(n);
-        assert(m || *n == 0);
+void mount_image_free_many(MountImage *m, size_t n) {
+        assert(m || n == 0);
 
-        for (size_t i = 0; i < *n; i++) {
-                free(m[i].source);
-                free(m[i].destination);
-                mount_options_free_all(m[i].mount_options);
+        FOREACH_ARRAY(i, m, n) {
+                free(i->source);
+                free(i->destination);
+                mount_options_free_all(i->mount_options);
         }
 
         free(m);
-        *n = 0;
-        return NULL;
 }
 
 int mount_image_add(MountImage **m, size_t *n, const MountImage *item) {

diff --git a/src/core/namespace.h b/src/core/namespace.h
index ba4f9b80a2d99787160c328f81a1c6a66d341245..b96e7b4372e68f81122382a782855d72715472be 100644 (file)
--- a/src/core/namespace.h
+++ b/src/core/namespace.h
@@ -286,13 +286,16 @@ DECLARE_STRING_TABLE_LOOKUP(private_pids, PrivatePIDs);
 void bind_mount_free_many(BindMount *b, size_t n);
 int bind_mount_add(BindMount **b, size_t *n, const BindMount *item);
 
-void temporary_filesystem_free_many(TemporaryFileSystem *t, size_t n);
-int temporary_filesystem_add(TemporaryFileSystem **t, size_t *n,
-                             const char *path, const char *options);
-
-MountImage* mount_image_free_many(MountImage *m, size_t *n);
+void mount_image_free_many(MountImage *m, size_t n);
 int mount_image_add(MountImage **m, size_t *n, const MountImage *item);
 
+void temporary_filesystem_free_many(TemporaryFileSystem *t, size_t n);
+int temporary_filesystem_add(
+                TemporaryFileSystem **t,
+                size_t *n,
+                const char *path,
+                const char *options);
+
 int refresh_extensions_in_namespace(
                 const PidRef *target,
                 const char *hierarchy_env,