diff mbox

batman-adv: fix memory access by setting mac_header in DAT

Message ID 1392114403-4069-1-git-send-email-antonio@meshcoding.com (mailing list archive)
State Accepted, archived
Commit df99b07081eeda5cca292afe2dcc5cb3bf5be154
Headers

Commit Message

Antonio Quartulli Feb. 11, 2014, 10:26 a.m. UTC 
  In the TX path we now have functions that rely on the
skb->mac_header field. DAT does not set such field when
creating its own ARP packets thus leading to wrong memory
access.

Fix it by always setting the mac_header after having forged
the ARP packet.

Reported-by: Russel Senior <russell@personaltelco.net>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
Tested-by: Russel Senior <russell@personaltelco.net>
---
 distributed-arp-table.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Antonio Quartulli Feb. 11, 2014, 10:35 a.m. UTC | #1 
On 11/02/14 11:26, Antonio Quartulli wrote:
> In the TX path we now have functions that rely on the
> skb->mac_header field. DAT does not set such field when
> creating its own ARP packets thus leading to wrong memory
> access.
> 
> Fix it by always setting the mac_header after having forged
> the ARP packet.
> 
> Reported-by: Russel Senior <russell@personaltelco.net>
> Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
> Tested-by: Russel Senior <russell@personaltelco.net>

This patch is supposed to be applied on maint.

Cheers,
Antonio Quartulli Feb. 11, 2014, 10:58 a.m. UTC | #2 
On 11/02/14 11:35, Antonio Quartulli wrote:
> On 11/02/14 11:26, Antonio Quartulli wrote:
>> In the TX path we now have functions that rely on the
>> skb->mac_header field. DAT does not set such field when
>> creating its own ARP packets thus leading to wrong memory
>> access.
>>
>> Fix it by always setting the mac_header after having forged
>> the ARP packet.
>>
>> Reported-by: Russel Senior <russell@personaltelco.net>
>> Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
>> Tested-by: Russel Senior <russell@personaltelco.net>
> 
> This patch is supposed to be applied on maint.

Introduced by 41b38727749a94c1a65cf0f4be9bfe1cbaf0adeb
("batman-adv: fix potential kernel paging error for unicast transmissions")

In this patch we have the introduction of eth_hdr() in
batadv_send_skb_unicast() which creates the problem..


Cheers,
Marek Lindner Feb. 11, 2014, 12:08 p.m. UTC | #3 
On Tuesday 11 February 2014 11:58:26 Antonio Quartulli wrote:
> On 11/02/14 11:35, Antonio Quartulli wrote:
> > On 11/02/14 11:26, Antonio Quartulli wrote:
> >> In the TX path we now have functions that rely on the
> >> skb->mac_header field. DAT does not set such field when
> >> creating its own ARP packets thus leading to wrong memory
> >> access.
> >> 
> >> Fix it by always setting the mac_header after having forged
> >> the ARP packet.
> >> 
> >> Reported-by: Russel Senior <russell@personaltelco.net>
> >> Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
> >> Tested-by: Russel Senior <russell@personaltelco.net>
> >
> > 
> >
> > This patch is supposed to be applied on maint.
> 
> Introduced by 41b38727749a94c1a65cf0f4be9bfe1cbaf0adeb
> ("batman-adv: fix potential kernel paging error for unicast transmissions")

Applied in revision df99b07.

Thanks,
Marek
diff mbox

Patch

diff --git a/distributed-arp-table.c b/distributed-arp-table.c
index 6da587a..0b69b61 100644
--- a/distributed-arp-table.c
+++ b/distributed-arp-table.c
@@ -1028,6 +1028,11 @@  bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv,
 	if (!skb_new)
 		goto out;
 
+	/* the rest of the TX path assumes that the mac_header offset pointing
+	 * to the inner Ethernet header has been set, therefore reset it now.
+	 */
+	skb_reset_mac_header(skb_new);
+
 	if (vid & BATADV_VLAN_HAS_TAG)
 		skb_new = vlan_insert_tag(skb_new, htons(ETH_P_8021Q),
 					  vid & VLAN_VID_MASK);