From 24dee4afc57e926575458d043c0bf82c695f88a3 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 6 Jul 2026 17:10:35 +0100 Subject: [PATCH 1/2] udev/net: reset the config list head in link_configs_free() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/udev/meson.build | 2 +- src/udev/net/link-config.c | 3 +-- src/udev/net/test-link-config-tables.c | 14 ---------- src/udev/net/test-link.c | 37 ++++++++++++++++++++++++++ 4 files changed, 39 insertions(+), 17 deletions(-) delete mode 100644 src/udev/net/test-link-config-tables.c create mode 100644 src/udev/net/test-link.c diff --git a/src/udev/meson.build b/src/udev/meson.build index 50060646ba..4c34627be9 100644 --- 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 9d2cb3730e..ec50a155ac 100644 --- 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 index 58b628bc9b..0000000000 --- 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 index 0000000000..624f51db37 --- /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); From dcfd156705d9ce7b3ff128d84a6628c946c3553a Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 6 Jul 2026 18:37:23 +0100 Subject: [PATCH 2/2] libudev: replace unique list entries in place udev_list_entry_add() freed the existing entry for a name before inserting its replacement. Today that cannot lose the old entry, but only because re-adding the key that was just removed never makes the hashmap grow, which is an internal detail of the hashmap. Use hashmap_ensure_replace() instead: it updates the existing bucket in place, making it explicit that replacing an entry neither allocates nor can drop the previous value under OOM. Follow-up for c01130824f22ec3835a35c8ab5f9aea65195a40f --- src/libudev/libudev-list.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c index 0385c4679e..3ea77507c9 100644 --- a/src/libudev/libudev-list.c +++ b/src/libudev/libudev-list.c @@ -40,7 +40,7 @@ static struct udev_list_entry* udev_list_entry_free(struct udev_list_entry *entr if (entry->list) { if (entry->list->unique && entry->name) - hashmap_remove(entry->list->unique_entries, entry->name); + hashmap_remove_value(entry->list->unique_entries, entry->name, entry); if (!entry->list->unique || entry->list->uptodate) LIST_REMOVE(entries, entry->list->entries, entry); @@ -90,11 +90,19 @@ struct udev_list_entry* udev_list_entry_add(struct udev_list *list, const char * return NULL; if (list->unique) { - udev_list_entry_free(hashmap_get(list->unique_entries, entry->name)); + struct udev_list_entry *old; - if (hashmap_ensure_put(&list->unique_entries, &udev_list_entry_hash_ops, entry->name, entry) < 0) + /* Replace the entry for an already known name in a single step. hashmap_replace() reuses the + * existing bucket and hence never allocates, so an existing entry is never dropped, not even + * under memory pressure. Allocating may only fail when adding a genuinely new name, in which + * case there is nothing to lose. */ + old = hashmap_get(list->unique_entries, entry->name); + + if (hashmap_ensure_replace(&list->unique_entries, &udev_list_entry_hash_ops, entry->name, entry) < 0) return NULL; + udev_list_entry_free(old); + list->uptodate = false; } else LIST_APPEND(entries, list->entries, entry);