From 2f72db10d9a0cdf3c1b22044df44c2e9fd61ff1b Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 17:55:37 +0100 Subject: [PATCH 01/11] sd-device: bound the tag filter BPF program size The tag-match loop emits 6 instructions per tag into a fixed 512-entry stack array without a bounds check (unlike the subsystem loop below), so enough tags overflow the stack. Refuse with -E2BIG instead. Follow-up for b1c097af8df58a94cba031a347061b7cb9b62d9b --- src/libsystemd/sd-device/device-monitor.c | 77 ++++++++++++------- .../sd-device/test-sd-device-monitor.c | 16 ++++ 2 files changed, 66 insertions(+), 27 deletions(-) diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index 962329a865..7d9433920e 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -781,34 +781,57 @@ int device_monitor_send( return count; } -static void bpf_stmt(struct sock_filter *ins, unsigned *i, - unsigned short code, unsigned data) { +static int bpf_stmt_impl(struct sock_filter *ins, size_t *i, size_t n_ins, + unsigned short code, unsigned data) { + assert(ins); assert(i); + if (*i >= n_ins) + return -E2BIG; + ins[(*i)++] = (struct sock_filter) { .code = code, .k = data, }; + return 0; } -static void bpf_jmp(struct sock_filter *ins, unsigned *i, - unsigned short code, unsigned data, - unsigned short jt, unsigned short jf) { +#define bpf_stmt(ins, i, code, data) \ + bpf_stmt_impl((ins), (i), ELEMENTSOF(ins), (code), (data)) + +static int bpf_jmp_impl(struct sock_filter *ins, size_t *i, size_t n_ins, + unsigned short code, unsigned data, + unsigned jt, unsigned jf) { + assert(ins); assert(i); + if (*i >= n_ins) + return -E2BIG; + + /* The jump offsets are stored in single bytes (struct sock_filter.jt/.jf are __u8). A larger + * offset would be silently truncated and make the filter branch to the wrong instruction, i.e. + * drop events that should match. */ + if (jt > UINT8_MAX || jf > UINT8_MAX) + return -E2BIG; + ins[(*i)++] = (struct sock_filter) { .code = code, .jt = jt, .jf = jf, .k = data, }; + return 0; } +#define bpf_jmp(ins, i, code, data, jt, jf) \ + bpf_jmp_impl((ins), (i), ELEMENTSOF(ins), (code), (data), (jt), (jf)) + _public_ int sd_device_monitor_filter_update(sd_device_monitor *m) { struct sock_filter ins[512] = {}; struct sock_fprog filter; const char *subsystem, *devtype, *tag; - unsigned i = 0; + size_t i = 0; + int r; assert_return(m, -EINVAL); @@ -823,11 +846,11 @@ _public_ int sd_device_monitor_filter_update(sd_device_monitor *m) { } /* load magic in A */ - bpf_stmt(ins, &i, BPF_LD|BPF_W|BPF_ABS, offsetof(monitor_netlink_header, magic)); + r = bpf_stmt(ins, &i, BPF_LD|BPF_W|BPF_ABS, offsetof(monitor_netlink_header, magic)); /* jump if magic matches */ - bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, UDEV_MONITOR_MAGIC, 1, 0); + RET_GATHER(r, bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, UDEV_MONITOR_MAGIC, 1, 0)); /* wrong magic, pass packet */ - bpf_stmt(ins, &i, BPF_RET|BPF_K, 0xffffffff); + RET_GATHER(r, bpf_stmt(ins, &i, BPF_RET|BPF_K, 0xffffffff)); if (!set_isempty(m->tag_filter)) { int tag_matches = set_size(m->tag_filter); @@ -839,23 +862,23 @@ _public_ int sd_device_monitor_filter_update(sd_device_monitor *m) { uint32_t tag_bloom_lo = tag_bloom_bits & 0xffffffff; /* load device bloom bits in A */ - bpf_stmt(ins, &i, BPF_LD|BPF_W|BPF_ABS, offsetof(monitor_netlink_header, filter_tag_bloom_hi)); + RET_GATHER(r, bpf_stmt(ins, &i, BPF_LD|BPF_W|BPF_ABS, offsetof(monitor_netlink_header, filter_tag_bloom_hi))); /* clear bits (tag bits & bloom bits) */ - bpf_stmt(ins, &i, BPF_ALU|BPF_AND|BPF_K, tag_bloom_hi); + RET_GATHER(r, bpf_stmt(ins, &i, BPF_ALU|BPF_AND|BPF_K, tag_bloom_hi)); /* jump to next tag if it does not match */ - bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, tag_bloom_hi, 0, 3); + RET_GATHER(r, bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, tag_bloom_hi, 0, 3)); /* load device bloom bits in A */ - bpf_stmt(ins, &i, BPF_LD|BPF_W|BPF_ABS, offsetof(monitor_netlink_header, filter_tag_bloom_lo)); + RET_GATHER(r, bpf_stmt(ins, &i, BPF_LD|BPF_W|BPF_ABS, offsetof(monitor_netlink_header, filter_tag_bloom_lo))); /* clear bits (tag bits & bloom bits) */ - bpf_stmt(ins, &i, BPF_ALU|BPF_AND|BPF_K, tag_bloom_lo); + RET_GATHER(r, bpf_stmt(ins, &i, BPF_ALU|BPF_AND|BPF_K, tag_bloom_lo)); /* jump behind end of tag match block if tag matches */ tag_matches--; - bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, tag_bloom_lo, 1 + (tag_matches * 6), 0); + RET_GATHER(r, bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, tag_bloom_lo, 1 + (tag_matches * 6), 0)); } /* nothing matched, drop packet */ - bpf_stmt(ins, &i, BPF_RET|BPF_K, 0); + RET_GATHER(r, bpf_stmt(ins, &i, BPF_RET|BPF_K, 0)); } /* add all subsystem matches */ @@ -864,33 +887,33 @@ _public_ int sd_device_monitor_filter_update(sd_device_monitor *m) { uint32_t hash = string_hash32(subsystem); /* load device subsystem value in A */ - bpf_stmt(ins, &i, BPF_LD|BPF_W|BPF_ABS, offsetof(monitor_netlink_header, filter_subsystem_hash)); + RET_GATHER(r, bpf_stmt(ins, &i, BPF_LD|BPF_W|BPF_ABS, offsetof(monitor_netlink_header, filter_subsystem_hash))); if (!devtype) { /* jump if subsystem does not match */ - bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, hash, 0, 1); + RET_GATHER(r, bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, hash, 0, 1)); } else { /* jump if subsystem does not match */ - bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, hash, 0, 3); + RET_GATHER(r, bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, hash, 0, 3)); /* load device devtype value in A */ - bpf_stmt(ins, &i, BPF_LD|BPF_W|BPF_ABS, offsetof(monitor_netlink_header, filter_devtype_hash)); + RET_GATHER(r, bpf_stmt(ins, &i, BPF_LD|BPF_W|BPF_ABS, offsetof(monitor_netlink_header, filter_devtype_hash))); /* jump if value does not match */ hash = string_hash32(devtype); - bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, hash, 0, 1); + RET_GATHER(r, bpf_jmp(ins, &i, BPF_JMP|BPF_JEQ|BPF_K, hash, 0, 1)); } /* matched, pass packet */ - bpf_stmt(ins, &i, BPF_RET|BPF_K, 0xffffffff); - - if (i+1 >= ELEMENTSOF(ins)) - return -E2BIG; + RET_GATHER(r, bpf_stmt(ins, &i, BPF_RET|BPF_K, 0xffffffff)); } /* nothing matched, drop packet */ - bpf_stmt(ins, &i, BPF_RET|BPF_K, 0); + RET_GATHER(r, bpf_stmt(ins, &i, BPF_RET|BPF_K, 0)); } /* matched, pass packet */ - bpf_stmt(ins, &i, BPF_RET|BPF_K, 0xffffffff); + RET_GATHER(r, bpf_stmt(ins, &i, BPF_RET|BPF_K, 0xffffffff)); + + if (r < 0) + return r; /* install filter */ filter = (struct sock_fprog) { diff --git a/src/libsystemd/sd-device/test-sd-device-monitor.c b/src/libsystemd/sd-device/test-sd-device-monitor.c index 8d88f3eb67..523cc7c6cd 100644 --- a/src/libsystemd/sd-device/test-sd-device-monitor.c +++ b/src/libsystemd/sd-device/test-sd-device-monitor.c @@ -13,6 +13,7 @@ #include "path-util.h" #include "socket-util.h" #include "stat-util.h" +#include "stdio-util.h" #include "string-util.h" #include "tests.h" #include "time-util.h" @@ -282,6 +283,21 @@ TEST(sd_device_monitor_filter_add_match_tag) { ASSERT_EQ(sd_event_loop(sd_device_monitor_get_event(monitor_client)), 100); } +TEST(sd_device_monitor_filter_update_bounds) { + _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *m = NULL; + + ASSERT_OK(device_monitor_new_full(&m, MONITOR_GROUP_NONE, -EBADF)); + + /* Each tag emits 6 BPF instructions into a fixed 512-entry array, too many must be refused */ + for (unsigned u = 0; u < 200; u++) { + char t[32]; + xsprintf(t, "tag%u", u); + ASSERT_OK(sd_device_monitor_filter_add_match_tag(m, t)); + } + + ASSERT_ERROR(sd_device_monitor_filter_update(m), E2BIG); +} + TEST(sd_device_monitor_filter_add_match_sysattr) { _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL; _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; From 87e57c52c16030e73ea323578034cd5b67e60da2 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 18:02:10 +0100 Subject: [PATCH 02/11] hashmap: honor the value destructor in set_ensure_consume() Sets store their element in the key slot but use a value destructor (DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR). On the reject/duplicate path set_ensure_consume() only checked free_key and otherwise called free(). Follow-up for fcc1d0315d335ba31d85d6023c79d6404c62e167 --- src/basic/hashmap.c | 3 +++ src/test/test-set.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index a2868e2455..89517b99f7 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -1330,6 +1330,9 @@ int set_ensure_consume(Set **s, const struct hash_ops *hash_ops, void *key) { if (r <= 0) { if (hash_ops && hash_ops->free_key) hash_ops->free_key(key); + else if (hash_ops && hash_ops->free_value) + /* Sets store their element in the key slot but may carry a value destructor. */ + hash_ops->free_value(key); else free(key); } diff --git a/src/test/test-set.c b/src/test/test-set.c index 4c1872d636..652b6837be 100644 --- a/src/test/test-set.c +++ b/src/test/test-set.c @@ -2,8 +2,10 @@ #include +#include "alloc-util.h" #include "random-util.h" #include "set.h" +#include "siphash24.h" #include "strv.h" #include "tests.h" @@ -51,6 +53,46 @@ TEST(set_free_with_hash_ops) { assert_se(items[3].seen == 0); } +static unsigned set_ensure_consume_freed = 0; + +typedef struct ValItem { + int key; +} ValItem; + +static void val_item_free(ValItem *i) { + set_ensure_consume_freed++; + free(i); +} + +static void val_item_hash_func(const ValItem *i, struct siphash *state) { + siphash24_compress_typesafe(i->key, state); +} + +static int val_item_compare_func(const ValItem *a, const ValItem *b) { + return CMP(a->key, b->key); +} + +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + val_item_hash_ops, ValItem, val_item_hash_func, val_item_compare_func, + ValItem, val_item_free); + +TEST(set_ensure_consume_value_destructor) { + _cleanup_set_free_ Set *s = NULL; + ValItem *a, *b; + + ASSERT_NOT_NULL(a = new0(ValItem, 1)); + a->key = 1; + ASSERT_OK_POSITIVE(set_ensure_consume(&s, &val_item_hash_ops, a)); + + ASSERT_NOT_NULL(b = new0(ValItem, 1)); + b->key = 1; /* same key, different pointer -> duplicate */ + + set_ensure_consume_freed = 0; + /* The rejected duplicate must be released via the value destructor, not plain free(). */ + ASSERT_OK_ZERO(set_ensure_consume(&s, &val_item_hash_ops, b)); + ASSERT_EQ(set_ensure_consume_freed, 1u); +} + TEST(set_put) { _cleanup_set_free_ Set *m = NULL; From 7003a9bf098869e5622fe5d863199910afcdda6d Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 18:26:57 +0100 Subject: [PATCH 03/11] sd-device: avoid 32-bit overflow in the monitor properties bounds check "properties_off + 32 > n" is evaluated in 32-bit arithmetic, so a properties_off near UINT32_MAX wraps but passes the check. Follow-up for efbd4b3ca84c0426b6ff98d6352f82f3b7c090b2 --- .../sd-device/device-monitor-private.h | 21 +++++++++++++++ src/libsystemd/sd-device/device-monitor.c | 27 +++---------------- .../sd-device/test-sd-device-monitor.c | 26 ++++++++++++++++++ 3 files changed, 50 insertions(+), 24 deletions(-) diff --git a/src/libsystemd/sd-device/device-monitor-private.h b/src/libsystemd/sd-device/device-monitor-private.h index 4582108df6..dc1af6c19c 100644 --- a/src/libsystemd/sd-device/device-monitor-private.h +++ b/src/libsystemd/sd-device/device-monitor-private.h @@ -15,3 +15,24 @@ int device_monitor_new_full(sd_device_monitor **ret, MonitorNetlinkGroup group, int device_monitor_get_address(sd_device_monitor *m, union sockaddr_union *ret); int device_monitor_allow_unicast_sender(sd_device_monitor *m, sd_device_monitor *sender); int device_monitor_send(sd_device_monitor *m, const union sockaddr_union *destination, sd_device *device); + +#define UDEV_MONITOR_MAGIC 0xfeedcafe + +typedef struct monitor_netlink_header { + /* "libudev" prefix to distinguish libudev and kernel messages */ + char prefix[8]; + /* Magic to protect against daemon <-> Library message format mismatch + * Used in the kernel from socket filter rules; needs to be stored in network order */ + unsigned magic; + /* Total length of header structure known to the sender */ + unsigned header_size; + /* Properties string buffer */ + unsigned properties_off; + unsigned properties_len; + /* Hashes of primary device properties strings, to let libudev subscribers + * use in-kernel socket filters; values need to be stored in network order */ + unsigned filter_subsystem_hash; + unsigned filter_devtype_hash; + unsigned filter_tag_bloom_hi; + unsigned filter_tag_bloom_lo; +} monitor_netlink_header; diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index 7d9433920e..9799d9b4da 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -67,27 +67,6 @@ struct sd_device_monitor { void *userdata; }; -#define UDEV_MONITOR_MAGIC 0xfeedcafe - -typedef struct monitor_netlink_header { - /* "libudev" prefix to distinguish libudev and kernel messages */ - char prefix[8]; - /* Magic to protect against daemon <-> Library message format mismatch - * Used in the kernel from socket filter rules; needs to be stored in network order */ - unsigned magic; - /* Total length of header structure known to the sender */ - unsigned header_size; - /* Properties string buffer */ - unsigned properties_off; - unsigned properties_len; - /* Hashes of primary device properties strings, to let libudev subscribers - * use in-kernel socket filters; values need to be stored in network order */ - unsigned filter_subsystem_hash; - unsigned filter_devtype_hash; - unsigned filter_tag_bloom_hi; - unsigned filter_tag_bloom_lo; -} monitor_netlink_header; - static int monitor_set_nl_address(sd_device_monitor *m) { union sockaddr_union snl; socklen_t addrlen; @@ -642,10 +621,10 @@ _public_ int sd_device_monitor_receive(sd_device_monitor *m, sd_device **ret) { "Invalid message signature (%x != %x).", message.nlh->magic, htobe32(UDEV_MONITOR_MAGIC)); - if (message.nlh->properties_off + 32 > (size_t) n) + if (message.nlh->properties_off > LESS_BY((size_t) n, 32u)) return log_monitor_errno(m, SYNTHETIC_ERRNO(EAGAIN), - "Invalid offset for properties (%u > %zi).", - message.nlh->properties_off + 32, n); + "Invalid properties offset (%u) for message of length %zi.", + message.nlh->properties_off, n); offset = message.nlh->properties_off; diff --git a/src/libsystemd/sd-device/test-sd-device-monitor.c b/src/libsystemd/sd-device/test-sd-device-monitor.c index 523cc7c6cd..c4034d27c7 100644 --- a/src/libsystemd/sd-device/test-sd-device-monitor.c +++ b/src/libsystemd/sd-device/test-sd-device-monitor.c @@ -9,6 +9,7 @@ #include "device-private.h" #include "device-util.h" #include "io-util.h" +#include "iovec-util.h" #include "mountpoint-util.h" #include "path-util.h" #include "socket-util.h" @@ -424,6 +425,31 @@ TEST(sd_device_monitor_receive) { ASSERT_STREQ(s, syspath); } +TEST(sd_device_monitor_receive_bad_properties_off) { + _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL; + union sockaddr_union sa; + + prepare_monitor(&monitor_server, &monitor_client, &sa); + + monitor_netlink_header nlh = { + .prefix = "libudev", + .magic = htobe32(UDEV_MONITOR_MAGIC), + .header_size = sizeof nlh, + .properties_off = UINT32_MAX - 10, + }; + struct iovec iov = IOVEC_MAKE(&nlh, sizeof nlh); + struct msghdr smsg = { + .msg_iov = &iov, + .msg_iovlen = 1, + .msg_name = &sa, + .msg_namelen = sizeof(struct sockaddr_nl), + }; + ASSERT_OK_ERRNO(sendmsg(sd_device_monitor_get_fd(monitor_server), &smsg, 0)); + + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + ASSERT_ERROR(sd_device_monitor_receive(monitor_client, &dev), EAGAIN); +} + static int intro(void) { if (getuid() != 0) return log_tests_skipped("not root"); From a8d8009b7a0c2ec4f5d3097a0ae6fdb29fb0001c Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 18:37:24 +0100 Subject: [PATCH 04/11] udev: track remaining buffer size across $links devlinks FORMAT_SUBST_LINKS passes the same 'l' (remaining size) to strpcpy for each devlink and discards the returned remaining value, so the cursor advances while the bound stays at the full size, overflowing dest. Capture the return value. Follow-up for f6caab8995a27244db185f075e751a119e4bdedc --- src/udev/test-udev-format.c | 24 ++++++++++++++++++++++++ src/udev/udev-format.c | 4 ++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/udev/test-udev-format.c b/src/udev/test-udev-format.c index 1acd8db72b..b71e1e182a 100644 --- a/src/udev/test-udev-format.c +++ b/src/udev/test-udev-format.c @@ -1,9 +1,13 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "sd-device.h" + +#include "device-private.h" #include "log.h" #include "mountpoint-util.h" #include "string-util.h" #include "tests.h" +#include "udev-event.h" #include "udev-format.h" static void test_udev_resolve_subsys_kernel_one(const char *str, bool read_value, int retval, const char *expected) { @@ -36,6 +40,26 @@ TEST(udev_resolve_subsys_kernel) { test_udev_resolve_subsys_kernel_one("[net/lo]/address", true, 0, "00:00:00:00:00:00"); } +TEST(udev_event_apply_format_links) { + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + _cleanup_(udev_event_unrefp) UdevEvent *event = NULL; + char dest[64]; + bool truncated = false; + + ASSERT_OK(sd_device_new_from_syspath(&dev, "/sys/class/net/lo")); + + for (unsigned u = 0; u < 32; u++) { + _cleanup_free_ char *l = NULL; + ASSERT_OK(asprintf(&l, "/dev/link-that-is-long-%u", u)); + ASSERT_OK(device_add_devlink(dev, l)); + } + + ASSERT_NOT_NULL((event = udev_event_new(dev, NULL, EVENT_TEST_SPAWN))); + + udev_event_apply_format(event, "$links", dest, sizeof dest, false, &truncated); + ASSERT_TRUE(truncated); +} + static int intro(void) { if (path_is_mount_point("/sys") <= 0) return log_tests_skipped("/sys is not mounted"); diff --git a/src/udev/udev-format.c b/src/udev/udev-format.c index d670ee129f..c37ea0406f 100644 --- a/src/udev/udev-format.c +++ b/src/udev/udev-format.c @@ -334,9 +334,9 @@ static ssize_t udev_event_subst_format( case FORMAT_SUBST_LINKS: FOREACH_DEVICE_DEVLINK(dev, link) { if (s == dest) - strpcpy_full(&s, l, link + STRLEN("/dev/"), &truncated); + l = strpcpy_full(&s, l, link + STRLEN("/dev/"), &truncated); else - strpcpyl_full(&s, l, &truncated, " ", link + STRLEN("/dev/"), NULL); + l = strpcpyl_full(&s, l, &truncated, " ", link + STRLEN("/dev/"), NULL); if (truncated) break; } From 9cae6d9c565e820d9a5bf0b35c8b73b6004553bb Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 18:47:27 +0100 Subject: [PATCH 05/11] network: initialize dot_servers before CLEANUP_ARRAY If dns_resolvers_to_dot_addrs() fails (e.g. OOM) it returns without assigning *ret_addrs, so the CLEANUP_ARRAY() cleanup would free an uninitialized dot_servers pointer on the error return. Follow-up for 3fd6708cde0fa4283e2335aca288a40a49406f3c --- src/network/networkd-state-file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-state-file.c b/src/network/networkd-state-file.c index 3c505f39e6..f077ed5f4b 100644 --- a/src/network/networkd-state-file.c +++ b/src/network/networkd-state-file.c @@ -134,7 +134,7 @@ static int link_put_dns(Link *link, OrderedSet **s) { r = sd_dhcp_lease_get_dnr(link->dhcp_lease, &resolvers); if (r >= 0) { - struct in_addr_full **dot_servers; + struct in_addr_full **dot_servers = NULL; size_t n = 0; CLEANUP_ARRAY(dot_servers, n, in_addr_full_free_array); @@ -163,7 +163,7 @@ static int link_put_dns(Link *link, OrderedSet **s) { r = sd_dhcp6_lease_get_dnr(link->dhcp6_lease, &resolvers); if (r >= 0) { - struct in_addr_full **dot_servers; + struct in_addr_full **dot_servers = NULL; size_t n = 0; CLEANUP_ARRAY(dot_servers, n, in_addr_full_free_array); From 25c016f472b775cd6d8bcf7fd29506663cbf81de Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 18:58:11 +0100 Subject: [PATCH 06/11] sd-netlink: disconnect the slot on async/match error paths netlink_slot_allocate() links the slot into nl->slots and takes an nl reference, but sd_netlink_call_async() and netlink_add_match_internal() hold it in a _cleanup_free_, so an error after allocation frees the slot with a bare free() Follow-up for ee38400bbaf04ff093c5450a554d5cc70face217 --- src/libsystemd/sd-netlink/sd-netlink.c | 31 +++++++++++++++----------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/libsystemd/sd-netlink/sd-netlink.c b/src/libsystemd/sd-netlink/sd-netlink.c index 65d731a20b..8c2790e94b 100644 --- a/src/libsystemd/sd-netlink/sd-netlink.c +++ b/src/libsystemd/sd-netlink/sd-netlink.c @@ -548,7 +548,7 @@ int sd_netlink_call_async( uint64_t usec, const char *description) { - _cleanup_free_ sd_netlink_slot *slot = NULL; + _cleanup_(sd_netlink_slot_unrefp) sd_netlink_slot *slot = NULL; int r, k; assert_return(nl, -EINVAL); @@ -859,34 +859,39 @@ int netlink_add_match_internal( void *userdata, const char *description) { - _cleanup_free_ sd_netlink_slot *slot = NULL; + _cleanup_(sd_netlink_slot_unrefp) sd_netlink_slot *slot = NULL; int r; assert(groups); assert(n_groups > 0); - for (size_t i = 0; i < n_groups; i++) { - r = socket_broadcast_group_ref(nl, groups[i]); - if (r < 0) - return r; - } - r = netlink_slot_allocate(nl, !ret_slot, NETLINK_MATCH_CALLBACK, sizeof(struct match_callback), userdata, description, &slot); if (r < 0) return r; - slot->match_callback.groups = newdup(uint32_t, groups, n_groups); - if (!slot->match_callback.groups) - return -ENOMEM; - - slot->match_callback.n_groups = n_groups; slot->match_callback.callback = callback; slot->match_callback.type = type; slot->match_callback.cmd = cmd; + slot->match_callback.groups = newdup(uint32_t, groups, n_groups); + if (!slot->match_callback.groups) + return -ENOMEM; + + /* Link the callback before taking the broadcast group references, so that on a partial failure the + * cleanup path (sd_netlink_slot_unref() -> netlink_slot_disconnect()) can unlink it again and drop + * the references we already took. n_groups is bumped as each reference is acquired, so the disconnect + * only releases what we actually got. */ LIST_PREPEND(match_callbacks, nl->match_callbacks, &slot->match_callback); + for (size_t i = 0; i < n_groups; i++) { + r = socket_broadcast_group_ref(nl, groups[i]); + if (r < 0) + return r; + + slot->match_callback.n_groups = i + 1; + } + /* Set this at last. Otherwise, some failures in above call the destroy callback but some do not. */ slot->destroy_callback = destroy_callback; From 058b0de7bfda390cf08682b0179cf8f07685c0c3 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 19:26:23 +0100 Subject: [PATCH 07/11] sd-bus: re-check match_callbacks_modified in the argNhas value loop The CAN_HASH branch of bus_match_run() that iterates the array values of an argNhas= match dispatches a callback per value but, unlike the leaf and manual-iteration branches, does not re-check bus->match_callbacks_modified afterwards. A callback that adds or removes a match (freeing the node) would then have the next hashmap_get(node->compare.children, ...) read freed memory. Add the same check the sibling branches already have. Follow-up for 198b158f4941b817f26f8eb0ff75809bf5436496 --- src/libsystemd/sd-bus/bus-match.c | 3 ++ src/libsystemd/sd-bus/test-bus-match.c | 59 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/libsystemd/sd-bus/bus-match.c b/src/libsystemd/sd-bus/bus-match.c index f2521d21b0..c365c5f3d8 100644 --- a/src/libsystemd/sd-bus/bus-match.c +++ b/src/libsystemd/sd-bus/bus-match.c @@ -391,6 +391,9 @@ int bus_match_run( r = bus_match_run(bus, found, m); if (r != 0) return r; + + if (bus && bus->match_callbacks_modified) + return 0; } } diff --git a/src/libsystemd/sd-bus/test-bus-match.c b/src/libsystemd/sd-bus/test-bus-match.c index 58ba71e21f..31d7602c67 100644 --- a/src/libsystemd/sd-bus/test-bus-match.c +++ b/src/libsystemd/sd-bus/test-bus-match.c @@ -67,6 +67,63 @@ static void test_match_scope(const char *match, BusMatchScope scope) { assert_se(bus_match_get_scope(components, n_components) == scope); } +static sd_bus *modify_bus; +static BusMatchNode *modify_root; +static sd_bus_slot *modify_slots; + +static int strv_uaf_modify_filter(sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { + /* Remove both argNhas matches from within the callback (as bus_slot_disconnect() would), which + * frees the argNhas node that bus_match_run() is still iterating over the array values of. */ + assert_se(bus_match_remove(modify_root, &modify_slots[0].match_callback) >= 0); + assert_se(bus_match_remove(modify_root, &modify_slots[1].match_callback) >= 0); + modify_bus->match_callbacks_modified = true; + return 0; +} + +static int strv_uaf_noop_filter(sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { + return 0; +} + +static void add_cb_match(BusMatchNode *root, sd_bus_slot *slot, const char *match, sd_bus_message_handler_t cb) { + BusMatchComponent *components = NULL; + size_t n_components = 0; + + CLEANUP_ARRAY(components, n_components, bus_match_parse_free); + + assert_se(bus_match_parse(match, &components, &n_components) >= 0); + slot->n_ref = 1; /* bus_match_run() refs the slot around each callback */ + slot->match_callback.callback = cb; + assert_se(bus_match_add(root, components, n_components, &slot->match_callback) >= 0); +} + +/* Two argNhas= matches under the same node; the first callback frees that node. bus_match_run() must + * not dereference it for the next array value. */ +static void test_match_run_strv_modified(sd_bus *bus) { + BusMatchNode root = { .type = BUS_MATCH_ROOT }; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; + sd_bus_slot slots[2] = {}; + + modify_bus = bus; + modify_root = &root; + modify_slots = slots; + + add_cb_match(&root, &slots[0], "arg0has='pi'", strv_uaf_modify_filter); + add_cb_match(&root, &slots[1], "arg0has='pa'", strv_uaf_noop_filter); + + assert_se(sd_bus_message_new_signal(bus, &m, "/", "a.b", "c") >= 0); + assert_se(sd_bus_message_append(m, "as", 2, "pi", "pa") >= 0); + assert_se(sd_bus_message_seal(m, 1, 0) >= 0); + + /* Let the leaf gating pass for our synthetic message/matches. */ + m->read_counter = 1; + bus->iteration_counter = 1; + bus->match_callbacks_modified = false; + + assert_se(bus_match_run(bus, &root, m) == 0); + + bus_match_free(&root); +} + int main(int argc, char *argv[]) { BusMatchNode root = { .type = BUS_MATCH_ROOT, @@ -142,5 +199,7 @@ int main(int argc, char *argv[]) { test_match_scope("member='gurke',path='/org/freedesktop/DBus/Local'", BUS_MATCH_LOCAL); test_match_scope("arg2='piep',sender='org.freedesktop.DBus',member='waldo'", BUS_MATCH_DRIVER); + test_match_run_strv_modified(bus); + return 0; } From b7b13032092c93a0ee5457beeabf65808c8a1433 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 20:08:14 +0100 Subject: [PATCH 08/11] sd-bus: drop half-registered vtable members on failure add_object_vtable_internal() inserts BusVTableMember entries into the global vtable_methods/vtable_properties sets as it walks the vtable, but only sets node_vtable.node (which gates the disconnect-time cleanup) after the loop. A failure mid-loop thus left the already-inserted members behind, dangling once the slot's interface string and the node's path were freed, so a later registration reading those keys hit a use-after-free. Remove this slot's members on the fail path. The test asserts the failed registration leaves no member behind. Follow-up for 19befb2d5fc087f96e40ddc432b2cc9385666209 --- src/libsystemd/sd-bus/bus-objects.c | 31 ++++++++++++++++++++++++ src/libsystemd/sd-bus/test-bus-objects.c | 22 +++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index 795cc68837..aa371cceca 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -2099,6 +2099,37 @@ static int add_object_vtable_internal( return 0; fail: + /* Remove members we already inserted into the global tables before failing (identified by + * parent), so they don't dangle once the slot and node are freed below. */ + if (s && s->node_vtable.interface) + for (v = bus_vtable_next(vtable, vtable); v->type != _SD_BUS_VTABLE_END; v = bus_vtable_next(vtable, v)) { + BusVTableMember key, *x; + Set *set; + + if (v->type == _SD_BUS_VTABLE_METHOD) { + set = bus->vtable_methods; + key.member = v->x.method.member; + } else if (IN_SET(v->type, _SD_BUS_VTABLE_PROPERTY, _SD_BUS_VTABLE_WRITABLE_PROPERTY)) { + set = bus->vtable_properties; + key.member = v->x.property.member; + } else + continue; + + /* The entry that failed validation may carry a NULL member, which was never + * inserted and would crash the hashmap lookup below. Skip it. */ + if (isempty(key.member)) + continue; + + key.path = n->path; + key.interface = s->node_vtable.interface; + + x = set_get(set, &key); + if (x && x->parent == &s->node_vtable) { + assert_se(set_remove(set, &key) == x); + free(x); + } + } + sd_bus_slot_unref(s); bus_node_gc(bus, n); diff --git a/src/libsystemd/sd-bus/test-bus-objects.c b/src/libsystemd/sd-bus/test-bus-objects.c index ac33086a6f..5d8e772cb4 100644 --- a/src/libsystemd/sd-bus/test-bus-objects.c +++ b/src/libsystemd/sd-bus/test-bus-objects.c @@ -7,6 +7,7 @@ #include "bus-internal.h" #include "bus-message.h" #include "log.h" +#include "set.h" #include "strv.h" #include "tests.h" @@ -566,6 +567,25 @@ static int client(void *p) { return 0; } +static const sd_bus_vtable partial_vtable[] = { + SD_BUS_VTABLE_START(0), + SD_BUS_METHOD("Foo", NULL, NULL, NULL, 0), + SD_BUS_METHOD("has space", NULL, NULL, NULL, 0), /* invalid member name, registration fails here */ + SD_BUS_VTABLE_END +}; + +static void test_vtable_partial_failure(void) { + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + + ASSERT_OK(sd_bus_new(&bus)); + + /* Registration fails at the invalid member, after "Foo" was already inserted into the global vtable + * member tables. Check that the fail path removes it again. */ + ASSERT_ERROR(sd_bus_add_object_vtable(bus, NULL, "/x", "org.test.iface", partial_vtable, NULL), EINVAL); + + ASSERT_TRUE(set_isempty(bus->vtable_methods)); +} + int main(int argc, char *argv[]) { _cleanup_(sd_event_unrefp) sd_event *e = NULL; _cleanup_(sd_future_unrefp) sd_future *f_server = NULL, *f_client = NULL; @@ -573,6 +593,8 @@ int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); + test_vtable_partial_failure(); + c.automatic_integer_property = 4711; ASSERT_NOT_NULL(c.automatic_string_property = strdup("dudeldu")); From 7eb7d46f9750d3baf9eaa9f3edb64d5c00fc25cf Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 20:55:53 +0100 Subject: [PATCH 09/11] udev: clear event back-pointer when freeing a worker worker_free() did not clear worker->event->worker, leaving an event that was still attached to a worker pointing at freed memory once the worker was gone. manager_free() frees the workers (hashmap_free()) before it walks manager->events to process them, and event_free() then writes event->worker->event = NULL into the already-freed worker. Clear the back-pointer in worker_free(), mirroring event_free() which already clears the worker's pointer. The link used to be torn down by the event_free(worker->event) call in worker_free(), which was dropped when events became reference counted. The Event and Worker definitions move to udev-manager.h (next to Manager) so the unit test can construct and free a bound worker/event pair. Follow-up for cb16d47f30cc0d848ca6868c39b844810520d713 --- src/udev/test-udev-manager.c | 19 +++++++++++ src/udev/udev-manager.c | 64 ++++-------------------------------- src/udev/udev-manager.h | 62 ++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 58 deletions(-) diff --git a/src/udev/test-udev-manager.c b/src/udev/test-udev-manager.c index d444b0bf00..7de5a37289 100644 --- a/src/udev/test-udev-manager.c +++ b/src/udev/test-udev-manager.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "alloc-util.h" #include "tests.h" #include "udev-manager.h" @@ -16,4 +17,22 @@ TEST(devpath_conflict) { "/devices/pci0000:00/0000:00:1c.4/0000:3c:00.0/nvme/nvme0/nvme0n1/nvme0n1p1")); } +TEST(worker_free_detaches_event) { + /* worker_free() must clear the back-pointer of the event it is processing, otherwise the event is + * left referencing the freed worker. This happens e.g. on manager_free(), which frees the workers + * before the events still referencing them. */ + + Worker *worker = ASSERT_PTR(new0(Worker, 1)); + worker->pidref = PIDREF_NULL; + + _cleanup_free_ Event *event = ASSERT_PTR(new0(Event, 1)); + + worker->event = event; + event->worker = worker; + + worker_free(worker); + + ASSERT_NULL(event->worker); +} + DEFINE_TEST_MAIN(LOG_DEBUG); diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index df13df6b50..fabbdbb88c 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -50,63 +50,6 @@ #define EVENT_REQUEUE_INTERVAL_USEC (200 * USEC_PER_MSEC) #define EVENT_REQUEUE_TIMEOUT_USEC (3 * USEC_PER_MINUTE) -typedef enum EventState { - EVENT_UNDEF, - EVENT_QUEUED, - EVENT_RUNNING, - EVENT_LOCKED, - EVENT_PROCESSED, -} EventState; - -typedef struct Event { - /* All events that have not been processed (state != EVENT_PROCESSED) are referenced by the Manager. - * Additionally, an event may be referenced by events blocked by this event. See event_find_blocker(). */ - unsigned n_ref; - - Manager *manager; - Worker *worker; - EventState state; - - sd_device *dev; - - sd_device_action_t action; - uint64_t seqnum; - const char *id; - const char *devpath; - const char *devpath_old; - const char *devnode; - - /* Used when the device is locked by another program. */ - usec_t requeue_next_usec; - usec_t requeue_timeout_usec; - unsigned locked_event_prioq_index; - char *whole_disk; - LIST_FIELDS(Event, same_disk); - - /* The last blocker for this event. This event must not be processed before the blocker is processed. */ - Event *blocker; - - LIST_FIELDS(Event, event); -} Event; - -typedef enum WorkerState { - WORKER_UNDEF, - WORKER_RUNNING, - WORKER_IDLE, - WORKER_KILLED, -} WorkerState; - -typedef struct Worker { - Manager *manager; - PidRef pidref; - sd_event_source *child_event_source; - sd_event_source *timeout_warning_event_source; - sd_event_source *timeout_kill_event_source; - union sockaddr_union address; - WorkerState state; - Event *event; -} Worker; - static void event_unset_whole_disk(Event *event) { Manager *manager = ASSERT_PTR(ASSERT_PTR(event)->manager); @@ -171,13 +114,18 @@ static Event* event_enter_processed(Event *event) { DEFINE_TRIVIAL_CLEANUP_FUNC(Event*, event_enter_processed); -static Worker* worker_free(Worker *worker) { +Worker* worker_free(Worker *worker) { if (!worker) return NULL; if (worker->manager) hashmap_remove(worker->manager->workers, &worker->pidref); + /* If an event is still attached, clear its back-pointer so it doesn't dangle (mirrors event_free(), + * which clears the worker's pointer). */ + if (worker->event) + worker->event->worker = NULL; + sd_event_source_disable_unref(worker->child_event_source); sd_event_source_unref(worker->timeout_warning_event_source); sd_event_source_unref(worker->timeout_kill_event_source); diff --git a/src/udev/udev-manager.h b/src/udev/udev-manager.h index c1b98c52d1..4e18dd5404 100644 --- a/src/udev/udev-manager.h +++ b/src/udev/udev-manager.h @@ -1,9 +1,12 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ #pragma once +#include "sd-device.h" #include "sd-event.h" #include "list.h" +#include "pidref.h" +#include "socket-util.h" #include "udev-config.h" #include "udev-forward.h" @@ -76,10 +79,69 @@ typedef struct Manager { bool exit; } Manager; +typedef enum EventState { + EVENT_UNDEF, + EVENT_QUEUED, + EVENT_RUNNING, + EVENT_LOCKED, + EVENT_PROCESSED, +} EventState; + +typedef struct Event { + /* All events that have not been processed (state != EVENT_PROCESSED) are referenced by the Manager. + * Additionally, an event may be referenced by events blocked by this event. See event_find_blocker(). */ + unsigned n_ref; + + Manager *manager; + Worker *worker; + EventState state; + + sd_device *dev; + + sd_device_action_t action; + uint64_t seqnum; + const char *id; + const char *devpath; + const char *devpath_old; + const char *devnode; + + /* Used when the device is locked by another program. */ + usec_t requeue_next_usec; + usec_t requeue_timeout_usec; + unsigned locked_event_prioq_index; + char *whole_disk; + LIST_FIELDS(Event, same_disk); + + /* The last blocker for this event. This event must not be processed before the blocker is processed. */ + Event *blocker; + + LIST_FIELDS(Event, event); +} Event; + +typedef enum WorkerState { + WORKER_UNDEF, + WORKER_RUNNING, + WORKER_IDLE, + WORKER_KILLED, +} WorkerState; + +typedef struct Worker { + Manager *manager; + PidRef pidref; + sd_event_source *child_event_source; + sd_event_source *timeout_warning_event_source; + sd_event_source *timeout_kill_event_source; + union sockaddr_union address; + WorkerState state; + Event *event; +} Worker; + Manager* manager_new(void); Manager* manager_free(Manager *manager); DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); +Worker* worker_free(Worker *worker); + int manager_main(Manager *manager); void manager_reload(Manager *manager, bool force); void manager_revert(Manager *manager); From 134df27257c8ff8b63fdd36dc0778f2229c7229b Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 22:06:25 +0100 Subject: [PATCH 10/11] network: implement refcounting for SR-IOV objects link_request_sr_iov_vfs() queued each request with a raw SRIOV* as userdata and a NULL free_func, so the request did not own the object. The SRIOV is owned by link->network->sr_iov_by_section, which is freed by network_free() on reload while the RTM_SETLINK reply may still be in flight (the floating netlink slot keeps the request alive but does not keep the userdata alive). A later reply, or the 25s netlink timeout, then dispatches sr_iov_handler() which reads the freed SRIOV. Follow-up for cb8453cc51a9d49e094e746af2e074669a71cc4a --- src/network/networkd-sriov.c | 5 ++++- src/shared/netif-sriov.c | 43 ++++++++++++++++++++++++++---------- src/shared/netif-sriov.h | 4 ++++ 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/network/networkd-sriov.c b/src/network/networkd-sriov.c index 4664425979..d35e108a4a 100644 --- a/src/network/networkd-sriov.c +++ b/src/network/networkd-sriov.c @@ -98,11 +98,12 @@ int link_request_sr_iov_vfs(Link *link) { if (!sr_iov_has_config(sr_iov, attr)) continue; + /* The request takes a reference on the configuration. */ r = link_queue_request_safe( link, _REQUEST_TYPE_SRIOV_BASE + attr, sr_iov, - NULL, + sr_iov_unref, sr_iov_hash_func, sr_iov_compare_func, sr_iov_process_request, @@ -113,6 +114,8 @@ int link_request_sr_iov_vfs(Link *link) { return log_link_warning_errno(link, r, "Failed to request to set up %s for SR-IOV virtual function %"PRIu32": %m", sr_iov_attribute_to_string(attr), sr_iov->vf); + if (r > 0) + sr_iov_ref(sr_iov); } } diff --git a/src/shared/netif-sriov.c b/src/shared/netif-sriov.c index f5df7fb392..79fd86c4f6 100644 --- a/src/shared/netif-sriov.c +++ b/src/shared/netif-sriov.c @@ -16,24 +16,42 @@ #include "string-table.h" #include "string-util.h" +static SRIOV* sr_iov_detach_impl(SRIOV *sr_iov) { + assert(sr_iov); + + if (!sr_iov->sr_iov_by_section) + return NULL; + + assert(sr_iov->section); + ordered_hashmap_remove(sr_iov->sr_iov_by_section, sr_iov->section); + sr_iov->sr_iov_by_section = NULL; + + return sr_iov; +} + static SRIOV* sr_iov_free(SRIOV *sr_iov) { if (!sr_iov) return NULL; - if (sr_iov->sr_iov_by_section && sr_iov->section) - ordered_hashmap_remove(sr_iov->sr_iov_by_section, sr_iov->section); + sr_iov_detach_impl(sr_iov); config_section_free(sr_iov->section); return mfree(sr_iov); } -DEFINE_SECTION_CLEANUP_FUNCTIONS(SRIOV, sr_iov_free); +DEFINE_TRIVIAL_REF_UNREF_FUNC(SRIOV, sr_iov, sr_iov_free); +DEFINE_SECTION_CLEANUP_FUNCTIONS(SRIOV, sr_iov_unref); + +static void sr_iov_detach(SRIOV *sr_iov) { + assert(sr_iov); + sr_iov_unref(sr_iov_detach_impl(sr_iov)); +} DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( sr_iov_hash_ops_by_section, ConfigSection, config_section_hash_func, config_section_compare_func, - SRIOV, sr_iov_free); + SRIOV, sr_iov_detach); static int sr_iov_new(SRIOV **ret) { SRIOV *sr_iov; @@ -45,6 +63,7 @@ static int sr_iov_new(SRIOV **ret) { return -ENOMEM; *sr_iov = (SRIOV) { + .n_ref = 1, .vf = UINT32_MAX, .vlan_proto = ETH_P_8021Q, .vf_spoof_check_setting = -1, @@ -60,7 +79,7 @@ static int sr_iov_new(SRIOV **ret) { static int sr_iov_new_static(OrderedHashmap **sr_iov_by_section, const char *filename, unsigned section_line, SRIOV **ret) { _cleanup_(config_section_freep) ConfigSection *n = NULL; - _cleanup_(sr_iov_freep) SRIOV *sr_iov = NULL; + _cleanup_(sr_iov_unrefp) SRIOV *sr_iov = NULL; SRIOV *existing = NULL; int r; @@ -368,7 +387,7 @@ int sr_iov_drop_invalid_sections(uint32_t num_vfs, OrderedHashmap *sr_iov_by_sec SRIOV *dup; if (sr_iov_section_verify(num_vfs, sr_iov) < 0) { - sr_iov_free(sr_iov); + sr_iov_detach(sr_iov); continue; } @@ -378,7 +397,7 @@ int sr_iov_drop_invalid_sections(uint32_t num_vfs, OrderedHashmap *sr_iov_by_sec "dropping the [SR-IOV] section specified at line %u.", dup->section->filename, sr_iov->section->line, dup->section->line, dup->section->line); - sr_iov_free(dup); + sr_iov_detach(dup); } r = set_ensure_put(&set, &sr_iov_hash_ops, sr_iov); @@ -402,7 +421,7 @@ int config_parse_sr_iov_uint32( void *data, void *userdata) { - _cleanup_(sr_iov_free_or_set_invalidp) SRIOV *sr_iov = NULL; + _cleanup_(sr_iov_unref_or_set_invalidp) SRIOV *sr_iov = NULL; OrderedHashmap **sr_iov_by_section = ASSERT_PTR(data); uint32_t k; int r; @@ -469,7 +488,7 @@ int config_parse_sr_iov_vlan_proto( void *data, void *userdata) { - _cleanup_(sr_iov_free_or_set_invalidp) SRIOV *sr_iov = NULL; + _cleanup_(sr_iov_unref_or_set_invalidp) SRIOV *sr_iov = NULL; OrderedHashmap **sr_iov_by_section = ASSERT_PTR(data); int r; @@ -507,7 +526,7 @@ int config_parse_sr_iov_link_state( void *data, void *userdata) { - _cleanup_(sr_iov_free_or_set_invalidp) SRIOV *sr_iov = NULL; + _cleanup_(sr_iov_unref_or_set_invalidp) SRIOV *sr_iov = NULL; OrderedHashmap **sr_iov_by_section = ASSERT_PTR(data); int r; @@ -558,7 +577,7 @@ int config_parse_sr_iov_boolean( void *data, void *userdata) { - _cleanup_(sr_iov_free_or_set_invalidp) SRIOV *sr_iov = NULL; + _cleanup_(sr_iov_unref_or_set_invalidp) SRIOV *sr_iov = NULL; OrderedHashmap **sr_iov_by_section = ASSERT_PTR(data); int r; @@ -615,7 +634,7 @@ int config_parse_sr_iov_mac( void *data, void *userdata) { - _cleanup_(sr_iov_free_or_set_invalidp) SRIOV *sr_iov = NULL; + _cleanup_(sr_iov_unref_or_set_invalidp) SRIOV *sr_iov = NULL; OrderedHashmap **sr_iov_by_section = ASSERT_PTR(data); int r; diff --git a/src/shared/netif-sriov.h b/src/shared/netif-sriov.h index cd2befc634..578a7ea579 100644 --- a/src/shared/netif-sriov.h +++ b/src/shared/netif-sriov.h @@ -27,6 +27,8 @@ typedef enum SRIOVLinkState { } SRIOVLinkState; typedef struct SRIOV { + unsigned n_ref; + ConfigSection *section; OrderedHashmap *sr_iov_by_section; @@ -43,6 +45,8 @@ typedef struct SRIOV { DECLARE_STRING_TABLE_LOOKUP_TO_STRING(sr_iov_attribute, SRIOVAttribute); +DECLARE_TRIVIAL_REF_UNREF_FUNC(SRIOV, sr_iov); + void sr_iov_hash_func(const SRIOV *sr_iov, struct siphash *state); int sr_iov_compare_func(const SRIOV *s1, const SRIOV *s2); bool sr_iov_has_config(SRIOV *sr_iov, SRIOVAttribute attr); From 46c15e88e0c2d78679670a60d3406e74de455036 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 2 Jul 2026 22:20:31 +0100 Subject: [PATCH 11/11] resolve: anchor the service browser from mDNS maintenance queries mdns_maintenance_query() takes a ref on the browser's varlink link but never installs itself as that link's userdata. dns_query_free() then unconditionally runs sd_varlink_set_userdata(varlink_request, NULL), so freeing any maintenance query wipes the browse query's registration on the shared sb->link slot, disabling the abort paths in vl_on_disconnect() and dns_service_browser_free(). The maintenance query also never takes a reference on the DnsServiceBrowser, so a client disconnect could free the browser while a maintenance query was still in flight, leaving the per-service schedule_event timer and the raw service->service_browser back-pointer dangling (use-after-free on the next timer tick or query completion). Take a service_browser_request reference instead, matching the browse query path. dns_query_free() already drops it. The browser now outlives its in-flight maintenance queries. Follow-up for 8458b7fb91ea5e5109b6f3c94f8a781a120c798b --- src/resolve/resolved-dns-browse-services.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-browse-services.c b/src/resolve/resolved-dns-browse-services.c index 68dcbee349..6a48089a59 100644 --- a/src/resolve/resolved-dns-browse-services.c +++ b/src/resolve/resolved-dns-browse-services.c @@ -126,7 +126,7 @@ static int mdns_maintenance_query(sd_event_source *s, uint64_t usec, void *userd return log_error_errno(r, "Failed to create mDNS query for maintenance: %m"); q->complete = mdns_maintenance_query_complete; - q->varlink_request = sd_varlink_ref(service->service_browser->link); + q->service_browser_request = dns_service_browser_ref(service->service_browser); q->dnsservice_request = dnssd_discovered_service_ref(service); /* Schedule the next maintenance query based on the TTL */