here you have four fixes intended for net.
1/4 fixes the parsing of a string sent from userspace in order to avoid random memory access in case of string length of 0.
2/4 adds a check for the return value of pskb_trim_rcsum() in order to stop processing the skb in case of failure.
3/4 prevents DAT (the Distributed ARP Table) to send cached ARP replies when both the source and the destination of the snooped ARP request are local clients (meaning: directly connected to the node). This can confuse a bridge where batman-adv is enslaved.
4/4 fix a race condition in the main clean up procedure by reordering sub-components freeing function invocations.
Please pull or let me know if there is any problem.
Thanks a lot, Antonio
The following changes since commit 4f924b2aa4d3cb30f07e57d6b608838edcbc0d88:
if_cablemodem.h: Add parenthesis around ioctl macros (2013-05-08 13:13:30 -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 a4361860351e87876aebd9595906d928ce8572c6:
batman-adv: reorder clean up routine in order to avoid race conditions (2013-05-09 12:39:45 +0200)
---------------------------------------------------------------- Included changes: - fix parsing of user typed protocol string to avoid random memory access in some cases - check pskb_trim_rcsum() return value - prevent DAT from sending ARP replies when not needed - reorder the main clean up routine to prevent race conditions
---------------------------------------------------------------- Antonio Quartulli (2): batman-adv: make DAT drop ARP requests targeting local clients batman-adv: reorder clean up routine in order to avoid race conditions
Marek Lindner (2): batman-adv: check proto length before accessing proto string buffer batman-adv: check return value of pskb_trim_rcsum()
net/batman-adv/distributed-arp-table.c | 13 +++++++++++++ net/batman-adv/main.c | 18 +++++++++++++----- net/batman-adv/network-coding.c | 8 ++++++-- 3 files changed, 32 insertions(+), 7 deletions(-)
From: Marek Lindner lindner_marek@yahoo.de
batadv_param_set_ra() strips the trailing '\n' from the supplied string buffer without checking the length of the buffer first. This patches avoids random memory access and associated potential crashes.
Reported-by: Sasha Levin sasha.levin@oracle.com Signed-off-by: Marek Lindner lindner_marek@yahoo.de Signed-off-by: Antonio Quartulli ordex@autistici.org --- net/batman-adv/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index 3e30a0f..9c620cd 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c @@ -475,7 +475,7 @@ static int batadv_param_set_ra(const char *val, const struct kernel_param *kp) char *algo_name = (char *)val; size_t name_len = strlen(algo_name);
- if (algo_name[name_len - 1] == '\n') + if (name_len > 0 && algo_name[name_len - 1] == '\n') algo_name[name_len - 1] = '\0';
bat_algo_ops = batadv_algo_get(algo_name);
From: Marek Lindner lindner_marek@yahoo.de
Reported-by: Sven Eckelmann sven@narfation.org Signed-off-by: Marek Lindner lindner_marek@yahoo.de Acked-by: Martin Hundebøll martin@hundeboll.net Signed-off-by: Antonio Quartulli ordex@autistici.org --- net/batman-adv/network-coding.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c index f7c5430..e84629e 100644 --- a/net/batman-adv/network-coding.c +++ b/net/batman-adv/network-coding.c @@ -1514,6 +1514,7 @@ batadv_nc_skb_decode_packet(struct batadv_priv *bat_priv, struct sk_buff *skb, struct ethhdr *ethhdr, ethhdr_tmp; uint8_t *orig_dest, ttl, ttvn; unsigned int coding_len; + int err;
/* Save headers temporarily */ memcpy(&coded_packet_tmp, skb->data, sizeof(coded_packet_tmp)); @@ -1568,8 +1569,11 @@ batadv_nc_skb_decode_packet(struct batadv_priv *bat_priv, struct sk_buff *skb, coding_len);
/* Resize decoded skb if decoded with larger packet */ - if (nc_packet->skb->len > coding_len + h_size) - pskb_trim_rcsum(skb, coding_len + h_size); + if (nc_packet->skb->len > coding_len + h_size) { + err = pskb_trim_rcsum(skb, coding_len + h_size); + if (err) + return NULL; + }
/* Create decoded unicast packet */ unicast_packet = (struct batadv_unicast_packet *)skb->data;
In the outgoing ARP request snooping routine in DAT, ARP Request sent by local clients which are supposed to be replied by other local clients can be silently dropped.
The destination host will reply by itself through the LAN and therefore there is no need to involve DAT.
Reported-by: Carlos Quijano carlos@crqgestion.es Signed-off-by: Antonio Quartulli ordex@autistici.org Tested-by: Carlos Quijano carlos@crqgestion.es Signed-off-by: Marek Lindner lindner_marek@yahoo.de --- net/batman-adv/distributed-arp-table.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c index 8e15d96..2399920 100644 --- a/net/batman-adv/distributed-arp-table.c +++ b/net/batman-adv/distributed-arp-table.c @@ -837,6 +837,19 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv,
dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_dst); if (dat_entry) { + /* If the ARP request is destined for a local client the local + * client will answer itself. DAT would only generate a + * duplicate packet. + * + * Moreover, if the soft-interface is enslaved into a bridge, an + * 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)) { + ret = true; + goto out; + } + skb_new = arp_create(ARPOP_REPLY, ETH_P_ARP, ip_src, bat_priv->soft_iface, ip_dst, hw_src, dat_entry->mac_addr, hw_src);
nc_worker accesses the originator table during its periodic work, but since the originator table is freed before stopping the worker this leads to a global protection fault.
Fix this by killing the worker (in nc_free) before freeing the originator table.
Moreover tidy up the entire clean up routine by running all the subcomponents freeing procedures first and then killing the TT and the originator tables at the end.
Signed-off-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Marek Lindner lindner_marek@yahoo.de --- net/batman-adv/main.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index 9c620cd..1240f07 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c @@ -163,14 +163,22 @@ void batadv_mesh_free(struct net_device *soft_iface) batadv_vis_quit(bat_priv);
batadv_gw_node_purge(bat_priv); - batadv_originator_free(bat_priv); batadv_nc_free(bat_priv); - - batadv_tt_free(bat_priv); - - batadv_bla_free(bat_priv); - batadv_dat_free(bat_priv); + batadv_bla_free(bat_priv); + + /* Free the TT and the originator tables only after having terminated + * all the other depending components which may use these structures for + * their purposes. + */ + batadv_tt_free(bat_priv); + + /* Since the originator table clean up routine is accessing the TT + * tables as well, it has to be invoked after the TT tables have been + * freed and marked as empty. This ensures that no cleanup RCU callbacks + * accessing the TT data are scheduled for later execution. + */ + batadv_originator_free(bat_priv);
free_percpu(bat_priv->bat_counters);
From: Antonio Quartulli ordex@autistici.org Date: Thu, 9 May 2013 12:56:43 +0200
here you have four fixes intended for net.
1/4 fixes the parsing of a string sent from userspace in order to avoid random memory access in case of string length of 0.
2/4 adds a check for the return value of pskb_trim_rcsum() in order to stop processing the skb in case of failure.
3/4 prevents DAT (the Distributed ARP Table) to send cached ARP replies when both the source and the destination of the snooped ARP request are local clients (meaning: directly connected to the node). This can confuse a bridge where batman-adv is enslaved.
4/4 fix a race condition in the main clean up procedure by reordering sub-components freeing function invocations.
Please pull or let me know if there is any problem.
Pulled, thanks Antonio.
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
David Miller