From: Luca Boccassi Date: Thu, 2 Jul 2026 16:55:37 +0000 (+0100) Subject: sd-device: bound the tag filter BPF program size X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2f72db10d9a0cdf3c1b22044df44c2e9fd61ff1b;p=thirdparty%2Fsystemd.git 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 --- diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index 962329a865e..7d9433920ec 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 8d88f3eb672..523cc7c6cd1 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;