udev/net: reset the config list head in link_configs_free()

link_configs_free() freed every LinkConfig via LIST_FOREACH but left
ctx->configs pointing at the (freed) former list head. That is harmless for
link_config_ctx_free(), which frees the context immediately afterwards, but
link_config_load() calls link_configs_free() up front to clear the context
before (re)loading — and the dangling head then makes the subsequent
LIST_PREPEND() and the eventual teardown dereference freed memory.

Use LIST_CLEAR(), which pops and frees each entry and resets the head to NULL.

Follow-up for af6f0d422c521374ee6a2dd92df5935a5a476ae5

diff --git a/src/udev/meson.build b/src/udev/meson.build
index 50060646ba9f82ea78bf8a1d2fa2daf41e65d74c..4c34627be923fcf30f5408b375d415187141d8db 100644 (file)
--- a/src/udev/meson.build
+++ b/src/udev/meson.build
@@ -202,7 +202,7 @@ executables += [
                 'suite' : 'udev',
         },
         udev_test_template + {
-                'sources' : files('net/test-link-config-tables.c'),
+                'sources' : files('net/test-link.c'),
                 'suite' : 'udev',
         },
         udev_test_template + {

diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c
index 9d2cb3730e2e8f7d208770063250efdf39fee83d..ec50a155acdbd5be9dc9e98bbf7495f2e2138fe8 100644 (file)
--- a/src/udev/net/link-config.c
+++ b/src/udev/net/link-config.c
@@ -98,8 +98,7 @@ static void link_configs_free(LinkConfigContext *ctx) {
 
         ctx->stats_by_path = hashmap_free(ctx->stats_by_path);
 
-        LIST_FOREACH(configs, config, ctx->configs)
-                link_config_free(config);
+        LIST_CLEAR(configs, ctx->configs, link_config_free);
 }
 
 LinkConfigContext *link_config_ctx_free(LinkConfigContext *ctx) {

diff --git a/src/udev/net/test-link-config-tables.c b/src/udev/net/test-link-config-tables.c
deleted file mode 100644 (file)
index 58b628b..0000000
--- a/src/udev/net/test-link-config-tables.c
+++ /dev/null
@@ -1,14 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-
-#include "link-config.h"
-#include "test-tables.h"
-#include "tests.h"
-
-int main(int argc, char **argv) {
-        test_setup_logging(LOG_DEBUG);
-
-        test_table(MACAddressPolicy, mac_address_policy, MAC_ADDRESS_POLICY);
-        test_table(IRQAffinityPolicy, irq_affinity_policy, IRQ_AFFINITY_POLICY);
-
-        return EXIT_SUCCESS;
-}

diff --git a/src/udev/net/test-link.c b/src/udev/net/test-link.c
new file mode 100644 (file)
index 0000000..624f51d
--- /dev/null
+++ b/src/udev/net/test-link.c
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "fd-util.h"
+#include "link-config.h"
+#include "test-tables.h"
+#include "tests.h"
+#include "tmpfile-util.h"
+
+TEST(link_config_load_reload) {
+        /* Loading link configuration into an already-populated context (as link_config_load() itself
+         * does via its initial clear) must not leave ctx->configs pointing at freed entries. If the clear
+         * leaves the list head dangling, the reload below and the context teardown afterwards touch freed
+         * memory. */
+
+        _cleanup_(link_config_ctx_freep) LinkConfigContext *ctx = NULL;
+        _cleanup_(unlink_tempfilep) char filename[] = "/tmp/test-link-config.XXXXXX";
+        _cleanup_fclose_ FILE *f = NULL;
+
+        ASSERT_OK(link_config_ctx_new(&ctx));
+
+        ASSERT_OK(fmkostemp_safe(filename, "r+", &f));
+        ASSERT_OK(fputs("[Match]\nOriginalName=*\n\n[Link]\nMTUBytes=1500\n", f));
+        ASSERT_OK(fflush(f));
+
+        /* Populate the context with one configuration. */
+        ASSERT_OK(link_load_one(ctx, filename));
+
+        /* Reload into the same, already-populated context. */
+        ASSERT_OK(link_config_load(ctx));
+}
+
+TEST(tables) {
+        test_table(MACAddressPolicy, mac_address_policy, MAC_ADDRESS_POLICY);
+        test_table(IRQAffinityPolicy, irq_affinity_policy, IRQ_AFFINITY_POLICY);
+}
+
+DEFINE_TEST_MAIN(LOG_DEBUG);