From 690bb6fb64f5dc7437317153902573ecad67593d Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Mon, 28 Feb 2022 00:01:24 +0100 Subject: batman-adv: Request iflink once in batadv-on-batadv check There is no need to call dev_get_iflink multiple times for the same net_device in batadv_is_on_batman_iface. And since some of the .ndo_get_iflink callbacks are dynamic (for example via RCUs like in vxcan_get_iflink), it could easily happen that the returned values are not stable. The pre-checks before __dev_get_by_index are then of course bogus. Fixes: b7eddd0b3950 ("batman-adv: prevent using any virtual device created on batman-adv as hard-interface") Signed-off-by: Sven Eckelmann Signed-off-by: Simon Wunderlich --- net/batman-adv/hard-interface.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'net/batman-adv') diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 8a2b78f9c4b2..35aa1122043b 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -149,22 +149,23 @@ static bool batadv_is_on_batman_iface(const struct net_device *net_dev) struct net *net = dev_net(net_dev); struct net_device *parent_dev; struct net *parent_net; + int iflink; bool ret; /* check if this is a batman-adv mesh interface */ if (batadv_softif_is_valid(net_dev)) return true; + iflink = dev_get_iflink(net_dev); + /* no more parents..stop recursion */ - if (dev_get_iflink(net_dev) == 0 || - dev_get_iflink(net_dev) == net_dev->ifindex) + if (iflink == 0 || iflink == net_dev->ifindex) return false; parent_net = batadv_getlink_net(net_dev, net); /* recurse over the parent device */ - parent_dev = __dev_get_by_index((struct net *)parent_net, - dev_get_iflink(net_dev)); + parent_dev = __dev_get_by_index((struct net *)parent_net, iflink); /* if we got a NULL parent_dev there is something broken.. */ if (!parent_dev) { pr_err("Cannot find parent device\n"); -- cgit v1.2.3 From 6116ba09423f7d140f0460be6a1644dceaad00da Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Mon, 28 Feb 2022 00:01:24 +0100 Subject: batman-adv: Request iflink once in batadv_get_real_netdevice There is no need to call dev_get_iflink multiple times for the same net_device in batadv_get_real_netdevice. And since some of the ndo_get_iflink callbacks are dynamic (for example via RCUs like in vxcan_get_iflink), it could easily happen that the returned values are not stable. The pre-checks before __dev_get_by_index are then of course bogus. Fixes: 5ed4a460a1d3 ("batman-adv: additional checks for virtual interfaces on top of WiFi") Signed-off-by: Sven Eckelmann Signed-off-by: Simon Wunderlich --- net/batman-adv/hard-interface.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'net/batman-adv') diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 35aa1122043b..e2760cfce190 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -215,14 +215,16 @@ static struct net_device *batadv_get_real_netdevice(struct net_device *netdev) struct net_device *real_netdev = NULL; struct net *real_net; struct net *net; - int ifindex; + int iflink; ASSERT_RTNL(); if (!netdev) return NULL; - if (netdev->ifindex == dev_get_iflink(netdev)) { + iflink = dev_get_iflink(netdev); + + if (netdev->ifindex == iflink) { dev_hold(netdev); return netdev; } @@ -232,9 +234,8 @@ static struct net_device *batadv_get_real_netdevice(struct net_device *netdev) goto out; net = dev_net(hard_iface->soft_iface); - ifindex = dev_get_iflink(netdev); real_net = batadv_getlink_net(netdev, net); - real_netdev = dev_get_by_index(real_net, ifindex); + real_netdev = dev_get_by_index(real_net, iflink); out: batadv_hardif_put(hard_iface); -- cgit v1.2.3 From 6c1f41afc1dbe59d9d3c8bb0d80b749c119aa334 Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Sun, 27 Feb 2022 23:23:49 +0100 Subject: batman-adv: Don't expect inter-netns unique iflink indices The ifindex doesn't have to be unique for multiple network namespaces on the same machine. $ ip netns add test1 $ ip -net test1 link add dummy1 type dummy $ ip netns add test2 $ ip -net test2 link add dummy2 type dummy $ ip -net test1 link show dev dummy1 6: dummy1: mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000 link/ether 96:81:55:1e:dd:85 brd ff:ff:ff:ff:ff:ff $ ip -net test2 link show dev dummy2 6: dummy2: mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000 link/ether 5a:3c:af:35:07:c3 brd ff:ff:ff:ff:ff:ff But the batman-adv code to walk through the various layers of virtual interfaces uses this assumption because dev_get_iflink handles it internally and doesn't return the actual netns of the iflink. And dev_get_iflink only documents the situation where ifindex == iflink for physical devices. But only checking for dev->netdev_ops->ndo_get_iflink is also not an option because ipoib_get_iflink implements it even when it sometimes returns an iflink != ifindex and sometimes iflink == ifindex. The caller must therefore make sure itself to check both netns and iflink + ifindex for equality. Only when they are equal, a "physical" interface was detected which should stop the traversal. On the other hand, vxcan_get_iflink can also return 0 in case there was currently no valid peer. In this case, it is still necessary to stop. Fixes: b7eddd0b3950 ("batman-adv: prevent using any virtual device created on batman-adv as hard-interface") Fixes: 5ed4a460a1d3 ("batman-adv: additional checks for virtual interfaces on top of WiFi") Reported-by: Sabrina Dubroca Signed-off-by: Sven Eckelmann Signed-off-by: Simon Wunderlich --- net/batman-adv/hard-interface.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) (limited to 'net/batman-adv') diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index e2760cfce190..35fadb924849 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -157,13 +157,15 @@ static bool batadv_is_on_batman_iface(const struct net_device *net_dev) return true; iflink = dev_get_iflink(net_dev); - - /* no more parents..stop recursion */ - if (iflink == 0 || iflink == net_dev->ifindex) + if (iflink == 0) return false; parent_net = batadv_getlink_net(net_dev, net); + /* iflink to itself, most likely physical device */ + if (net == parent_net && iflink == net_dev->ifindex) + return false; + /* recurse over the parent device */ parent_dev = __dev_get_by_index((struct net *)parent_net, iflink); /* if we got a NULL parent_dev there is something broken.. */ @@ -223,8 +225,7 @@ static struct net_device *batadv_get_real_netdevice(struct net_device *netdev) return NULL; iflink = dev_get_iflink(netdev); - - if (netdev->ifindex == iflink) { + if (iflink == 0) { dev_hold(netdev); return netdev; } @@ -235,6 +236,14 @@ static struct net_device *batadv_get_real_netdevice(struct net_device *netdev) net = dev_net(hard_iface->soft_iface); real_net = batadv_getlink_net(netdev, net); + + /* iflink to itself, most likely physical device */ + if (net == real_net && netdev->ifindex == iflink) { + real_netdev = netdev; + dev_hold(real_netdev); + goto out; + } + real_netdev = dev_get_by_index(real_net, iflink); out: -- cgit v1.2.3