Hello David,
our debugging campaign continues..
Here I have another small set of fixes intended for net/linux-3.15 and *stable* (please enqueue them).
I know we already sent some fixes and we are running late in the rc cycle, but, as you can see from the stats below, the changes are very very small.
Patch 1 prevents a NULL dereference in batadv_orig_hardif_seq_print_text() along the failure path.
Patch 2 fixes a reference counting imbalance that gets triggered everytime the batman packet fragmentation mechanism is used; this imbalance prevents the netdev object held by batman-adv from being released on shutdown.
Patch 3 fixes the reference counting for the orig_node objects in order to avoid any access after they have been free'd.
Patch 4 fixes the TT local check in DAT (Distributed ARP Table) that is used to avoid sending (self-forged) ARP replies on behalf of other host in the LAN.
Please pull or let me know of any problem!
Thanks a lot, Antonio
The following changes since commit e84d2f8d2ae33c8215429824e1ecf24cbca9645e:
net: filter: s390: fix JIT address randomization (2014-05-14 16:10:16 -0400)
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 cc2f33860cea0e48ebec096130bd0f7c4bf6e0bc:
batman-adv: fix local TT check for outgoing arp requests in DAT (2014-05-15 20:23:47 +0200)
---------------------------------------------------------------- Include changes: - fix NULL dereference in batadv_orig_hardif_seq_print_text() - fix reference counting imbalance when using fragmentation - avoid access to orig_node objects after they have been free'd - fix local TT check for outgoing arp requests in DAT
---------------------------------------------------------------- Antonio Quartulli (3): batman-adv: fix reference counting imbalance while sending fragment batman-adv: increase orig refcount when storing ref in gw_node batman-adv: fix local TT check for outgoing arp requests in DAT
Marek Lindner (1): batman-adv: fix indirect hard_iface NULL dereference
net/batman-adv/distributed-arp-table.c | 3 +-- net/batman-adv/fragmentation.c | 11 ++++++++--- net/batman-adv/gateway_client.c | 11 +++++++++-- net/batman-adv/originator.c | 3 ++- 4 files changed, 20 insertions(+), 8 deletions(-)
From: Marek Lindner mareklindner@neomailbox.ch
If hard_iface is NULL and goto out is made batadv_hardif_free_ref() doesn't check for NULL before dereferencing it to get to refcount.
Introduced in cb1c92ec37fb70543d133a1fa7d9b54d6f8a1ecd ("batman-adv: add debugfs support to view multiif tables").
Reported-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Acked-by: Antonio Quartulli antonio@meshcoding.com Signed-off-by: Antonio Quartulli antonio@meshcoding.com --- net/batman-adv/originator.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c index 1785da3..6a48451 100644 --- a/net/batman-adv/originator.c +++ b/net/batman-adv/originator.c @@ -1079,7 +1079,8 @@ int batadv_orig_hardif_seq_print_text(struct seq_file *seq, void *offset) bat_priv->bat_algo_ops->bat_orig_print(bat_priv, seq, hard_iface);
out: - batadv_hardif_free_ref(hard_iface); + if (hard_iface) + batadv_hardif_free_ref(hard_iface); return 0; }
From: Antonio Quartulli antonio@open-mesh.com
In the new fragmentation code the batadv_frag_send_packet() function obtains a reference to the primary_if, but it does not release it upon return.
This reference imbalance prevents the primary_if (and then the related netdevice) to be properly released on shut down.
Fix this by releasing the primary_if in batadv_frag_send_packet().
Introduced by ee75ed88879af88558818a5c6609d85f60ff0df4 ("batman-adv: Fragment and send skbs larger than mtu")
Cc: Martin Hundebøll martin@hundeboll.net Signed-off-by: Antonio Quartulli antonio@open-mesh.com Signed-off-by: Marek Lindner mareklindner@neomailbox.ch Acked-by: Martin Hundebøll martin@hundeboll.net --- net/batman-adv/fragmentation.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index bcc4bea..f14e54a 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -418,12 +418,13 @@ bool batadv_frag_send_packet(struct sk_buff *skb, struct batadv_neigh_node *neigh_node) { struct batadv_priv *bat_priv; - struct batadv_hard_iface *primary_if; + struct batadv_hard_iface *primary_if = NULL; struct batadv_frag_packet frag_header; struct sk_buff *skb_fragment; unsigned mtu = neigh_node->if_incoming->net_dev->mtu; unsigned header_size = sizeof(frag_header); unsigned max_fragment_size, max_packet_size; + bool ret = false;
/* To avoid merge and refragmentation at next-hops we never send * fragments larger than BATADV_FRAG_MAX_FRAG_SIZE @@ -483,7 +484,11 @@ bool batadv_frag_send_packet(struct sk_buff *skb, skb->len + ETH_HLEN); batadv_send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
- return true; + ret = true; + out_err: - return false; + if (primary_if) + batadv_hardif_free_ref(primary_if); + + return ret; }
From: Antonio Quartulli antonio@open-mesh.com
A pointer to the orig_node representing a bat-gateway is stored in the gw_node->orig_node member, but the refcount for such orig_node is never increased. This leads to memory faults when gw_node->orig_node is accessed and the originator has already been freed.
Fix this by increasing the refcount on gw_node creation and decreasing it on gw_node free.
Signed-off-by: Antonio Quartulli antonio@open-mesh.com Signed-off-by: Marek Lindner mareklindner@neomailbox.ch --- net/batman-adv/gateway_client.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c index c835e13..90cff58 100644 --- a/net/batman-adv/gateway_client.c +++ b/net/batman-adv/gateway_client.c @@ -42,8 +42,10 @@
static void batadv_gw_node_free_ref(struct batadv_gw_node *gw_node) { - if (atomic_dec_and_test(&gw_node->refcount)) + if (atomic_dec_and_test(&gw_node->refcount)) { + batadv_orig_node_free_ref(gw_node->orig_node); kfree_rcu(gw_node, rcu); + } }
static struct batadv_gw_node * @@ -406,9 +408,14 @@ static void batadv_gw_node_add(struct batadv_priv *bat_priv, if (gateway->bandwidth_down == 0) return;
+ if (!atomic_inc_not_zero(&orig_node->refcount)) + return; + gw_node = kzalloc(sizeof(*gw_node), GFP_ATOMIC); - if (!gw_node) + if (!gw_node) { + batadv_orig_node_free_ref(orig_node); return; + }
INIT_HLIST_NODE(&gw_node->list); gw_node->orig_node = orig_node;
From: Antonio Quartulli antonio@open-mesh.com
Change introduced by 88e48d7b3340ef07b108eb8a8b3813dd093cc7f7 ("batman-adv: make DAT drop ARP requests targeting local clients") implements a check that prevents DAT from using the caching mechanism when the client that is supposed to provide a reply to an arp request is local.
However change brought by be1db4f6615b5e6156c807ea8985171c215c2d57 ("batman-adv: make the Distributed ARP Table vlan aware") has not converted the above check into its vlan aware version thus making it useless when the local client is behind a vlan.
Fix the behaviour by properly specifying the vlan when checking for a client being local or not.
Reported-by: Simon Wunderlich simon@open-mesh.com Signed-off-by: Antonio Quartulli antonio@open-mesh.com Signed-off-by: Marek Lindner mareklindner@neomailbox.ch --- net/batman-adv/distributed-arp-table.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index b25fd64..aa5d494 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -940,8 +940,7 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv, * additional DAT answer may trigger kernel warnings about * a packet coming from the wrong port. */ - if (batadv_is_my_client(bat_priv, dat_entry->mac_addr, - BATADV_NO_FLAGS)) { + if (batadv_is_my_client(bat_priv, dat_entry->mac_addr, vid)) { ret = true; goto out; }
From: Antonio Quartulli antonio@meshcoding.com Date: Thu, 15 May 2014 20:50:48 +0200
Please pull or let me know of any problem!
Pulled, thanks Antonio.
On 16/05/14 22:29, David Miller wrote:
From: Antonio Quartulli antonio@meshcoding.com Date: Thu, 15 May 2014 20:50:48 +0200
Please pull or let me know of any problem!
Pulled, thanks Antonio.
Thanks a lot David. Please enqueue them for stable as well.
Cheers,
From: Antonio Quartulli antonio@meshcoding.com Date: Fri, 16 May 2014 23:34:16 +0200
On 16/05/14 22:29, David Miller wrote:
From: Antonio Quartulli antonio@meshcoding.com Date: Thu, 15 May 2014 20:50:48 +0200
Please pull or let me know of any problem!
Pulled, thanks Antonio.
Thanks a lot David. Please enqueue them for stable as well.
Done, thanks for reminding me :)
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
David Miller