Hi,
following two patches were backported "automatically" applied in 4.14.y, 4.19.y, 5.4.y, 5.10.y, 5.5.y and 5.16.y. But they failed to apply cleanly in v4.9.y due to some changes in the patch context and one missing function in the older batman-adv version.
These problems were now fixed manually.
Kind regards, Sven
Sven Eckelmann (2): batman-adv: Request iflink once in batadv-on-batadv check batman-adv: Don't expect inter-netns unique iflink indices
net/batman-adv/hard-interface.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
commit 690bb6fb64f5dc7437317153902573ecad67593d upstream.
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 sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.9 backported: adjust context. ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/hard-interface.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 4f384abb4ced..64cf9cd3be4d 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -155,22 +155,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; const 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");
commit 6c1f41afc1dbe59d9d3c8bb0d80b749c119aa334 upstream.
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: <BROADCAST,NOARP> 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: <BROADCAST,NOARP> 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 sd@queasysnail.net Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de [ bp: 4.9 backported: drop modification of non-existing batadv_get_real_netdevice. ] Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/hard-interface.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index 64cf9cd3be4d..eaf0a483211a 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -163,13 +163,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.. */
On Wed, Mar 09, 2022 at 05:45:40PM +0100, Sven Eckelmann wrote:
Hi,
following two patches were backported "automatically" applied in 4.14.y, 4.19.y, 5.4.y, 5.10.y, 5.5.y and 5.16.y. But they failed to apply cleanly in v4.9.y due to some changes in the patch context and one missing function in the older batman-adv version.
These problems were now fixed manually.
Kind regards, Sven
Sven Eckelmann (2): batman-adv: Request iflink once in batadv-on-batadv check batman-adv: Don't expect inter-netns unique iflink indices
net/batman-adv/hard-interface.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
-- 2.30.2
Both now queued up, thanks.
greg k-h
b.a.t.m.a.n@lists.open-mesh.org
-
Greg KH -
Sven Eckelmann