* [PATCH 1/1] ipv6: Check AF_UNSPEC in ip6_route_multipath_add()
@ 2025-08-04 20:42 Maksimilijan Marosevic
2025-08-05 16:16 ` David Ahern
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Maksimilijan Marosevic @ 2025-08-04 20:42 UTC (permalink / raw)
To: davem, dsahern
Cc: netdev, linux-kernel, linux-kernel-mentees,
Maksimilijan Marosevic, syzbot+a259a17220263c2d73fc
This check was removed in commit e6f497955fb6 ("ipv6: Check GATEWAY
in rtm_to_fib6_multipath_config().") as part of rt6_qualify_for ecmp().
The author correctly recognises that rt6_qualify_for_ecmp() returns
false if fb_nh_gw_family is set to AF_UNSPEC, but then mistakes
AF_UNSPEC for AF_INET6 when reasoning that the check is unnecessary.
This means certain malformed entries don't get caught in
ip6_route_multipath_add().
This patch reintroduces the AF_UNSPEC check while respecting changes
of the initial patch.
Reported-by: syzbot+a259a17220263c2d73fc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a259a17220263c2d73fc
Fixes: e6f497955fb6 ("ipv6: Check GATEWAY in rtm_to_fib6_multipath_config().")
Signed-off-by: Maksimilijan Marosevic <maksimilijan.marosevic@proton.me>
---
net/ipv6/route.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 3299cfa12e21..d4b988bed920 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -5456,6 +5456,14 @@ static int ip6_route_multipath_add(struct fib6_config *cfg,
goto cleanup;
}
+ if (rt->fib6_nh->fib_nh_gw_family == AF_UNSPEC) {
+ err = -EINVAL;
+ NL_SET_ERR_MSG(extack,
+ "Device only routes can not be added for IPv6 using the multipath API.");
+ fib6_info_release(rt);
+ goto cleanup;
+ }
+
rt->fib6_nh->fib_nh_weight = rtnh->rtnh_hops + 1;
err = ip6_route_info_append(&rt6_nh_list, rt, &r_cfg);
--
2.50.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] ipv6: Check AF_UNSPEC in ip6_route_multipath_add()
2025-08-04 20:42 [PATCH 1/1] ipv6: Check AF_UNSPEC in ip6_route_multipath_add() Maksimilijan Marosevic
@ 2025-08-05 16:16 ` David Ahern
2025-08-12 8:16 ` Paolo Abeni
2025-09-04 16:03 ` Jakub Kicinski
2 siblings, 0 replies; 8+ messages in thread
From: David Ahern @ 2025-08-05 16:16 UTC (permalink / raw)
To: Maksimilijan Marosevic, davem
Cc: netdev, linux-kernel, linux-kernel-mentees,
syzbot+a259a17220263c2d73fc
On 8/4/25 2:42 PM, Maksimilijan Marosevic wrote:
> This check was removed in commit e6f497955fb6 ("ipv6: Check GATEWAY
> in rtm_to_fib6_multipath_config().") as part of rt6_qualify_for ecmp().
> The author correctly recognises that rt6_qualify_for_ecmp() returns
> false if fb_nh_gw_family is set to AF_UNSPEC, but then mistakes
> AF_UNSPEC for AF_INET6 when reasoning that the check is unnecessary.
> This means certain malformed entries don't get caught in
> ip6_route_multipath_add().
>
> This patch reintroduces the AF_UNSPEC check while respecting changes
> of the initial patch.
>
> Reported-by: syzbot+a259a17220263c2d73fc@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a259a17220263c2d73fc
> Fixes: e6f497955fb6 ("ipv6: Check GATEWAY in rtm_to_fib6_multipath_config().")
> Signed-off-by: Maksimilijan Marosevic <maksimilijan.marosevic@proton.me>
> ---
> net/ipv6/route.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 3299cfa12e21..d4b988bed920 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -5456,6 +5456,14 @@ static int ip6_route_multipath_add(struct fib6_config *cfg,
> goto cleanup;
> }
>
> + if (rt->fib6_nh->fib_nh_gw_family == AF_UNSPEC) {
> + err = -EINVAL;
> + NL_SET_ERR_MSG(extack,
> + "Device only routes can not be added for IPv6 using the multipath API.");
> + fib6_info_release(rt);
> + goto cleanup;
> + }
> +
> rt->fib6_nh->fib_nh_weight = rtnh->rtnh_hops + 1;
>
> err = ip6_route_info_append(&rt6_nh_list, rt, &r_cfg);
can you add another test to the routing selftests to cover this case?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] ipv6: Check AF_UNSPEC in ip6_route_multipath_add()
2025-08-04 20:42 [PATCH 1/1] ipv6: Check AF_UNSPEC in ip6_route_multipath_add() Maksimilijan Marosevic
2025-08-05 16:16 ` David Ahern
@ 2025-08-12 8:16 ` Paolo Abeni
2025-09-04 16:03 ` Jakub Kicinski
2 siblings, 0 replies; 8+ messages in thread
From: Paolo Abeni @ 2025-08-12 8:16 UTC (permalink / raw)
To: Maksimilijan Marosevic, davem, dsahern
Cc: netdev, linux-kernel, linux-kernel-mentees,
syzbot+a259a17220263c2d73fc
On 8/4/25 10:42 PM, Maksimilijan Marosevic wrote:
> This check was removed in commit e6f497955fb6 ("ipv6: Check GATEWAY
> in rtm_to_fib6_multipath_config().") as part of rt6_qualify_for ecmp().
> The author correctly recognises that rt6_qualify_for_ecmp() returns
> false if fb_nh_gw_family is set to AF_UNSPEC, but then mistakes
> AF_UNSPEC for AF_INET6 when reasoning that the check is unnecessary.
> This means certain malformed entries don't get caught in
> ip6_route_multipath_add().
>
> This patch reintroduces the AF_UNSPEC check while respecting changes
> of the initial patch.
>
> Reported-by: syzbot+a259a17220263c2d73fc@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a259a17220263c2d73fc
> Fixes: e6f497955fb6 ("ipv6: Check GATEWAY in rtm_to_fib6_multipath_config().")
> Signed-off-by: Maksimilijan Marosevic <maksimilijan.marosevic@proton.me>
Please resend in a 2 patches series including an additional self-test as
asked by David.
Also please insert into the subj prefix the target tree ('net' in this
case) and add Kuniyuki into the CC list.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] ipv6: Check AF_UNSPEC in ip6_route_multipath_add()
2025-08-04 20:42 [PATCH 1/1] ipv6: Check AF_UNSPEC in ip6_route_multipath_add() Maksimilijan Marosevic
2025-08-05 16:16 ` David Ahern
2025-08-12 8:16 ` Paolo Abeni
@ 2025-09-04 16:03 ` Jakub Kicinski
2025-09-05 22:12 ` Maksimilijan Marošević
2 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2025-09-04 16:03 UTC (permalink / raw)
To: Maksimilijan Marosevic
Cc: davem, dsahern, netdev, linux-kernel, linux-kernel-mentees,
syzbot+a259a17220263c2d73fc
On Mon, 04 Aug 2025 20:42:53 +0000 Maksimilijan Marosevic wrote:
> This check was removed in commit e6f497955fb6 ("ipv6: Check GATEWAY
> in rtm_to_fib6_multipath_config().") as part of rt6_qualify_for ecmp().
> The author correctly recognises that rt6_qualify_for_ecmp() returns
> false if fb_nh_gw_family is set to AF_UNSPEC, but then mistakes
> AF_UNSPEC for AF_INET6 when reasoning that the check is unnecessary.
> This means certain malformed entries don't get caught in
> ip6_route_multipath_add().
>
> This patch reintroduces the AF_UNSPEC check while respecting changes
> of the initial patch.
Hi Maksimilijan!
Are you planning to repost this with a test?
If not I suppose we can enlist the author of the commit to help with
the selftest..
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] ipv6: Check AF_UNSPEC in ip6_route_multipath_add()
2025-09-04 16:03 ` Jakub Kicinski
@ 2025-09-05 22:12 ` Maksimilijan Marošević
2025-09-28 20:47 ` [PATCH v2 1/2] selftests/net: add netdevsim.c Maksimilijan Marosevic
2025-09-28 20:47 ` [PATCH v2 2/2] ipv6: Check AF_UNSPEC in ip6_route_multipath_add() Maksimilijan Marosevic
0 siblings, 2 replies; 8+ messages in thread
From: Maksimilijan Marošević @ 2025-09-05 22:12 UTC (permalink / raw)
To: kuba@kernel.org
Cc: davem@davemloft.net, dsahern@kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
syzbot+a259a17220263c2d73fc@syzkaller.appspotmail.com
Hi Jakub,
Sorry for the delay.
I've managed to devise a test-case that reliably reproduces the error, but I now believe the fix can be implemented better. Unfortunately, I haven't had the chance to work on the new fix these past couple of weeks.
I'll try to finish it this weekend and then run it through syzbot once more.
Maksimilijan
-------- Original Message --------
On 04/09/2025 18:03, Jakub Kicinski <kuba@kernel.org> wrote:
> On Mon, 04 Aug 2025 20:42:53 +0000 Maksimilijan Marosevic wrote:
> > This check was removed in commit e6f497955fb6 ("ipv6: Check GATEWAY
> > in rtm_to_fib6_multipath_config().") as part of rt6_qualify_for ecmp().
> > The author correctly recognises that rt6_qualify_for_ecmp() returns
> > false if fb_nh_gw_family is set to AF_UNSPEC, but then mistakes
> > AF_UNSPEC for AF_INET6 when reasoning that the check is unnecessary.
> > This means certain malformed entries don't get caught in
> > ip6_route_multipath_add().
> >
> > This patch reintroduces the AF_UNSPEC check while respecting changes
> > of the initial patch.
>
> Hi Maksimilijan!
>
> Are you planning to repost this with a test?
> If not I suppose we can enlist the author of the commit to help with
> the selftest..
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] selftests/net: add netdevsim.c
2025-09-05 22:12 ` Maksimilijan Marošević
@ 2025-09-28 20:47 ` Maksimilijan Marosevic
2025-09-30 14:13 ` Paolo Abeni
2025-09-28 20:47 ` [PATCH v2 2/2] ipv6: Check AF_UNSPEC in ip6_route_multipath_add() Maksimilijan Marosevic
1 sibling, 1 reply; 8+ messages in thread
From: Maksimilijan Marosevic @ 2025-09-28 20:47 UTC (permalink / raw)
To: kuba, pabeni
Cc: Maksimilijan Marosevic, davem, dsahern, kuniyu, kuniyu, netdev,
linux-kselftest, shuah, syzbot+a259a17220263c2d73fc
Tests an edge case in the nsim module where gw_family == AF_UNSPEC.
Works by creating a new nsim device and then sending a multipath
path message to it and loopback. In unpatched kernels, this triggers
a WARN_ON_ONCE in netdevsim/fib.c.
Reported-by: syzbot+a259a17220263c2d73fc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a259a17220263c2d73fc
Fixes: e6f497955fb6 ("ipv6: Check GATEWAY in rtm_to_fib6_multipath_config().")
Signed-off-by: Maksimilijan Marosevic <maksimilijan.marosevic@proton.me>
---
tools/testing/selftests/net/netdevsim.c | 391 ++++++++++++++++++++++++
1 file changed, 391 insertions(+)
create mode 100644 tools/testing/selftests/net/netdevsim.c
diff --git a/tools/testing/selftests/net/netdevsim.c b/tools/testing/selftests/net/netdevsim.c
new file mode 100644
index 000000000000..cdc8ebef4dac
--- /dev/null
+++ b/tools/testing/selftests/net/netdevsim.c
@@ -0,0 +1,391 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This test creates a new netdevsim device and then sends
+ * an IPv6 multipath netlink message to it and the loopback
+ * interface.
+ *
+ * This triggers an edge case where the routing table is
+ * constructed with an entry where gw_family = AF_UNSPEC.
+ * If not caught, this causes an unexpected nsiblings count
+ * in netdevsim/fib.c: nsim_fib6_event_init(), raising a
+ * warning.
+ *
+ * NOTE: The warning in question is raised by WARN_ON_ONCE.
+ * Therefore, this test reports a false negative if the
+ * warning has already been triggered.
+ *
+ */
+
+#include <arpa/inet.h>
+#include <bits/types/struct_iovec.h>
+#include <linux/netlink.h>
+#include <linux/rtnetlink.h>
+#include <netinet/in.h>
+#include <stdio.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <unistd.h>
+#include <dirent.h>
+#include <stdbool.h>
+#include <net/if.h>
+
+#define RTF_UP 0x0001 // route usable
+#define RTF_HOST 0x0004 // host entry (net otherwise)
+
+#define NSIM_PORTS 1
+#define NETDEVSIM_DEV_DIR "/sys/bus/netdevsim/devices"
+#define NSIM_DEV_DIR_BUFFER_SIZE 128
+#define LO_DEV "lo"
+
+#define BUFSIZE 4096
+#define DST_PREFIX "2001:db8::"
+#define GW1 "fe80::1"
+#define GW2 "::1"
+
+#define PID_LEN 16
+
+int get_free_idx(void)
+{
+ int idx = 0;
+ int tmp = 0;
+ DIR *nsim_dir = opendir(NETDEVSIM_DEV_DIR);
+ struct dirent *entry = NULL;
+
+ if (nsim_dir == NULL) {
+ fprintf(stderr, "Unable to open nsim directory\n");
+ return -1;
+ }
+
+ do {
+ entry = readdir(nsim_dir);
+ if (entry != NULL &&
+ sscanf(entry->d_name, "netdevsim%d", &tmp) > 0) {
+ if (tmp >= idx)
+ idx = tmp + 1;
+ }
+ } while (entry != NULL);
+
+ closedir(nsim_dir);
+ return idx;
+}
+
+int create_netdevsim_device(int id, int num_ports)
+{
+ const char *path = "/sys/bus/netdevsim/new_device";
+ char buffer[64];
+ int fd;
+
+ fd = open(path, O_WRONLY);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open new_device\n");
+ return -1;
+ }
+
+ snprintf(buffer, sizeof(buffer), "%d %d", id, num_ports);
+ if (write(fd, buffer, strlen(buffer)) < 0) {
+ fprintf(stderr, "Failed to write to new_device\n");
+ close(fd);
+ return -1;
+ }
+
+ close(fd);
+ return 0;
+}
+
+int ensure_nsim_dev_exists(void)
+{
+ int ret;
+ int nsim_idx;
+
+ nsim_idx = get_free_idx();
+ ret = create_netdevsim_device(nsim_idx, NSIM_PORTS);
+ if (ret != 0) {
+ fprintf(stderr, "Failed to create nsim device\n");
+ return -1;
+ }
+
+ return nsim_idx;
+}
+
+char *get_nsim_dev_link(int nsim_idx)
+{
+ char nsim_dev_dir_buffer[NSIM_DEV_DIR_BUFFER_SIZE];
+ DIR *nsim_dev_dir;
+ struct dirent *entry;
+
+ sprintf(nsim_dev_dir_buffer, "%s/netdevsim%d/%s", NETDEVSIM_DEV_DIR,
+ nsim_idx, "net");
+
+ nsim_dev_dir = opendir(nsim_dev_dir_buffer);
+
+ if (nsim_dev_dir == NULL) {
+ fprintf(stderr, "Unable to open %s\n", nsim_dev_dir_buffer);
+ return NULL;
+ }
+
+ do {
+ entry = readdir(nsim_dev_dir);
+ if (entry != NULL && entry->d_name[0] != '.')
+ break;
+
+ } while (entry != NULL);
+
+ if (entry == NULL || entry->d_name[0] == '.') {
+ fprintf(stderr, "Device has no ports\n");
+ return NULL;
+ }
+
+ closedir(nsim_dev_dir);
+
+ return entry->d_name;
+}
+
+int get_nsim_dev(char **nsim_link)
+{
+ int nsim_idx;
+ char *nsim_dev_link;
+
+ nsim_idx = ensure_nsim_dev_exists();
+ if (nsim_idx < 0)
+ return -1;
+
+ nsim_dev_link = get_nsim_dev_link(nsim_idx);
+ if (nsim_dev_link == NULL)
+ return -1;
+
+ *nsim_link = nsim_dev_link;
+ return 0;
+}
+
+int prepare_socket(void)
+{
+ struct sockaddr_nl sa;
+ int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
+
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open socket\n");
+ return -1;
+ }
+
+ sa.nl_family = AF_NETLINK;
+
+ if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0)
+ fprintf(stderr, "Failed to bind socket\n");
+
+ return fd;
+}
+
+struct nlmsghdr *construct_header(char **pos)
+{
+ struct nlmsghdr *nlh = (struct nlmsghdr *)(*pos);
+
+ nlh->nlmsg_type = RTM_NEWROUTE;
+ nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE;
+
+ *pos += NLMSG_HDRLEN;
+
+ return nlh;
+}
+
+void construct_rtmsg(char **pos)
+{
+ struct rtmsg *rtm = (struct rtmsg *)(*pos);
+
+ rtm->rtm_family = AF_INET6;
+ rtm->rtm_table = RT_TABLE_MAIN;
+ rtm->rtm_protocol = RTPROT_STATIC;
+ rtm->rtm_type = RTN_UNICAST;
+ rtm->rtm_scope = RT_SCOPE_UNIVERSE;
+ rtm->rtm_dst_len = 128;
+ rtm->rtm_flags |= RTF_HOST | RTF_UP;
+
+ *pos += NLMSG_ALIGN(sizeof(struct rtmsg));
+}
+
+void construct_dest(char **pos)
+{
+ struct rtattr *rta_dest = (struct rtattr *)(*pos);
+ struct in6_addr dst6;
+
+ rta_dest->rta_type = RTA_DST;
+ rta_dest->rta_len = RTA_LENGTH(sizeof(struct in6_addr));
+ inet_pton(AF_INET6, DST_PREFIX, &dst6);
+ memcpy(RTA_DATA(rta_dest), &dst6, sizeof(dst6));
+ *pos += RTA_ALIGN(rta_dest->rta_len);
+}
+
+struct rtattr *construct_multipath_hdr(char **pos)
+{
+ struct rtattr *rta_mp = (struct rtattr *)(*pos);
+
+ rta_mp->rta_type = RTA_MULTIPATH;
+ *pos += sizeof(struct rtattr);
+
+ return rta_mp;
+}
+
+void add_nexthop(char **pos, int ifindex, char *gw_addr)
+{
+ struct rtnexthop *rtnh = (struct rtnexthop *)(*pos);
+
+ rtnh->rtnh_hops = 0;
+ rtnh->rtnh_ifindex = ifindex;
+ char *rtnh_pos = (char *)rtnh + RTNH_ALIGN(sizeof(struct rtnexthop));
+
+ struct rtattr *attr = (struct rtattr *)rtnh_pos;
+
+ attr->rta_type = RTA_GATEWAY;
+ attr->rta_len = RTA_LENGTH(sizeof(struct in6_addr));
+
+ struct in6_addr gw;
+
+ inet_pton(AF_INET6, gw_addr, &gw);
+ memcpy(RTA_DATA(attr), &gw, sizeof(gw));
+
+ rtnh_pos += RTA_ALIGN(attr->rta_len);
+ rtnh->rtnh_len = rtnh_pos - (char *)rtnh;
+
+ *pos = rtnh_pos;
+}
+
+struct nlmsghdr *construct_message(char *buf, int nsim_ifindex, int lo_ifindex)
+{
+ char *pos = buf;
+ struct nlmsghdr *nlh = construct_header(&pos);
+
+ construct_rtmsg(&pos);
+ construct_dest(&pos);
+
+ struct rtattr *rta_mp = construct_multipath_hdr(&pos);
+
+ add_nexthop(&pos, nsim_ifindex, GW1);
+ add_nexthop(&pos, lo_ifindex, GW2);
+
+ rta_mp->rta_len = pos - (char *)rta_mp;
+ nlh->nlmsg_len = pos - buf;
+
+ return nlh;
+}
+
+int send_nl_msg(struct nlmsghdr *nlh, int socket)
+{
+ struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
+ struct msghdr msg = {
+ .msg_iov = &iov,
+ .msg_iovlen = 1,
+ };
+
+ if (sendmsg(socket, (struct msghdr *)&msg, 0) < 0) {
+ fprintf(stderr, "Failed to send message\n");
+ return 1;
+ }
+
+ return 0;
+}
+
+int open_kmsg(void)
+{
+ int fd = open("/dev/kmsg", O_RDONLY | O_NONBLOCK);
+
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open kmsg\n");
+ return -1;
+ }
+
+ return fd;
+}
+
+int move_cursor_to_end(int fd)
+{
+ if (lseek(fd, 0, SEEK_END) == -1) {
+ fprintf(stderr, "Failed to lseek kmsg\n");
+ return -1;
+ }
+
+ return 0;
+}
+
+int look_for_warn(int kmsg_fd)
+{
+ char buffer[1024];
+ int bytes_read;
+ int pid = getpid();
+ char pid_str[PID_LEN];
+
+ snprintf(pid_str, PID_LEN, "%d", pid);
+
+ while ((bytes_read = read(kmsg_fd, buffer, sizeof(buffer) - 1)) > 0) {
+ buffer[bytes_read] = '\0';
+ if (strstr(buffer, "WARNING") && strstr(buffer, pid_str)) {
+ printf("Kernel warning detected\n");
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+int main(void)
+{
+ char *nsim_dev;
+ int if_lo, if_nsim;
+ int fd;
+ int kmsg_fd;
+ struct nlmsghdr *nlh;
+ char buf[BUFSIZE];
+
+ if (get_nsim_dev(&nsim_dev) != 0)
+ return EXIT_FAILURE;
+
+ sleep(1); // Doesn't work without a delay
+
+ if_lo = if_nametoindex(LO_DEV);
+ if_nsim = if_nametoindex(nsim_dev);
+
+ if (!if_lo || !if_nsim) {
+ fprintf(stderr, "Failed to get interface index\n");
+ return EXIT_FAILURE;
+ }
+
+ memset(buf, 0, sizeof(buf));
+ nlh = construct_message(buf, if_nsim, if_lo);
+
+ fd = prepare_socket();
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open socket\n");
+ close(fd);
+ return EXIT_FAILURE;
+ }
+
+ kmsg_fd = open_kmsg();
+ if (kmsg_fd < 0) {
+ fprintf(stderr, "Failed to open kmsg\n");
+ close(fd);
+ return EXIT_FAILURE;
+ }
+
+ if (move_cursor_to_end(kmsg_fd) < 0) {
+ fprintf(stderr, "Failed to open kmsg\n");
+ close(fd);
+ close(kmsg_fd);
+ return EXIT_FAILURE;
+ }
+
+ if (send_nl_msg(nlh, fd) != 0) {
+ close(fd);
+ close(kmsg_fd);
+ return EXIT_FAILURE;
+ }
+
+ if (look_for_warn(kmsg_fd) != 0) {
+ close(fd);
+ close(kmsg_fd);
+ return EXIT_FAILURE;
+ }
+
+ close(kmsg_fd);
+ close(fd);
+ return EXIT_SUCCESS;
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] ipv6: Check AF_UNSPEC in ip6_route_multipath_add()
2025-09-05 22:12 ` Maksimilijan Marošević
2025-09-28 20:47 ` [PATCH v2 1/2] selftests/net: add netdevsim.c Maksimilijan Marosevic
@ 2025-09-28 20:47 ` Maksimilijan Marosevic
1 sibling, 0 replies; 8+ messages in thread
From: Maksimilijan Marosevic @ 2025-09-28 20:47 UTC (permalink / raw)
To: kuba, pabeni
Cc: Maksimilijan Marosevic, davem, dsahern, kuniyu, kuniyu, netdev,
linux-kselftest, shuah, syzbot+a259a17220263c2d73fc
This check was removed in commit e6f497955fb6 ("ipv6: Check GATEWAY
in rtm_to_fib6_multipath_config().") as part of rt6_qualify_for ecmp().
The author correctly recognises that rt6_qualify_for_ecmp() returns
false if fb_nh_gw_family is set to AF_UNSPEC, but then mistakes
AF_UNSPEC for AF_INET6 when reasoning that the check is unnecessary.
This means certain malformed entries don't get caught in
ip6_route_multipath_add().
This patch reintroduces the AF_UNSPEC check while respecting changes
of the initial patch.
Reported-by: syzbot+a259a17220263c2d73fc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a259a17220263c2d73fc
Fixes: e6f497955fb6 ("ipv6: Check GATEWAY in rtm_to_fib6_multipath_config().")
Signed-off-by: Maksimilijan Marosevic <maksimilijan.marosevic@proton.me>
---
net/ipv6/route.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index aee6a10b112a..884bae3fb1b1 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -5454,6 +5454,14 @@ static int ip6_route_multipath_add(struct fib6_config *cfg,
goto cleanup;
}
+ if (rt->fib6_nh->fib_nh_gw_family == AF_UNSPEC) {
+ err = -EINVAL;
+ NL_SET_ERR_MSG(extack,
+ "Device only routes can not be added for IPv6 using the multipath API.");
+ fib6_info_release(rt);
+ goto cleanup;
+ }
+
rt->fib6_nh->fib_nh_weight = rtnh->rtnh_hops + 1;
err = ip6_route_info_append(&rt6_nh_list, rt, &r_cfg);
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] selftests/net: add netdevsim.c
2025-09-28 20:47 ` [PATCH v2 1/2] selftests/net: add netdevsim.c Maksimilijan Marosevic
@ 2025-09-30 14:13 ` Paolo Abeni
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Abeni @ 2025-09-30 14:13 UTC (permalink / raw)
To: Maksimilijan Marosevic, kuba
Cc: davem, dsahern, kuniyu, kuniyu, netdev, linux-kselftest, shuah,
syzbot+a259a17220263c2d73fc
On 9/28/25 10:47 PM, Maksimilijan Marosevic wrote:
> Tests an edge case in the nsim module where gw_family == AF_UNSPEC.
>
> Works by creating a new nsim device and then sending a multipath
> path message to it and loopback. In unpatched kernels, this triggers
> a WARN_ON_ONCE in netdevsim/fib.c.
>
> Reported-by: syzbot+a259a17220263c2d73fc@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a259a17220263c2d73fc
> Fixes: e6f497955fb6 ("ipv6: Check GATEWAY in rtm_to_fib6_multipath_config().")
> Signed-off-by: Maksimilijan Marosevic <maksimilijan.marosevic@proton.me>
> ---
> tools/testing/selftests/net/netdevsim.c | 391 ++++++++++++++++++++++++
> 1 file changed, 391 insertions(+)
> create mode 100644 tools/testing/selftests/net/netdevsim.c
>
> diff --git a/tools/testing/selftests/net/netdevsim.c b/tools/testing/selftests/net/netdevsim.c
> new file mode 100644
> index 000000000000..cdc8ebef4dac
> --- /dev/null
> +++ b/tools/testing/selftests/net/netdevsim.c
> @@ -0,0 +1,391 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * This test creates a new netdevsim device and then sends
> + * an IPv6 multipath netlink message to it and the loopback
> + * interface.
> + *
> + * This triggers an edge case where the routing table is
> + * constructed with an entry where gw_family = AF_UNSPEC.
> + * If not caught, this causes an unexpected nsiblings count
> + * in netdevsim/fib.c: nsim_fib6_event_init(), raising a
> + * warning.
> + *
> + * NOTE: The warning in question is raised by WARN_ON_ONCE.
> + * Therefore, this test reports a false negative if the
> + * warning has already been triggered.
> + *
> + */
> +
> +#include <arpa/inet.h>
> +#include <bits/types/struct_iovec.h>
> +#include <linux/netlink.h>
> +#include <linux/rtnetlink.h>
> +#include <netinet/in.h>
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/socket.h>
> +#include <unistd.h>
> +#include <dirent.h>
> +#include <stdbool.h>
> +#include <net/if.h>
> +
> +#define RTF_UP 0x0001 // route usable
> +#define RTF_HOST 0x0004 // host entry (net otherwise)
> +
> +#define NSIM_PORTS 1
> +#define NETDEVSIM_DEV_DIR "/sys/bus/netdevsim/devices"
> +#define NSIM_DEV_DIR_BUFFER_SIZE 128
> +#define LO_DEV "lo"
> +
> +#define BUFSIZE 4096
> +#define DST_PREFIX "2001:db8::"
> +#define GW1 "fe80::1"
> +#define GW2 "::1"
> +
> +#define PID_LEN 16
> +
> +int get_free_idx(void)
> +{
> + int idx = 0;
> + int tmp = 0;
> + DIR *nsim_dir = opendir(NETDEVSIM_DEV_DIR);
> + struct dirent *entry = NULL;
> +
> + if (nsim_dir == NULL) {
Prefer '!nsim_dir' to test for null PTR (above and elsewhere).
> + fprintf(stderr, "Unable to open nsim directory\n");
> + return -1;
You can probably streamline the code using the error() libcall here and
in many places below
> + }
> +
> + do {
> + entry = readdir(nsim_dir);
> + if (entry != NULL &&
> + sscanf(entry->d_name, "netdevsim%d", &tmp) > 0) {
> + if (tmp >= idx)
> + idx = tmp + 1;
> + }
> + } while (entry != NULL);
> +
> + closedir(nsim_dir);
> + return idx;
> +}
> +
> +int create_netdevsim_device(int id, int num_ports)
> +{
> + const char *path = "/sys/bus/netdevsim/new_device";
> + char buffer[64];
> + int fd;
> +
> + fd = open(path, O_WRONLY);
> + if (fd < 0) {
> + fprintf(stderr, "Failed to open new_device\n");
> + return -1;
> + }
> +
> + snprintf(buffer, sizeof(buffer), "%d %d", id, num_ports);
> + if (write(fd, buffer, strlen(buffer)) < 0) {
> + fprintf(stderr, "Failed to write to new_device\n");
> + close(fd);
> + return -1;
> + }
> +
> + close(fd);
> + return 0;
> +}
> +
> +int ensure_nsim_dev_exists(void)
> +{
> + int ret;
> + int nsim_idx;
> +
> + nsim_idx = get_free_idx();
> + ret = create_netdevsim_device(nsim_idx, NSIM_PORTS);
> + if (ret != 0) {
> + fprintf(stderr, "Failed to create nsim device\n");
> + return -1;
> + }
AFAICS the above is still racy. I think it could be made safer running
in a netns with a setup script creading an 'unique' netns name for this
test (see setup_ns() in tools/testing/selftests/net/lib.sh).
> +
> + return nsim_idx;
> +}
> +
> +char *get_nsim_dev_link(int nsim_idx)
> +{
> + char nsim_dev_dir_buffer[NSIM_DEV_DIR_BUFFER_SIZE];
> + DIR *nsim_dev_dir;
> + struct dirent *entry;
> +
> + sprintf(nsim_dev_dir_buffer, "%s/netdevsim%d/%s", NETDEVSIM_DEV_DIR,
> + nsim_idx, "net");
> +
> + nsim_dev_dir = opendir(nsim_dev_dir_buffer);
> +
> + if (nsim_dev_dir == NULL) {
> + fprintf(stderr, "Unable to open %s\n", nsim_dev_dir_buffer);
> + return NULL;
> + }
> +
> + do {
> + entry = readdir(nsim_dev_dir);
> + if (entry != NULL && entry->d_name[0] != '.')
> + break;
> +
> + } while (entry != NULL);
> +
> + if (entry == NULL || entry->d_name[0] == '.') {
> + fprintf(stderr, "Device has no ports\n");
> + return NULL;
> + }
> +
> + closedir(nsim_dev_dir);
> +
> + return entry->d_name;
> +}
> +
> +int get_nsim_dev(char **nsim_link)
> +{
> + int nsim_idx;
> + char *nsim_dev_link;
> +
> + nsim_idx = ensure_nsim_dev_exists();
> + if (nsim_idx < 0)
> + return -1;
> +
> + nsim_dev_link = get_nsim_dev_link(nsim_idx);
> + if (nsim_dev_link == NULL)
> + return -1;
> +
> + *nsim_link = nsim_dev_link;
> + return 0;
> +}
> +
> +int prepare_socket(void)
> +{
> + struct sockaddr_nl sa;
> + int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
> +
> + if (fd < 0) {
> + fprintf(stderr, "Failed to open socket\n");
> + return -1;
> + }
> +
> + sa.nl_family = AF_NETLINK;
> +
> + if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0)
> + fprintf(stderr, "Failed to bind socket\n");
Why you don't error out here? also you need to explictly zero 'sa' or
the kernel may try to bind to 'random' NL group.
> +
> + return fd;
> +}
> +
> +struct nlmsghdr *construct_header(char **pos)
> +{
> + struct nlmsghdr *nlh = (struct nlmsghdr *)(*pos);
> +
> + nlh->nlmsg_type = RTM_NEWROUTE;
> + nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE;
> +
> + *pos += NLMSG_HDRLEN;
> +
> + return nlh;
> +}
> +
> +void construct_rtmsg(char **pos)
> +{
> + struct rtmsg *rtm = (struct rtmsg *)(*pos);
> +
> + rtm->rtm_family = AF_INET6;
> + rtm->rtm_table = RT_TABLE_MAIN;
> + rtm->rtm_protocol = RTPROT_STATIC;
> + rtm->rtm_type = RTN_UNICAST;
> + rtm->rtm_scope = RT_SCOPE_UNIVERSE;
> + rtm->rtm_dst_len = 128;
> + rtm->rtm_flags |= RTF_HOST | RTF_UP;
> +
> + *pos += NLMSG_ALIGN(sizeof(struct rtmsg));
> +}
> +
> +void construct_dest(char **pos)
> +{
> + struct rtattr *rta_dest = (struct rtattr *)(*pos);
> + struct in6_addr dst6;
> +
> + rta_dest->rta_type = RTA_DST;
> + rta_dest->rta_len = RTA_LENGTH(sizeof(struct in6_addr));
> + inet_pton(AF_INET6, DST_PREFIX, &dst6);
> + memcpy(RTA_DATA(rta_dest), &dst6, sizeof(dst6));
> + *pos += RTA_ALIGN(rta_dest->rta_len);
> +}
> +
> +struct rtattr *construct_multipath_hdr(char **pos)
> +{
> + struct rtattr *rta_mp = (struct rtattr *)(*pos);
> +
> + rta_mp->rta_type = RTA_MULTIPATH;
> + *pos += sizeof(struct rtattr);
> +
> + return rta_mp;
> +}
> +
> +void add_nexthop(char **pos, int ifindex, char *gw_addr)
> +{
> + struct rtnexthop *rtnh = (struct rtnexthop *)(*pos);
> +
> + rtnh->rtnh_hops = 0;
> + rtnh->rtnh_ifindex = ifindex;
> + char *rtnh_pos = (char *)rtnh + RTNH_ALIGN(sizeof(struct rtnexthop));
> +
> + struct rtattr *attr = (struct rtattr *)rtnh_pos;
Please avoid mixing variable definition and code (above and elsewhere)
> +
> + attr->rta_type = RTA_GATEWAY;
> + attr->rta_len = RTA_LENGTH(sizeof(struct in6_addr));
> +
> + struct in6_addr gw;
> +
> + inet_pton(AF_INET6, gw_addr, &gw);
> + memcpy(RTA_DATA(attr), &gw, sizeof(gw));
> +
> + rtnh_pos += RTA_ALIGN(attr->rta_len);
> + rtnh->rtnh_len = rtnh_pos - (char *)rtnh;
> +
> + *pos = rtnh_pos;
> +}
> +
> +struct nlmsghdr *construct_message(char *buf, int nsim_ifindex, int lo_ifindex)
> +{
> + char *pos = buf;
> + struct nlmsghdr *nlh = construct_header(&pos);
> +
> + construct_rtmsg(&pos);
> + construct_dest(&pos);
> +
> + struct rtattr *rta_mp = construct_multipath_hdr(&pos);
> +
> + add_nexthop(&pos, nsim_ifindex, GW1);
> + add_nexthop(&pos, lo_ifindex, GW2);
> +
> + rta_mp->rta_len = pos - (char *)rta_mp;
> + nlh->nlmsg_len = pos - buf;
> +
> + return nlh;
> +}
> +
> +int send_nl_msg(struct nlmsghdr *nlh, int socket)
> +{
> + struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
> + struct msghdr msg = {
> + .msg_iov = &iov,
> + .msg_iovlen = 1,
> + };
> +
> + if (sendmsg(socket, (struct msghdr *)&msg, 0) < 0) {
> + fprintf(stderr, "Failed to send message\n");
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +int open_kmsg(void)
> +{
> + int fd = open("/dev/kmsg", O_RDONLY | O_NONBLOCK);
> +
> + if (fd < 0) {
> + fprintf(stderr, "Failed to open kmsg\n");
> + return -1;
> + }
> +
> + return fd;
> +}
> +
> +int move_cursor_to_end(int fd)
> +{
> + if (lseek(fd, 0, SEEK_END) == -1) {
> + fprintf(stderr, "Failed to lseek kmsg\n");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +int look_for_warn(int kmsg_fd)
> +{
> + char buffer[1024];
> + int bytes_read;
> + int pid = getpid();
> + char pid_str[PID_LEN];
> +
> + snprintf(pid_str, PID_LEN, "%d", pid);
> +
> + while ((bytes_read = read(kmsg_fd, buffer, sizeof(buffer) - 1)) > 0) {
> + buffer[bytes_read] = '\0';
> + if (strstr(buffer, "WARNING") && strstr(buffer, pid_str)) {
> + printf("Kernel warning detected\n");
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +int main(void)
> +{
> + char *nsim_dev;
> + int if_lo, if_nsim;
> + int fd;
> + int kmsg_fd;
> + struct nlmsghdr *nlh;
> + char buf[BUFSIZE];
> +
> + if (get_nsim_dev(&nsim_dev) != 0)
> + return EXIT_FAILURE;
> +
> + sleep(1); // Doesn't work without a delay
Why? also always use /* */ for comments.
/P
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-30 14:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-04 20:42 [PATCH 1/1] ipv6: Check AF_UNSPEC in ip6_route_multipath_add() Maksimilijan Marosevic
2025-08-05 16:16 ` David Ahern
2025-08-12 8:16 ` Paolo Abeni
2025-09-04 16:03 ` Jakub Kicinski
2025-09-05 22:12 ` Maksimilijan Marošević
2025-09-28 20:47 ` [PATCH v2 1/2] selftests/net: add netdevsim.c Maksimilijan Marosevic
2025-09-30 14:13 ` Paolo Abeni
2025-09-28 20:47 ` [PATCH v2 2/2] ipv6: Check AF_UNSPEC in ip6_route_multipath_add() Maksimilijan Marosevic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).