]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
networkd: fix race condition in per-interface ICMPv6 processing
authorPatrick Rohr <prohr@openai.com>
Mon, 4 May 2026 20:31:10 +0000 (13:31 -0700)
committerYu Watanabe <watanabe.yu+github@gmail.com>
Wed, 20 May 2026 01:26:39 +0000 (10:26 +0900)
There exists a small window of time in icmp6_bind() between creating the
ICMPv6 socket and binding it to an ifindex, where the link-scoped socket
can process an ICMPv6 packet received on any interface. The applies to
both sd-radv and sd-ndisc codepaths.

This change adds an explicit check for ifindex on the receive path and
ignores packets received on other interfaces.

Co-developed-by: OpenAI Codex <noreply@openai.com>
src/libsystemd-network/icmp6-packet.c
src/libsystemd-network/icmp6-packet.h
src/libsystemd-network/icmp6-test-util.c
src/libsystemd-network/icmp6-test-util.h
src/libsystemd-network/icmp6-util.c
src/libsystemd-network/icmp6-util.h
src/libsystemd-network/sd-ndisc.c
src/libsystemd-network/sd-radv.c
src/libsystemd-network/test-ndisc-ra.c
src/libsystemd-network/test-ndisc-rs.c

index b4e8e7f85147e9c82b15722688119c2b65ac7f6e..23e6582b67a0183008351ad7f5412a097c178c89 100644 (file)
@@ -112,7 +112,7 @@ int icmp6_packet_receive(int fd, ICMP6Packet **ret) {
         if (!p)
                 return -ENOMEM;
 
-        r = icmp6_receive(fd, p->raw_packet, p->raw_size, &p->sender_address, &p->timestamp);
+        r = icmp6_receive(fd, p->raw_packet, p->raw_size, &p->sender_address, &p->ifindex, &p->timestamp);
         if (r == -EADDRNOTAVAIL)
                 return log_debug_errno(r, "ICMPv6: Received a packet from neither link-local nor null address.");
         if (r == -EMULTIHOP)
index 929fff1f23c91aadace754e7e587afbd32278ca6..1b6f15727575cdce189b0f8ece52a0b15c0aff7f 100644 (file)
@@ -8,6 +8,7 @@
 
 typedef struct ICMP6Packet {
         unsigned n_ref;
+        int ifindex;
 
         struct in6_addr sender_address;
         struct triple_timestamp timestamp;
index 38b3537ebe0a2cb2247d3dd7e1322809487048f5..33315c438411378df982bd4ca1ea6dab3367b7b9 100644 (file)
@@ -9,6 +9,7 @@
 #include "time-util.h"
 
 int test_fd[2] = EBADF_PAIR;
+int test_ifindex = 42;
 
 static struct in6_addr dummy_link_local = {
         .s6_addr = {
@@ -18,6 +19,8 @@ static struct in6_addr dummy_link_local = {
 };
 
 int icmp6_bind(int ifindex, bool is_router) {
+        test_ifindex = ifindex;
+
         if (!is_router && socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, test_fd) < 0)
                 return -errno;
 
@@ -33,6 +36,7 @@ int icmp6_receive(
                 void *buffer,
                 size_t size,
                 struct in6_addr *ret_sender,
+                int *ret_ifindex,
                 triple_timestamp *ret_timestamp) {
 
         assert_se(read(fd, buffer, size) == (ssize_t) size);
@@ -43,5 +47,8 @@ int icmp6_receive(
         if (ret_sender)
                 *ret_sender = dummy_link_local;
 
+        if (ret_ifindex)
+                *ret_ifindex = test_ifindex;
+
         return 0;
 }
index 124cab555218a033a1b22500fcf5a30f0e5d87bd..8e9f17eb952fbd47450ee34dece01238d420fc9b 100644 (file)
@@ -2,3 +2,4 @@
 #pragma once
 
 extern int test_fd[2];
+extern int test_ifindex;
index 4347f8b723bf00739a8010d4ca1d436ef3c2a7b3..6df78bd2eed767aa25b338498932daecf8bb4f6a 100644 (file)
@@ -72,6 +72,10 @@ int icmp6_bind(int ifindex, bool is_router) {
         if (r < 0)
                 return r;
 
+        r = socket_set_recvpktinfo(s, AF_INET6, true);
+        if (r < 0)
+                return r;
+
         r = setsockopt_int(s, SOL_SOCKET, SO_TIMESTAMP, true);
         if (r < 0)
                 return r;
@@ -108,11 +112,13 @@ int icmp6_receive(
                 void *buffer,
                 size_t size,
                 struct in6_addr *ret_sender,
+                int *ret_ifindex,
                 triple_timestamp *ret_timestamp) {
 
         /* This needs to be initialized with zero. See #20741.
          * The issue is fixed on glibc-2.35 (8fba672472ae0055387e9315fc2eddfa6775ca79). */
         CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int)) + /* ttl */
+                         CMSG_SPACE(sizeof(struct in6_pktinfo)) +
                          CMSG_SPACE_TIMEVAL) control = {};
         struct iovec iov = { buffer, size };
         union sockaddr_union sa = {};
@@ -145,6 +151,10 @@ int icmp6_receive(
         if (hops && *hops != 255)
                 return -EMULTIHOP;
 
+        struct in6_pktinfo *pktinfo = CMSG_FIND_DATA(&msg, IPPROTO_IPV6, IPV6_PKTINFO, struct in6_pktinfo);
+        if (ret_ifindex)
+                *ret_ifindex = pktinfo ? pktinfo->ipi6_ifindex : 0;
+
         if (ret_timestamp)
                 triple_timestamp_from_cmsg(ret_timestamp, &msg);
         if (ret_sender)
index a1183724b289c70f0100cae700375c602ca03cc2..d23e0fe0eb3cbe1c88393a0389b277d798c1fb6a 100644 (file)
@@ -28,4 +28,5 @@ int icmp6_receive(
                 void *buffer,
                 size_t size,
                 struct in6_addr *ret_sender,
+                int *ret_ifindex,
                 triple_timestamp *ret_timestamp);
index 30c0de70047e2ebd4da14e335c4d7d5476ad5606..b6a55a5095547f4f1cf848d5bb21c5edd6cf9a59 100644 (file)
@@ -348,6 +348,12 @@ static int ndisc_recv(sd_event_source *s, int fd, uint32_t revents, void *userda
                 return 0;
         }
 
+        if (packet->ifindex != nd->ifindex) {
+                log_ndisc(nd, "Received an ICMPv6 packet on interface %i, expected %i, ignoring.",
+                          packet->ifindex, nd->ifindex);
+                return 0;
+        }
+
         /* The function icmp6_receive() accepts the null source address, but RFC 4861 Section 6.1.2 states
          * that hosts MUST discard messages with the null source address. */
         if (in6_addr_is_null(&packet->sender_address)) {
index 21a80da38ee9095fa0b2da09b45b7b418f484a96..948f7bc382cd64c418089736c7c79f9b25368d88 100644 (file)
@@ -226,6 +226,12 @@ static int radv_recv(sd_event_source *s, int fd, uint32_t revents, void *userdat
                 return 0;
         }
 
+        if (packet->ifindex != ra->ifindex) {
+                log_radv(ra, "Received an ICMPv6 packet on interface %i, expected %i, ignoring.",
+                         packet->ifindex, ra->ifindex);
+                return 0;
+        }
+
         (void) radv_process_packet(ra, packet);
         return 0;
 }
index 84dc73e84fa5f7bd6c83db6686a288dfaeaf549b..2ad42b5285f708507b3f1b55b27a89980f9132b3 100644 (file)
@@ -12,6 +12,7 @@
 #include "sd-radv.h"
 
 #include "alloc-util.h"
+#include "fd-util.h"
 #include "icmp6-test-util.h"
 #include "in-addr-util.h"
 #include "radv-internal.h"
@@ -245,6 +246,38 @@ static int radv_recv(sd_event_source *s, int fd, uint32_t revents, void *userdat
         return 0;
 }
 
+TEST(ra_ifindex_mismatch) {
+        static const struct nd_router_solicit rs = {
+                .nd_rs_type = ND_ROUTER_SOLICIT,
+        };
+
+        _cleanup_free_ uint8_t *buf = NULL;
+        _cleanup_(sd_event_unrefp) sd_event *e = NULL;
+        _cleanup_(sd_radv_unrefp) sd_radv *ra = NULL;
+        ssize_t buflen;
+
+        assert_se(socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, test_fd) >= 0);
+
+        assert_se(sd_event_new(&e) >= 0);
+        assert_se(sd_radv_new(&ra) >= 0);
+        assert_se(sd_radv_attach_event(ra, e, 0) >= 0);
+        assert_se(sd_radv_set_ifindex(ra, 42) >= 0);
+        assert_se(sd_radv_start(ra) >= 0);
+
+        assert_se(sd_event_run(e, UINT64_MAX) >= 0);
+        assert_se((buflen = next_datagram_size_fd(test_fd[0])) >= 0);
+        assert_se(buf = new(uint8_t, buflen));
+        assert_se(read(test_fd[0], buf, buflen) == buflen);
+
+        test_ifindex = 43;
+        assert_se(write(test_fd[0], &rs, sizeof(rs)) == sizeof(rs));
+        assert_se(sd_event_run(e, UINT64_MAX) >= 0);
+        assert_se(next_datagram_size_fd(test_fd[0]) == -EAGAIN);
+
+        assert_se(sd_radv_stop(ra) >= 0);
+        test_fd[0] = safe_close(test_fd[0]);
+}
+
 TEST(ra) {
         _cleanup_(sd_event_unrefp) sd_event *e = NULL;
         _cleanup_(sd_event_source_unrefp) sd_event_source *recv_router_advertisement = NULL;
index 51adc4e685316f597f08189b98077cb450a178ca..9c1e6f03a4c7f87ab218708822363cc7787b9616 100644 (file)
@@ -227,6 +227,18 @@ static void test_callback_ra(sd_ndisc *nd, sd_ndisc_event_t event, void *message
         sd_event_exit(e, 0);
 }
 
+static void test_callback_count(
+                sd_ndisc *nd,
+                sd_ndisc_event_t event,
+                void *message,
+                void *userdata) {
+
+        unsigned *count = ASSERT_PTR(userdata);
+
+        if (event == SD_NDISC_EVENT_ROUTER)
+                (*count)++;
+}
+
 static int on_recv_rs(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
         _cleanup_(icmp6_packet_unrefp) ICMP6Packet *packet = NULL;
         assert_se(icmp6_packet_receive(fd, &packet) >= 0);
@@ -234,6 +246,27 @@ static int on_recv_rs(sd_event_source *s, int fd, uint32_t revents, void *userda
         return send_ra(0);
 }
 
+TEST(rs_ifindex_mismatch) {
+        _cleanup_(sd_event_unrefp) sd_event *e = NULL;
+        _cleanup_(sd_ndisc_unrefp) sd_ndisc *nd = NULL;
+        unsigned count = 0;
+
+        assert_se(sd_event_new(&e) >= 0);
+        assert_se(sd_ndisc_new(&nd) >= 0);
+        assert_se(sd_ndisc_attach_event(nd, e, 0) >= 0);
+        assert_se(sd_ndisc_set_ifindex(nd, 42) >= 0);
+        assert_se(sd_ndisc_set_callback(nd, test_callback_count, &count) >= 0);
+        assert_se(sd_ndisc_start(nd) >= 0);
+
+        test_ifindex = 43;
+        send_ra(0);
+        assert_se(sd_event_run(e, UINT64_MAX) >= 0);
+        assert_se(count == 0);
+
+        assert_se(sd_ndisc_stop(nd) >= 0);
+        test_fd[1] = safe_close(test_fd[1]);
+}
+
 TEST(rs) {
         _cleanup_(sd_event_unrefp) sd_event *e = NULL;
         _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL;