B.A.T.M.A.N August 2013

b.a.t.m.a.n@lists.open-mesh.org

12 participants
45 discussions

[B.A.T.M.A.N.] [PATCHv4 next] batman-adv: fix potential kernel paging errors for unicast transmissions
by Linus Lüssing 07 Aug '13

07 Aug '13

There are several functions which might reallocate skb data. Currently some places keep reusing their old ethhdr pointer regardless of whether they became invalid after such a reallocation or not. This potentially leads to kernel paging errors. This patch fixes these by refetching the ethdr pointer after the potential reallocations. Signed-off-by: Linus Lüssing <linus.luessing(a)web.de> --- v4: * Reassign new ethhdr correctly in batadv_unicast_generic_send_skb() v3: * Remove the change from 'Returns' to 'Return * Avoid using eth_hdr(skb), using (struct ethhdr *)skb->data instead * Added fix for batadv_gw_is_dhcp_target() too bridge_loop_avoidance.c | 2 ++ gateway_client.c | 12 +++++++++++- gateway_client.h | 3 +-- soft-interface.c | 8 +++++++- unicast.c | 12 ++++++++++-- 5 files changed, 31 insertions(+), 6 deletions(-) diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c index e14531f..264de88 100644 --- a/bridge_loop_avoidance.c +++ b/bridge_loop_avoidance.c @@ -1529,6 +1529,8 @@ out: * in these cases, the skb is further handled by this function and * returns 1, otherwise it returns 0 and the caller shall further * process the skb. + * + * This call might reallocate skb data. */ int batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb, unsigned short vid) diff --git a/gateway_client.c b/gateway_client.c index f105219..e1516d5 100644 --- a/gateway_client.c +++ b/gateway_client.c @@ -508,6 +508,7 @@ out: return 0; } +/* this call might reallocate skb data */ static bool batadv_is_type_dhcprequest(struct sk_buff *skb, int header_len) { int ret = false; @@ -568,6 +569,7 @@ out: return ret; } +/* this call might reallocate skb data */ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len) { struct ethhdr *ethhdr; @@ -619,6 +621,11 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len) if (!pskb_may_pull(skb, *header_len + sizeof(*udphdr))) return false; + + ethhdr = (struct ethhdr *)skb->data; + if (ntohs(ethhdr->h_proto) == ETH_P_8021Q) + ethhdr = (struct ethhdr *)(skb->data + VLAN_HLEN); + udphdr = (struct udphdr *)(skb->data + *header_len); *header_len += sizeof(*udphdr); @@ -634,12 +641,14 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len) return true; } +/* this call might reallocate skb data */ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, - struct sk_buff *skb, struct ethhdr *ethhdr) + struct sk_buff *skb) { struct batadv_neigh_node *neigh_curr = NULL, *neigh_old = NULL; struct batadv_orig_node *orig_dst_node = NULL; struct batadv_gw_node *curr_gw = NULL; + struct ethhdr *ethhdr; bool ret, out_of_range = false; unsigned int header_len = 0; uint8_t curr_tq_avg; @@ -648,6 +657,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, if (!ret) goto out; + ethhdr = (struct ethhdr *)skb->data; orig_dst_node = batadv_transtable_search(bat_priv, ethhdr->h_source, ethhdr->h_dest); if (!orig_dst_node) diff --git a/gateway_client.h b/gateway_client.h index 039902d..1037d75 100644 --- a/gateway_client.h +++ b/gateway_client.h @@ -34,7 +34,6 @@ void batadv_gw_node_delete(struct batadv_priv *bat_priv, void batadv_gw_node_purge(struct batadv_priv *bat_priv); int batadv_gw_client_seq_print_text(struct seq_file *seq, void *offset); bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len); -bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, - struct sk_buff *skb, struct ethhdr *ethhdr); +bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, struct sk_buff *skb); #endif /* _NET_BATMAN_ADV_GATEWAY_CLIENT_H_ */ diff --git a/soft-interface.c b/soft-interface.c index 700d0b4..b39e50d 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -180,6 +180,8 @@ static int batadv_interface_tx(struct sk_buff *skb, if (batadv_bla_tx(bat_priv, skb, vid)) goto dropped; + ethhdr = (struct ethhdr *)skb->data; + /* Register the client MAC in the transtable */ if (!is_multicast_ether_addr(ethhdr->h_source)) batadv_tt_local_add(soft_iface, ethhdr->h_source, skb->skb_iif); @@ -220,6 +222,10 @@ static int batadv_interface_tx(struct sk_buff *skb, default: break; } + + /* reminder: ethhdr might have become unusable from here on + * (batadv_gw_is_dhcp_target() might have reallocated skb data) + */ } /* ethernet packet should be broadcasted */ @@ -266,7 +272,7 @@ static int batadv_interface_tx(struct sk_buff *skb, /* unicast packet */ } else { if (atomic_read(&bat_priv->gw_mode) != BATADV_GW_MODE_OFF) { - ret = batadv_gw_out_of_range(bat_priv, skb, ethhdr); + ret = batadv_gw_out_of_range(bat_priv, skb); if (ret) goto dropped; } diff --git a/unicast.c b/unicast.c index 4c5a1aa..49ffb7c 100644 --- a/unicast.c +++ b/unicast.c @@ -326,7 +326,9 @@ static bool batadv_unicast_push_and_fill_skb(struct sk_buff *skb, int hdr_size, * @skb: the skb containing the payload to encapsulate * @orig_node: the destination node * - * Returns false if the payload could not be encapsulated or true otherwise + * Returns false if the payload could not be encapsulated or true otherwise. + * + * This call might reallocate skb data. */ static bool batadv_unicast_prepare_skb(struct sk_buff *skb, struct batadv_orig_node *orig_node) @@ -343,7 +345,9 @@ static bool batadv_unicast_prepare_skb(struct sk_buff *skb, * @orig_node: the destination node * @packet_subtype: the batman 4addr packet subtype to use * - * Returns false if the payload could not be encapsulated or true otherwise + * Returns false if the payload could not be encapsulated or true otherwise. + * + * This call might reallocate skb data. */ bool batadv_unicast_4addr_prepare_skb(struct batadv_priv *bat_priv, struct sk_buff *skb, @@ -402,6 +406,7 @@ int batadv_unicast_generic_send_skb(struct batadv_priv *bat_priv, int data_len = skb->len; int ret = NET_RX_DROP; unsigned int dev_mtu; + unsigned int header_len; /* get routing information */ if (is_multicast_ether_addr(ethhdr->h_dest)) { @@ -430,11 +435,13 @@ find_router: case BATADV_UNICAST: if (!batadv_unicast_prepare_skb(skb, orig_node)) goto out; + header_len = sizeof(struct batadv_unicast_packet); break; case BATADV_UNICAST_4ADDR: if (!batadv_unicast_4addr_prepare_skb(bat_priv, skb, orig_node, packet_subtype)) goto out; + header_len = sizeof(struct batadv_unicast_4addr_packet); break; default: /* this function supports UNICAST and UNICAST_4ADDR only. It @@ -443,6 +450,7 @@ find_router: goto out; } + ethhdr = (struct ethhdr *)(skb->data + header_len); unicast_packet = (struct batadv_unicast_packet *)skb->data; /* inform the destination node that we are still missing a correct route -- 1.7.10.4

2 1

[B.A.T.M.A.N.] [PATCHv3 next] batman-adv: fix potential kernel paging errors for unicast transmissions
by Linus Lüssing 06 Aug '13

06 Aug '13

There are several functions which might reallocate skb data. Currently some places keep reusing their old ethhdr pointer regardless of whether they became invalid after such a reallocation or not. This potentially leads to kernel paging errors. This patch fixes these by refetching the ethdr pointer after the potential reallocations. Signed-off-by: Linus Lüssing <linus.luessing(a)web.de> --- v3: * Remove the change from 'Returns' to 'Return * Avoid using eth_hdr(skb), using (struct ethhdr *)skb->data instead * Added fix for batadv_gw_is_dhcp_target() too bridge_loop_avoidance.c | 2 ++ gateway_client.c | 12 +++++++++++- gateway_client.h | 3 +-- soft-interface.c | 8 +++++++- unicast.c | 9 +++++++-- 5 files changed, 28 insertions(+), 6 deletions(-) diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c index e14531f..264de88 100644 --- a/bridge_loop_avoidance.c +++ b/bridge_loop_avoidance.c @@ -1529,6 +1529,8 @@ out: * in these cases, the skb is further handled by this function and * returns 1, otherwise it returns 0 and the caller shall further * process the skb. + * + * This call might reallocate skb data. */ int batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb, unsigned short vid) diff --git a/gateway_client.c b/gateway_client.c index f105219..e1516d5 100644 --- a/gateway_client.c +++ b/gateway_client.c @@ -508,6 +508,7 @@ out: return 0; } +/* this call might reallocate skb data */ static bool batadv_is_type_dhcprequest(struct sk_buff *skb, int header_len) { int ret = false; @@ -568,6 +569,7 @@ out: return ret; } +/* this call might reallocate skb data */ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len) { struct ethhdr *ethhdr; @@ -619,6 +621,11 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len) if (!pskb_may_pull(skb, *header_len + sizeof(*udphdr))) return false; + + ethhdr = (struct ethhdr *)skb->data; + if (ntohs(ethhdr->h_proto) == ETH_P_8021Q) + ethhdr = (struct ethhdr *)(skb->data + VLAN_HLEN); + udphdr = (struct udphdr *)(skb->data + *header_len); *header_len += sizeof(*udphdr); @@ -634,12 +641,14 @@ bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len) return true; } +/* this call might reallocate skb data */ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, - struct sk_buff *skb, struct ethhdr *ethhdr) + struct sk_buff *skb) { struct batadv_neigh_node *neigh_curr = NULL, *neigh_old = NULL; struct batadv_orig_node *orig_dst_node = NULL; struct batadv_gw_node *curr_gw = NULL; + struct ethhdr *ethhdr; bool ret, out_of_range = false; unsigned int header_len = 0; uint8_t curr_tq_avg; @@ -648,6 +657,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, if (!ret) goto out; + ethhdr = (struct ethhdr *)skb->data; orig_dst_node = batadv_transtable_search(bat_priv, ethhdr->h_source, ethhdr->h_dest); if (!orig_dst_node) diff --git a/gateway_client.h b/gateway_client.h index 039902d..1037d75 100644 --- a/gateway_client.h +++ b/gateway_client.h @@ -34,7 +34,6 @@ void batadv_gw_node_delete(struct batadv_priv *bat_priv, void batadv_gw_node_purge(struct batadv_priv *bat_priv); int batadv_gw_client_seq_print_text(struct seq_file *seq, void *offset); bool batadv_gw_is_dhcp_target(struct sk_buff *skb, unsigned int *header_len); -bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, - struct sk_buff *skb, struct ethhdr *ethhdr); +bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, struct sk_buff *skb); #endif /* _NET_BATMAN_ADV_GATEWAY_CLIENT_H_ */ diff --git a/soft-interface.c b/soft-interface.c index 700d0b4..b39e50d 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -180,6 +180,8 @@ static int batadv_interface_tx(struct sk_buff *skb, if (batadv_bla_tx(bat_priv, skb, vid)) goto dropped; + ethhdr = (struct ethhdr *)skb->data; + /* Register the client MAC in the transtable */ if (!is_multicast_ether_addr(ethhdr->h_source)) batadv_tt_local_add(soft_iface, ethhdr->h_source, skb->skb_iif); @@ -220,6 +222,10 @@ static int batadv_interface_tx(struct sk_buff *skb, default: break; } + + /* reminder: ethhdr might have become unusable from here on + * (batadv_gw_is_dhcp_target() might have reallocated skb data) + */ } /* ethernet packet should be broadcasted */ @@ -266,7 +272,7 @@ static int batadv_interface_tx(struct sk_buff *skb, /* unicast packet */ } else { if (atomic_read(&bat_priv->gw_mode) != BATADV_GW_MODE_OFF) { - ret = batadv_gw_out_of_range(bat_priv, skb, ethhdr); + ret = batadv_gw_out_of_range(bat_priv, skb); if (ret) goto dropped; } diff --git a/unicast.c b/unicast.c index 4c5a1aa..7250b10 100644 --- a/unicast.c +++ b/unicast.c @@ -326,7 +326,9 @@ static bool batadv_unicast_push_and_fill_skb(struct sk_buff *skb, int hdr_size, * @skb: the skb containing the payload to encapsulate * @orig_node: the destination node * - * Returns false if the payload could not be encapsulated or true otherwise + * Returns false if the payload could not be encapsulated or true otherwise. + * + * This call might reallocate skb data. */ static bool batadv_unicast_prepare_skb(struct sk_buff *skb, struct batadv_orig_node *orig_node) @@ -343,7 +345,9 @@ static bool batadv_unicast_prepare_skb(struct sk_buff *skb, * @orig_node: the destination node * @packet_subtype: the batman 4addr packet subtype to use * - * Returns false if the payload could not be encapsulated or true otherwise + * Returns false if the payload could not be encapsulated or true otherwise. + * + * This call might reallocate skb data. */ bool batadv_unicast_4addr_prepare_skb(struct batadv_priv *bat_priv, struct sk_buff *skb, @@ -444,6 +448,7 @@ find_router: } unicast_packet = (struct batadv_unicast_packet *)skb->data; + ethhdr = (struct ethhdr *)skb->data; /* inform the destination node that we are still missing a correct route * for this client. The destination will receive this packet and will -- 1.7.10.4

1 0

[B.A.T.M.A.N.] [PATCH 0/2] TT-VLAN: BLA2 integration
by Antonio Quartulli 06 Aug '13

06 Aug '13

Hello people, this is the last patchset of the TT-VLAN series and requires the last one to be applied (per-VLAN TT CRC). In a few words... These changes enable BLA2 to filter only those TT entries belonging to the VLAN where BLA discovered other backbone nodes. The current code instead assumes that _all_ the local clients are shared among the backbones, despite of the VLAN used to interconnect them. Example. Imagine 2 nodes having several vlans over the wired connection. Node1 having eth0.1 and eth0.2 Node2 having eth0.1 and eth0.3 Node1 and Node2 are connected using a cable and so they hear each other on eth0.1. Clients connected to eth0.2 and eth0.3 are reachable only through the respective nodes therefore, if Node1 wanted to talk to a client connected to eth0.3 needs to know that it is served by Node2. In the current implementation this is not possible because BLA would consider all the clients reachable through any node of the LAN and will reject all the global TT entries from any other backbone, so losing this TT information. With this patchset, instead, BLA will detect that the backbone connection is only over eth0.1 and so will continue to store all the global clients belonging to the other VLANs. I hope the example did not create more confusion :) Cheers, Antonio Quartulli (2): batman-adv: make the TT global purge routine VLAN specific batman-adv: make the backbone gw check VLAN specific bridge_loop_avoidance.c | 23 +++++++++++--------- bridge_loop_avoidance.h | 10 +++++---- originator.c | 2 +- routing.c | 2 +- translation-table.c | 58 ++++++++++++++++++++++--------------------------- translation-table.h | 2 +- 6 files changed, 48 insertions(+), 49 deletions(-) -- 1.8.1.5

1 2

[B.A.T.M.A.N.] (no subject)
by Linus Lüssing 05 Aug '13

05 Aug '13

Hi, This second version adds a few more cases I could spot which look like trouble. Secondly, instead of copying the destination mac on the stack it simply uses eth_hdr(). Why do we need no skb_reset_mac_header() or skb_set_mac_header()? Because batadv_skb_head_push() calls skb_headers_offset_update() after the reallocation (via skb_cow_head()->__skb_cow()->pskb_expand_head()-> skb_headers_offset_update(). I also checked that the ether header in the newly allocated buffer is not going to end up being fragmented. But looks like pskb_expand_head() actually reserves enough space for both the old ethernet header plus the newly requested head space. So if the mac header was set correctly before, then eth_hdr() will point us to the right one already. This patch is maybe not so straight forward, you need to read quite skb code. At least not as straight forward as the previous version of this patch. So I leave it up to you to decide whether this is suitable for next or whether you think there's a potential for unseen pitfalls, therefore making it suitable for master only :). This patch fixed the kernel panic I was reproduceably getting just as well. (Which was while sending multicast packets through the unicast functions while having a bridge on top of bat0) Cheers, Linus

2 2

[B.A.T.M.A.N.] [PATCH next 1/2] batman-adv: check return type of unicast packet preparations
by Linus Lüssing 02 Aug '13

02 Aug '13

batadv_send_skb_prepare_unicast(_4addr) might reallocate the skb's data. And if it tries to do so then this can potentially fail. We shouldn't continue working on this skb in such a case. Signed-off-by: Linus Lüssing <linus.luessing(a)web.de> --- unicast.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/unicast.c b/unicast.c index dc8b5d4..4c5a1aa 100644 --- a/unicast.c +++ b/unicast.c @@ -428,11 +428,13 @@ find_router: switch (packet_type) { case BATADV_UNICAST: - batadv_unicast_prepare_skb(skb, orig_node); + if (!batadv_unicast_prepare_skb(skb, orig_node)) + goto out; break; case BATADV_UNICAST_4ADDR: - batadv_unicast_4addr_prepare_skb(bat_priv, skb, orig_node, - packet_subtype); + if (!batadv_unicast_4addr_prepare_skb(bat_priv, skb, orig_node, + packet_subtype)) + goto out; break; default: /* this function supports UNICAST and UNICAST_4ADDR only. It -- 1.7.10.4

3 7

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

B.A.T.M.A.N August 2013