Hello David,
this is a pull request intended for net/linux-4.1. I know it is rather late in the release cycle, but the fixes are pretty small and they were worth being sent now.
Patch 1/2 fixes an overflow of a u32 variable that may happen while computing the metric to select the best batman-GW in the network. ** It would be great if this patch could be queued for inclusion in any stable release >= 3.2
Patch 2/2 prevents the Distributed ARP Table from forging unwanted ARP replies on behalf of another host that may fool an Ethernet switch sitting behind batman-adv. ** It would be great if this patch could be queued for inclusion in any stable release >= 3.8
Please pull or let me know if something is wrong
Thanks a lot, Antonio
The following changes since commit ac0a72a3e6e8d817f60ce4d9a8f3b43dc256d847:
net/mlx4_core: Disable Granular QoS per VF under IB/Eth VPI configuration (2015-06-15 16:42:57 -0700)
are available in the git repository at:
git://git.open-mesh.org/linux-merge.git tags/batman-adv-fix-for-davem
for you to fetch changes up to c065d51055924f6daad8e16307c364602b0f9805:
batman-adv: avoid DAT to mess up LAN state (2015-06-16 11:13:12 +0200)
---------------------------------------------------------------- Included changes: - fix gateway selection metric overflow - avoid the Distriubted ARP Table to fool Ethernet switches by forging unexpected packets
---------------------------------------------------------------- Antonio Quartulli (1): batman-adv: avoid DAT to mess up LAN state
Ruben Wisniewski (1): batman-adv: Avoid u32 overflow during gateway select
net/batman-adv/distributed-arp-table.c | 18 +++++++++++++----- net/batman-adv/gateway_client.c | 2 +- 2 files changed, 14 insertions(+), 6 deletions(-)
From: Ruben Wisniewski ruben@freifunk-nrw.de
The gateway selection based on fast connections is using a single value calculated from the average tq (0-255) and the download bandwidth (in 100Kibit). The formula for the first step (tq ** 2 * 10000 * bandwidth) tends to overflow a u32 with low bandwidth settings like 50 [100KiBit] and a tq value of over 92.
Changing this to a 64 bit unsigned integer allows to support a bandwidth_down with up to ~2.8e10 [100KiBit] and a perfect tq of 255. This is ~6.6 times higher than the maximum possible value of the gateway announcement TVLV.
This problem only affects the non-default gw_sel_class 1.
Signed-off-by: Ruben Wisniewsi ruben@vfn-nrw.de [sven@narfation.org: rewritten commit message, changed to kernel type] Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch
Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/gateway_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c index 090828c..ca734f8 100644 --- a/net/batman-adv/gateway_client.c +++ b/net/batman-adv/gateway_client.c @@ -133,7 +133,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv) struct batadv_neigh_node *router; struct batadv_neigh_ifinfo *router_ifinfo; struct batadv_gw_node *gw_node, *curr_gw = NULL; - uint32_t max_gw_factor = 0, tmp_gw_factor = 0; + uint64_t max_gw_factor = 0, tmp_gw_factor = 0; uint32_t gw_divisor; uint8_t max_tq = 0; uint8_t tq_avg;
From: Antonio Quartulli antonio@meshcoding.com Date: Tue, 16 Jun 2015 21:06:23 +0200
@@ -133,7 +133,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv) struct batadv_neigh_node *router; struct batadv_neigh_ifinfo *router_ifinfo; struct batadv_gw_node *gw_node, *curr_gw = NULL;
- uint32_t max_gw_factor = 0, tmp_gw_factor = 0;
- uint64_t max_gw_factor = 0, tmp_gw_factor = 0; uint32_t gw_divisor;
Divides are performed using these variables, so if you make them 64-bit the build will fail. You have to use one of the routines in include/linux/math64.h in this situation, but realize that this operation is now going to be quite expensive.
On Sunday 21 June 2015 09:37:13 David Miller wrote:
From: Antonio Quartulli antonio@meshcoding.com Date: Tue, 16 Jun 2015 21:06:23 +0200
@@ -133,7 +133,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)> struct batadv_neigh_node *router; struct batadv_neigh_ifinfo *router_ifinfo; struct batadv_gw_node *gw_node, *curr_gw = NULL;
- uint32_t max_gw_factor = 0, tmp_gw_factor = 0;
uint64_t max_gw_factor = 0, tmp_gw_factor = 0;
uint32_t gw_divisor;
Divides are performed using these variables, so if you make them 64-bit the build will fail. You have to use one of the routines in include/linux/math64.h in this situation, but realize that this operation is now going to be quite expensive.
Just a minor comment: The division is currently "max_gw_factor >> 18" which the compiler also outputs as such (shrd + shr on i586). This is the reason why it currently also works on architectures without a native 64 bit div.
Kind regards, Sven
On 21/06/15 20:27, Sven Eckelmann wrote:
On Sunday 21 June 2015 09:37:13 David Miller wrote:
From: Antonio Quartulli antonio@meshcoding.com Date: Tue, 16 Jun 2015 21:06:23 +0200
@@ -133,7 +133,7 @@ batadv_gw_get_best_gw_node(struct batadv_priv *bat_priv)> struct batadv_neigh_node *router; struct batadv_neigh_ifinfo *router_ifinfo; struct batadv_gw_node *gw_node, *curr_gw = NULL;
- uint32_t max_gw_factor = 0, tmp_gw_factor = 0;
uint64_t max_gw_factor = 0, tmp_gw_factor = 0;
uint32_t gw_divisor;
Divides are performed using these variables, so if you make them 64-bit the build will fail. You have to use one of the routines in include/linux/math64.h in this situation, but realize that this operation is now going to be quite expensive.
Just a minor comment: The division is currently "max_gw_factor >> 18" which the compiler also outputs as such (shrd + shr on i586). This is the reason why it currently also works on architectures without a native 64 bit div.
Thanks for the comment David and Sven.
David you can drop this pull request. I'll send a new one later with a modified patch that explicitly introduces the shift.
Thanks!
When a node running DAT receives an ARP request from the LAN for the first time, it is likely that this node will request the ARP entry through the distributed ARP table (DAT) in the mesh.
Once a DAT reply is received the asking node must check if the MAC address for which the IP address has been asked is local. If it is, the node must drop the ARP reply bceause the client should have replied on its own locally.
Forwarding this reply means fooling any L2 bridge (e.g. Ethernet switches) lying between the batman-adv node and the LAN. This happens because the L2 bridge will think that the client sending the ARP reply lies somewhere in the mesh, while this node is sitting in the same LAN.
Reported-by: Simon Wunderlich sw@simonwunderlich.de Signed-off-by: Antonio Quartulli antonio@meshcoding.com Signed-off-by: Marek Lindner mareklindner@neomailbox.ch --- net/batman-adv/distributed-arp-table.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index aad022d..5a4d45a 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -1107,6 +1107,9 @@ void batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv, * @bat_priv: the bat priv with all the soft interface information * @skb: packet to check * @hdr_size: size of the encapsulation header + * + * Returns true if the packet was snooped and consumed by DAT. False if the + * packet has to be delivered to the interface */ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv, struct sk_buff *skb, int hdr_size) @@ -1114,7 +1117,7 @@ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv, uint16_t type; __be32 ip_src, ip_dst; uint8_t *hw_src, *hw_dst; - bool ret = false; + bool dropped = false; unsigned short vid;
if (!atomic_read(&bat_priv->distributed_arp_table)) @@ -1143,12 +1146,17 @@ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv, /* if this REPLY is directed to a client of mine, let's deliver the * packet to the interface */ - ret = !batadv_is_my_client(bat_priv, hw_dst, vid); + dropped = !batadv_is_my_client(bat_priv, hw_dst, vid); + + /* if this REPLY is sent on behalf of a client of mine, let's drop the + * packet because the client will reply by itself + */ + dropped |= batadv_is_my_client(bat_priv, hw_src, vid); out: - if (ret) + if (dropped) kfree_skb(skb); - /* if ret == false -> packet has to be delivered to the interface */ - return ret; + /* if dropped == false -> deliver to the interface */ + return dropped; }
/**
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
David Miller -
Sven Eckelmann