batman-adv: Merge bugfixes from 2022.1
authorSven Eckelmann <sven@narfation.org>
Fri, 6 May 2022 20:26:35 +0000 (22:26 +0200)
committerSven Eckelmann <sven@narfation.org>
Fri, 6 May 2022 20:26:35 +0000 (22:26 +0200)
* Request iflink once in batadv-on-batadv check
* Request iflink once in batadv_get_real_netdevice
* Don't expect inter-netns unique iflink indices
* Don't skb_split skbuffs with frag_list

Signed-off-by: Sven Eckelmann <sven@narfation.org>
batman-adv/patches/0008-batman-adv-Request-iflink-once-in-batadv-on-batadv-c.patch [new file with mode: 0644]
batman-adv/patches/0009-batman-adv-Request-iflink-once-in-batadv_get_real_ne.patch [new file with mode: 0644]
batman-adv/patches/0010-batman-adv-Don-t-expect-inter-netns-unique-iflink-in.patch [new file with mode: 0644]
batman-adv/patches/0011-batman-adv-Don-t-skb_split-skbuffs-with-frag_list.patch [new file with mode: 0644]

diff --git a/batman-adv/patches/0008-batman-adv-Request-iflink-once-in-batadv-on-batadv-c.patch b/batman-adv/patches/0008-batman-adv-Request-iflink-once-in-batadv-on-batadv-c.patch
new file mode 100644 (file)
index 0000000..c117cca
--- /dev/null
@@ -0,0 +1,44 @@
+From: Sven Eckelmann <sven@narfation.org>
+Date: Wed, 2 Mar 2022 19:49:44 +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: 3d48811b27f5 ("batman-adv: prevent using any virtual device created on batman-adv as hard-interface")
+Signed-off-by: Sven Eckelmann <sven@narfation.org>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/f6c0c45356fe3ab9c5e01b81d060f8a436658037
+
+--- a/net/batman-adv/hard-interface.c
++++ b/net/batman-adv/hard-interface.c
+@@ -148,22 +148,23 @@ static bool batadv_is_on_batman_iface(co
+       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");
diff --git a/batman-adv/patches/0009-batman-adv-Request-iflink-once-in-batadv_get_real_ne.patch b/batman-adv/patches/0009-batman-adv-Request-iflink-once-in-batadv_get_real_ne.patch
new file mode 100644 (file)
index 0000000..d0e1270
--- /dev/null
@@ -0,0 +1,46 @@
+From: Sven Eckelmann <sven@narfation.org>
+Date: Wed, 2 Mar 2022 19:49:45 +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: 2b45bb6c3aad ("batman-adv: additional checks for virtual interfaces on top of WiFi")
+Signed-off-by: Sven Eckelmann <sven@narfation.org>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/445f477ec3e805425186f5fbe8ed0fe89cc226b6
+
+--- a/net/batman-adv/hard-interface.c
++++ b/net/batman-adv/hard-interface.c
+@@ -214,14 +214,16 @@ static struct net_device *batadv_get_rea
+       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;
+       }
+@@ -231,9 +233,8 @@ static struct net_device *batadv_get_rea
+               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);
diff --git a/batman-adv/patches/0010-batman-adv-Don-t-expect-inter-netns-unique-iflink-in.patch b/batman-adv/patches/0010-batman-adv-Don-t-expect-inter-netns-unique-iflink-in.patch
new file mode 100644 (file)
index 0000000..1a7859c
--- /dev/null
@@ -0,0 +1,86 @@
+From: Sven Eckelmann <sven@narfation.org>
+Date: Wed, 2 Mar 2022 19:49:46 +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: <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: 3d48811b27f5 ("batman-adv: prevent using any virtual device created on batman-adv as hard-interface")
+Fixes: 2b45bb6c3aad ("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>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/0aac7a9fbbbeec25f2f54a9e6d53ea91217ba720
+
+--- a/net/batman-adv/hard-interface.c
++++ b/net/batman-adv/hard-interface.c
+@@ -156,13 +156,15 @@ static bool batadv_is_on_batman_iface(co
+               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.. */
+@@ -222,8 +224,7 @@ static struct net_device *batadv_get_rea
+               return NULL;
+       iflink = dev_get_iflink(netdev);
+-
+-      if (netdev->ifindex == iflink) {
++      if (iflink == 0) {
+               dev_hold(netdev);
+               return netdev;
+       }
+@@ -234,6 +235,14 @@ static struct net_device *batadv_get_rea
+       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:
diff --git a/batman-adv/patches/0011-batman-adv-Don-t-skb_split-skbuffs-with-frag_list.patch b/batman-adv/patches/0011-batman-adv-Don-t-skb_split-skbuffs-with-frag_list.patch
new file mode 100644 (file)
index 0000000..4f6f30e
--- /dev/null
@@ -0,0 +1,46 @@
+From: Sven Eckelmann <sven@narfation.org>
+Date: Sat, 16 Apr 2022 14:24:34 +0200
+Subject: batman-adv: Don't skb_split skbuffs with frag_list
+
+The receiving interface might have used GRO to receive more fragments than
+MAX_SKB_FRAGS fragments. In this case, these will not be stored in
+skb_shinfo(skb)->frags but merged into the frag list.
+
+batman-adv relies on the function skb_split to split packets up into
+multiple smaller packets which are not larger than the MTU on the outgoing
+interface. But this function cannot handle frag_list entries and is only
+operating on skb_shinfo(skb)->frags. If it is still trying to split such an
+skb and xmit'ing it on an interface without support for NETIF_F_FRAGLIST,
+then validate_xmit_skb() will try to linearize it. But this fails due to
+inconsistent information. And __pskb_pull_tail will trigger a BUG_ON after
+skb_copy_bits() returns an error.
+
+In case of entries in frag_list, just linearize the skb before operating on
+it with skb_split().
+
+Reported-by: Felix Kaechele <felix@kaechele.ca>
+Tested-by: Felix Kaechele <felix@kaechele.ca>
+Fixes: 9de347143505 ("batman-adv: layer2 unicast packet fragmentation")
+Signed-off-by: Sven Eckelmann <sven@narfation.org>
+Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/d467720acaf1b22b0cee58144eeaf9ef6c5e285c
+
+--- a/net/batman-adv/fragmentation.c
++++ b/net/batman-adv/fragmentation.c
+@@ -475,6 +475,17 @@ int batadv_frag_send_packet(struct sk_bu
+               goto free_skb;
+       }
++      /* GRO might have added fragments to the fragment list instead of
++       * frags[]. But this is not handled by skb_split and must be
++       * linearized to avoid incorrect length information after all
++       * batman-adv fragments were created and submitted to the
++       * hard-interface
++       */
++      if (skb_has_frag_list(skb) && __skb_linearize(skb)) {
++              ret = -ENOMEM;
++              goto free_skb;
++      }
++
+       /* Create one header to be copied to all fragments */
+       frag_header.packet_type = BATADV_UNICAST_FRAG;
+       frag_header.version = BATADV_COMPAT_VERSION;