From 4674f407b2a96baaed4c8226342bccb313463717 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Sep 2023 16:51:25 +0900 Subject: [PATCH 1/4] sd-ndisc,sd-radv: fix use of uninitialized value When icmp6_receive() fails, then the source address is not initialized yet. --- src/libsystemd-network/sd-ndisc.c | 3 +-- src/libsystemd-network/sd-radv.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libsystemd-network/sd-ndisc.c b/src/libsystemd-network/sd-ndisc.c index ca5365b95f..8a6760188d 100644 --- a/src/libsystemd-network/sd-ndisc.c +++ b/src/libsystemd-network/sd-ndisc.c @@ -225,8 +225,7 @@ static int ndisc_recv(sd_event_source *s, int fd, uint32_t revents, void *userda if (r < 0) switch (r) { case -EADDRNOTAVAIL: - log_ndisc(nd, "Received RA from non-link-local address %s. Ignoring.", - IN6_ADDR_TO_STRING(&rt->address)); + log_ndisc(nd, "Received RA from non-link-local address. Ignoring."); return 0; case -EMULTIHOP: diff --git a/src/libsystemd-network/sd-radv.c b/src/libsystemd-network/sd-radv.c index cb7bc07f5a..afb0fd95b6 100644 --- a/src/libsystemd-network/sd-radv.c +++ b/src/libsystemd-network/sd-radv.c @@ -264,8 +264,7 @@ static int radv_recv(sd_event_source *s, int fd, uint32_t revents, void *userdat if (r < 0) switch (r) { case -EADDRNOTAVAIL: - log_radv(ra, "Received RS from non-link-local address %s. Ignoring", - IN6_ADDR_TO_STRING(&src)); + log_radv(ra, "Received RS from non-link-local address. Ignoring"); return 0; case -EMULTIHOP: From 512116383215c2f61172dd20e45871ff5d16c030 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Sep 2023 17:00:31 +0900 Subject: [PATCH 2/4] icmp6-util: several cleanups for icmp6_receive() - Rename the argument for storing the sender address, - allow to call it with NULL for ret_xyz, - reduce needless copy of timestamp when the message does not have timestamp. --- src/libsystemd-network/fuzz-ndisc-rs.c | 13 +++++++++---- src/libsystemd-network/icmp6-util.c | 20 ++++++++++++++------ src/libsystemd-network/icmp6-util.h | 8 ++++++-- src/libsystemd-network/test-ndisc-ra.c | 13 +++++++++---- src/libsystemd-network/test-ndisc-rs.c | 13 +++++++++---- 5 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src/libsystemd-network/fuzz-ndisc-rs.c b/src/libsystemd-network/fuzz-ndisc-rs.c index 7a5c45a2f9..5493538fec 100644 --- a/src/libsystemd-network/fuzz-ndisc-rs.c +++ b/src/libsystemd-network/fuzz-ndisc-rs.c @@ -24,12 +24,17 @@ int icmp6_bind_router_advertisement(int index) { return -ENOSYS; } -int icmp6_receive(int fd, void *iov_base, size_t iov_len, - struct in6_addr *dst, triple_timestamp *timestamp) { +int icmp6_receive( + int fd, + void *iov_base, + size_t iov_len, + struct in6_addr *ret_sender, + triple_timestamp *ret_timestamp) { + assert_se(read(fd, iov_base, iov_len) == (ssize_t) iov_len); - if (timestamp) - triple_timestamp_get(timestamp); + if (ret_timestamp) + triple_timestamp_get(ret_timestamp); return 0; } diff --git a/src/libsystemd-network/icmp6-util.c b/src/libsystemd-network/icmp6-util.c index ecddab61e4..46318505ba 100644 --- a/src/libsystemd-network/icmp6-util.c +++ b/src/libsystemd-network/icmp6-util.c @@ -144,8 +144,12 @@ int icmp6_send_router_solicitation(int s, const struct ether_addr *ether_addr) { return 0; } -int icmp6_receive(int fd, void *buffer, size_t size, struct in6_addr *ret_dst, - triple_timestamp *ret_timestamp) { +int icmp6_receive( + int fd, + void *buffer, + size_t size, + struct in6_addr *ret_sender, + triple_timestamp *ret_timestamp) { /* This needs to be initialized with zero. See #20741. */ CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int)) + /* ttl */ @@ -206,10 +210,14 @@ int icmp6_receive(int fd, void *buffer, size_t size, struct in6_addr *ret_dst, } } - if (!triple_timestamp_is_set(&t)) - triple_timestamp_get(&t); + if (ret_timestamp) { + if (triple_timestamp_is_set(&t)) + *ret_timestamp = t; + else + triple_timestamp_get(ret_timestamp); + } - *ret_dst = addr; - *ret_timestamp = t; + if (ret_sender) + *ret_sender = addr; return 0; } diff --git a/src/libsystemd-network/icmp6-util.h b/src/libsystemd-network/icmp6-util.h index f7ad26b5e6..0a9ecb4c44 100644 --- a/src/libsystemd-network/icmp6-util.h +++ b/src/libsystemd-network/icmp6-util.h @@ -20,5 +20,9 @@ int icmp6_bind_router_solicitation(int ifindex); int icmp6_bind_router_advertisement(int ifindex); int icmp6_send_router_solicitation(int s, const struct ether_addr *ether_addr); -int icmp6_receive(int fd, void *buffer, size_t size, struct in6_addr *ret_dst, - triple_timestamp *ret_timestamp); +int icmp6_receive( + int fd, + void *buffer, + size_t size, + struct in6_addr *ret_sender, + triple_timestamp *ret_timestamp); diff --git a/src/libsystemd-network/test-ndisc-ra.c b/src/libsystemd-network/test-ndisc-ra.c index bd8c0fd426..d3d96e776c 100644 --- a/src/libsystemd-network/test-ndisc-ra.c +++ b/src/libsystemd-network/test-ndisc-ra.c @@ -223,12 +223,17 @@ int icmp6_send_router_solicitation(int s, const struct ether_addr *ether_addr) { return 0; } -int icmp6_receive(int fd, void *iov_base, size_t iov_len, - struct in6_addr *dst, triple_timestamp *timestamp) { +int icmp6_receive( + int fd, + void *iov_base, + size_t iov_len, + struct in6_addr *ret_sender, + triple_timestamp *ret_timestamp) { + assert_se(read (fd, iov_base, iov_len) == (ssize_t)iov_len); - if (timestamp) - triple_timestamp_get(timestamp); + if (ret_timestamp) + triple_timestamp_get(ret_timestamp); return 0; } diff --git a/src/libsystemd-network/test-ndisc-rs.c b/src/libsystemd-network/test-ndisc-rs.c index e501b64377..e7bf3fab58 100644 --- a/src/libsystemd-network/test-ndisc-rs.c +++ b/src/libsystemd-network/test-ndisc-rs.c @@ -176,12 +176,17 @@ int icmp6_bind_router_advertisement(int ifindex) { return -ENOSYS; } -int icmp6_receive(int fd, void *iov_base, size_t iov_len, - struct in6_addr *dst, triple_timestamp *timestamp) { +int icmp6_receive( + int fd, + void *iov_base, + size_t iov_len, + struct in6_addr *ret_sender, + triple_timestamp *ret_timestamp) { + assert_se(read (fd, iov_base, iov_len) == (ssize_t)iov_len); - if (timestamp) - triple_timestamp_get(timestamp); + if (ret_timestamp) + triple_timestamp_get(ret_timestamp); return 0; } From 4961f5664647ee32fe9f2c8981cdd7852e511a09 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Sep 2023 17:44:45 +0900 Subject: [PATCH 3/4] icmp6-util: make icmp6_receive() accept the null source address Fixes #29050. --- src/libsystemd-network/fuzz-ndisc-rs.c | 10 ++++++++++ src/libsystemd-network/icmp6-util.c | 2 +- src/libsystemd-network/sd-ndisc.c | 7 ++++++- src/libsystemd-network/sd-radv.c | 5 ++++- src/libsystemd-network/test-ndisc-rs.c | 13 ++++++++++++- 5 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/libsystemd-network/fuzz-ndisc-rs.c b/src/libsystemd-network/fuzz-ndisc-rs.c index 5493538fec..ef5567329e 100644 --- a/src/libsystemd-network/fuzz-ndisc-rs.c +++ b/src/libsystemd-network/fuzz-ndisc-rs.c @@ -24,6 +24,13 @@ int icmp6_bind_router_advertisement(int index) { return -ENOSYS; } +static struct in6_addr dummy_link_local = { + .s6_addr = { + 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x12, 0x34, 0x56, 0xff, 0xfe, 0x78, 0x9a, 0xbc, + }, +}; + int icmp6_receive( int fd, void *iov_base, @@ -36,6 +43,9 @@ int icmp6_receive( if (ret_timestamp) triple_timestamp_get(ret_timestamp); + if (ret_sender) + *ret_sender = dummy_link_local; + return 0; } diff --git a/src/libsystemd-network/icmp6-util.c b/src/libsystemd-network/icmp6-util.c index 46318505ba..ecd0dd0f31 100644 --- a/src/libsystemd-network/icmp6-util.c +++ b/src/libsystemd-network/icmp6-util.c @@ -182,7 +182,7 @@ int icmp6_receive( sa.in6.sin6_family == AF_INET6) { addr = sa.in6.sin6_addr; - if (!in6_addr_is_link_local(&addr)) + if (!in6_addr_is_link_local(&addr) && !in6_addr_is_null(&addr)) return -EADDRNOTAVAIL; } else if (msg.msg_namelen > 0) diff --git a/src/libsystemd-network/sd-ndisc.c b/src/libsystemd-network/sd-ndisc.c index 8a6760188d..d7d9e7e2d9 100644 --- a/src/libsystemd-network/sd-ndisc.c +++ b/src/libsystemd-network/sd-ndisc.c @@ -225,7 +225,7 @@ static int ndisc_recv(sd_event_source *s, int fd, uint32_t revents, void *userda if (r < 0) switch (r) { case -EADDRNOTAVAIL: - log_ndisc(nd, "Received RA from non-link-local address. Ignoring."); + log_ndisc(nd, "Received RA from neither link-local nor null address. Ignoring."); return 0; case -EMULTIHOP: @@ -241,6 +241,11 @@ static int ndisc_recv(sd_event_source *s, int fd, uint32_t revents, void *userda 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(&rt->address)) + log_ndisc(nd, "Received RA from null address. Ignoring."); + (void) event_source_disable(nd->timeout_event_source); (void) ndisc_handle_datagram(nd, rt); return 0; diff --git a/src/libsystemd-network/sd-radv.c b/src/libsystemd-network/sd-radv.c index afb0fd95b6..5e25aab9b1 100644 --- a/src/libsystemd-network/sd-radv.c +++ b/src/libsystemd-network/sd-radv.c @@ -264,7 +264,7 @@ static int radv_recv(sd_event_source *s, int fd, uint32_t revents, void *userdat if (r < 0) switch (r) { case -EADDRNOTAVAIL: - log_radv(ra, "Received RS from non-link-local address. Ignoring"); + log_radv(ra, "Received RS from neither link-local nor null address. Ignoring"); return 0; case -EMULTIHOP: @@ -285,6 +285,9 @@ static int radv_recv(sd_event_source *s, int fd, uint32_t revents, void *userdat return 0; } + /* TODO: if the sender address is null, check that the message does not have the source link-layer + * address option. See RFC 4861 Section 6.1.1. */ + const char *addr = IN6_ADDR_TO_STRING(&src); r = radv_send(ra, &src, ra->lifetime_usec); diff --git a/src/libsystemd-network/test-ndisc-rs.c b/src/libsystemd-network/test-ndisc-rs.c index e7bf3fab58..baa9cb2696 100644 --- a/src/libsystemd-network/test-ndisc-rs.c +++ b/src/libsystemd-network/test-ndisc-rs.c @@ -41,7 +41,8 @@ static void router_dump(sd_ndisc_router *rt) { assert_se(rt); log_info("--"); - assert_se(sd_ndisc_router_get_address(rt, &addr) == -ENODATA); + assert_se(sd_ndisc_router_get_address(rt, &addr) >= 0); + log_info("Sender: %s", IN6_ADDR_TO_STRING(&addr)); assert_se(sd_ndisc_router_get_timestamp(rt, CLOCK_REALTIME, &t) >= 0); log_info("Timestamp: %s", FORMAT_TIMESTAMP(t)); @@ -176,6 +177,13 @@ int icmp6_bind_router_advertisement(int ifindex) { return -ENOSYS; } +static struct in6_addr dummy_link_local = { + .s6_addr = { + 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x12, 0x34, 0x56, 0xff, 0xfe, 0x78, 0x9a, 0xbc, + }, +}; + int icmp6_receive( int fd, void *iov_base, @@ -188,6 +196,9 @@ int icmp6_receive( if (ret_timestamp) triple_timestamp_get(ret_timestamp); + if (ret_sender) + *ret_sender = dummy_link_local; + return 0; } From 690afe79dd7612910abc6ac6d1fb247ed4db72b0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Sep 2023 17:38:10 +0900 Subject: [PATCH 4/4] test: extract dummy icmp6 utils for tests This extracts common implementation of dummy icmp6 utils used by tests. --- src/libsystemd-network/fuzz-ndisc-rs.c | 42 +------------------ src/libsystemd-network/icmp6-util-unix.c | 53 ++++++++++++++++++++++++ src/libsystemd-network/icmp6-util-unix.h | 9 ++++ src/libsystemd-network/meson.build | 15 +++++-- src/libsystemd-network/test-ndisc-ra.c | 33 +-------------- src/libsystemd-network/test-ndisc-rs.c | 51 +---------------------- 6 files changed, 77 insertions(+), 126 deletions(-) create mode 100644 src/libsystemd-network/icmp6-util-unix.c create mode 100644 src/libsystemd-network/icmp6-util-unix.h diff --git a/src/libsystemd-network/fuzz-ndisc-rs.c b/src/libsystemd-network/fuzz-ndisc-rs.c index ef5567329e..54dbfc1c76 100644 --- a/src/libsystemd-network/fuzz-ndisc-rs.c +++ b/src/libsystemd-network/fuzz-ndisc-rs.c @@ -9,50 +9,10 @@ #include "alloc-util.h" #include "fd-util.h" #include "fuzz.h" -#include "icmp6-util.h" +#include "icmp6-util-unix.h" #include "ndisc-internal.h" #include "socket-util.h" -static int test_fd[2] = PIPE_EBADF; - -int icmp6_bind_router_solicitation(int index) { - assert_se(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, test_fd) >= 0); - return test_fd[0]; -} - -int icmp6_bind_router_advertisement(int index) { - return -ENOSYS; -} - -static struct in6_addr dummy_link_local = { - .s6_addr = { - 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x12, 0x34, 0x56, 0xff, 0xfe, 0x78, 0x9a, 0xbc, - }, -}; - -int icmp6_receive( - int fd, - void *iov_base, - size_t iov_len, - struct in6_addr *ret_sender, - triple_timestamp *ret_timestamp) { - - assert_se(read(fd, iov_base, iov_len) == (ssize_t) iov_len); - - if (ret_timestamp) - triple_timestamp_get(ret_timestamp); - - if (ret_sender) - *ret_sender = dummy_link_local; - - return 0; -} - -int icmp6_send_router_solicitation(int s, const struct ether_addr *ether_addr) { - return 0; -} - int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { struct ether_addr mac_addr = { .ether_addr_octet = {'A', 'B', 'C', '1', '2', '3'} diff --git a/src/libsystemd-network/icmp6-util-unix.c b/src/libsystemd-network/icmp6-util-unix.c new file mode 100644 index 0000000000..bbb3bc8cc6 --- /dev/null +++ b/src/libsystemd-network/icmp6-util-unix.c @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include +#include + +#include "fd-util.h" +#include "icmp6-util-unix.h" + +send_ra_t send_ra_function = NULL; +int test_fd[2] = PIPE_EBADF; + +static struct in6_addr dummy_link_local = { + .s6_addr = { + 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x12, 0x34, 0x56, 0xff, 0xfe, 0x78, 0x9a, 0xbc, + }, +}; + +int icmp6_bind_router_solicitation(int ifindex) { + if (socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, test_fd) < 0) + return -errno; + + return test_fd[0]; +} + +int icmp6_bind_router_advertisement(int ifindex) { + return test_fd[1]; +} + +int icmp6_send_router_solicitation(int s, const struct ether_addr *ether_addr) { + if (!send_ra_function) + return 0; + + return send_ra_function(0); +} + +int icmp6_receive( + int fd, + void *iov_base, + size_t iov_len, + struct in6_addr *ret_sender, + triple_timestamp *ret_timestamp) { + + assert_se(read (fd, iov_base, iov_len) == (ssize_t) iov_len); + + if (ret_timestamp) + triple_timestamp_get(ret_timestamp); + + if (ret_sender) + *ret_sender = dummy_link_local; + + return 0; +} diff --git a/src/libsystemd-network/icmp6-util-unix.h b/src/libsystemd-network/icmp6-util-unix.h new file mode 100644 index 0000000000..a9cb05a96e --- /dev/null +++ b/src/libsystemd-network/icmp6-util-unix.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include "icmp6-util.h" + +typedef int (*send_ra_t)(uint8_t flags); + +extern send_ra_t send_ra_function; +extern int test_fd[2]; diff --git a/src/libsystemd-network/meson.build b/src/libsystemd-network/meson.build index 043d3bc254..6f62a80cb3 100644 --- a/src/libsystemd-network/meson.build +++ b/src/libsystemd-network/meson.build @@ -82,10 +82,16 @@ executables += [ 'sources' : files('test-lldp-rx.c'), }, network_test_template + { - 'sources' : files('test-ndisc-ra.c'), + 'sources' : files( + 'test-ndisc-ra.c', + 'icmp6-util-unix.c', + ), }, network_test_template + { - 'sources' : files('test-ndisc-rs.c'), + 'sources' : files( + 'test-ndisc-rs.c', + 'icmp6-util-unix.c', + ), }, network_test_template + { 'sources' : files('test-sd-dhcp-lease.c'), @@ -106,6 +112,9 @@ executables += [ 'sources' : files('fuzz-lldp-rx.c'), }, network_fuzz_template + { - 'sources' : files('fuzz-ndisc-rs.c'), + 'sources' : files( + 'fuzz-ndisc-rs.c', + 'icmp6-util-unix.c', + ), }, ] diff --git a/src/libsystemd-network/test-ndisc-ra.c b/src/libsystemd-network/test-ndisc-ra.c index d3d96e776c..18004f6466 100644 --- a/src/libsystemd-network/test-ndisc-ra.c +++ b/src/libsystemd-network/test-ndisc-ra.c @@ -11,7 +11,7 @@ #include "alloc-util.h" #include "hexdecoct.h" -#include "icmp6-util.h" +#include "icmp6-util-unix.h" #include "socket-util.h" #include "strv.h" #include "tests.h" @@ -52,7 +52,6 @@ static uint8_t advertisement[] = { }; static bool test_stopped; -static int test_fd[2]; static struct { struct in6_addr address; unsigned char prefixlen; @@ -208,36 +207,6 @@ TEST(radv) { assert_se(!ra); } -int icmp6_bind_router_solicitation(int ifindex) { - return -ENOSYS; -} - -int icmp6_bind_router_advertisement(int ifindex) { - assert_se(ifindex == 42); - - return test_fd[1]; -} - -int icmp6_send_router_solicitation(int s, const struct ether_addr *ether_addr) { - - return 0; -} - -int icmp6_receive( - int fd, - void *iov_base, - size_t iov_len, - struct in6_addr *ret_sender, - triple_timestamp *ret_timestamp) { - - assert_se(read (fd, iov_base, iov_len) == (ssize_t)iov_len); - - if (ret_timestamp) - triple_timestamp_get(ret_timestamp); - - return 0; -} - static int radv_recv(sd_event_source *s, int fd, uint32_t revents, void *userdata) { sd_radv *ra = userdata; unsigned char buf[168]; diff --git a/src/libsystemd-network/test-ndisc-rs.c b/src/libsystemd-network/test-ndisc-rs.c index baa9cb2696..09e292a2ec 100644 --- a/src/libsystemd-network/test-ndisc-rs.c +++ b/src/libsystemd-network/test-ndisc-rs.c @@ -12,7 +12,7 @@ #include "alloc-util.h" #include "fd-util.h" #include "hexdecoct.h" -#include "icmp6-util.h" +#include "icmp6-util-unix.h" #include "socket-util.h" #include "strv.h" #include "ndisc-internal.h" @@ -23,12 +23,8 @@ static struct ether_addr mac_addr = { }; static bool verbose = false; -static int test_fd[2]; static sd_ndisc *test_timeout_nd; -typedef int (*send_ra_t)(uint8_t flags); -static send_ra_t send_ra_function; - static void router_dump(sd_ndisc_router *rt) { struct in6_addr addr; uint8_t hop_limit; @@ -164,44 +160,6 @@ static void router_dump(sd_ndisc_router *rt) { } } -int icmp6_bind_router_solicitation(int ifindex) { - assert_se(ifindex == 42); - - if (socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, test_fd) < 0) - return -errno; - - return test_fd[0]; -} - -int icmp6_bind_router_advertisement(int ifindex) { - return -ENOSYS; -} - -static struct in6_addr dummy_link_local = { - .s6_addr = { - 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x12, 0x34, 0x56, 0xff, 0xfe, 0x78, 0x9a, 0xbc, - }, -}; - -int icmp6_receive( - int fd, - void *iov_base, - size_t iov_len, - struct in6_addr *ret_sender, - triple_timestamp *ret_timestamp) { - - assert_se(read (fd, iov_base, iov_len) == (ssize_t)iov_len); - - if (ret_timestamp) - triple_timestamp_get(ret_timestamp); - - if (ret_sender) - *ret_sender = dummy_link_local; - - return 0; -} - static int send_ra(uint8_t flags) { uint8_t advertisement[] = { 0x86, 0x00, 0xde, 0x83, 0x40, 0xc0, 0x00, 0xb4, @@ -230,13 +188,6 @@ static int send_ra(uint8_t flags) { return 0; } -int icmp6_send_router_solicitation(int s, const struct ether_addr *ether_addr) { - if (!send_ra_function) - return 0; - - return send_ra_function(0); -} - static void test_callback(sd_ndisc *nd, sd_ndisc_event_t event, sd_ndisc_router *rt, void *userdata) { sd_event *e = userdata; static unsigned idx = 0;