Skip to contentSkip to conversation

atl1c: reserve min skb headroom

PATCH In review Florian Westphal 3 participants 3 messages Jul 20, 2018 – Jul 22, 2018

Subsystems (3)
Networking Network drivers Atlx Ethernet drivers
lore
Share & tools
lore

Florian Westphal

author
#0 lore raw

PATCH 1/1 atl1c: reserve min skb headroom

Got crash report with following backtrace:

BUG: unable to handle kernel paging request at ffff8801869daffe
RIP: 0010:[<ffffffff816429c4>]  [<ffffffff816429c4>] ip6_finish_output2+0x394/0x4c0
RSP: 0018:ffff880186c83a98  EFLAGS: 00010283
RAX: ffff8801869db00e ...
  [<ffffffff81644cdc>] ip6_finish_output+0x8c/0xf0
  [<ffffffff81644d97>] ip6_output+0x57/0x100
  [<ffffffff81643dc9>] ip6_forward+0x4b9/0x840
  [<ffffffff81645566>] ip6_rcv_finish+0x66/0xc0
  [<ffffffff81645db9>] ipv6_rcv+0x319/0x530
  [<ffffffff815892ac>] netif_receive_skb+0x1c/0x70
  [<ffffffffc0060bec>] atl1c_clean+0x1ec/0x310 [atl1c]
  ...

The bad access is in neigh_hh_output(), at skb->data - 16 (HH_DATA_MOD). atl1c driver provided skb with no headroom, so 14 bytes (ethernet header) got pulled, but then 16 are copied.

Reserve NET_SKB_PAD bytes headroom, like netdev_alloc_skb().

Compile tested only; I lack hardware.

Fixes 7b7017642199 ("atl1c: Fix misuse of netdev_alloc_skb in refilling rx ring")
Signed-off-by Florian Westphal <[email protected]>

---

drivers/net/ethernet/atheros/atl1c/atl1c_main.c +1 -0
index 94270f654b3b..7087b88550db 100644
@@ -1686,6 +1686,7 @@ static struct sk_buff *atl1c_alloc_skb(struct atl1c_adapter *adapter)
 	skb = build_skb(page_address(page) + adapter->rx_page_offset,
 			adapter->rx_frag_size);
 	if (likely(skb)) {
+		skb_reserve(skb, NET_SKB_PAD);
 		adapter->rx_page_offset += adapter->rx_frag_size;
 		if (adapter->rx_page_offset >= PAGE_SIZE)
 			adapter->rx_page = NULL;
Signature · 1 line
2.16.4
Reviewed-by on v1 1/1
#1 lore raw
Quoted 31 lines · Florian Westphal

Got crash report with following backtrace:

BUG: unable to handle kernel paging request at ffff8801869daffe
RIP: 0010:[<ffffffff816429c4>]  [<ffffffff816429c4>] ip6_finish_output2+0x394/0x4c0
RSP: 0018:ffff880186c83a98  EFLAGS: 00010283
RAX: ffff8801869db00e ...
  [<ffffffff81644cdc>] ip6_finish_output+0x8c/0xf0
  [<ffffffff81644d97>] ip6_output+0x57/0x100
  [<ffffffff81643dc9>] ip6_forward+0x4b9/0x840
  [<ffffffff81645566>] ip6_rcv_finish+0x66/0xc0
  [<ffffffff81645db9>] ipv6_rcv+0x319/0x530
  [<ffffffff815892ac>] netif_receive_skb+0x1c/0x70
  [<ffffffffc0060bec>] atl1c_clean+0x1ec/0x310 [atl1c]
  ...

The bad access is in neigh_hh_output(), at skb->data - 16 (HH_DATA_MOD). atl1c driver provided skb with no headroom, so 14 bytes (ethernet header) got pulled, but then 16 are copied.

Reserve NET_SKB_PAD bytes headroom, like netdev_alloc_skb().

Compile tested only; I lack hardware.

Fixes 7b7017642199 ("atl1c: Fix misuse of netdev_alloc_skb in refilling rx ring")
Signed-off-by Florian Westphal <[email protected]>

---

drivers/net/ethernet/atheros/atl1c/atl1c_main.c +0 -0
index 94270f654b3b..7087b88550db 100644

drivers/net/ethernet/atheros/atl1c/atl1c_main.c static struct sk_buff *atl1c_alloc_skb(struct atl1c_adapter *adapter) · line 1692

Quoted 7 lines
drivers/net/ethernet/atheros/atl1c/atl1c_main.c +1 -0
@@ -1686,6 +1686,7 @@ static struct sk_buff *atl1c_alloc_skb(struct atl1c_adapter *adapter)
 	skb = build_skb(page_address(page) + adapter->rx_page_offset,
 			adapter->rx_frag_size);
 	if (likely(skb)) {
+		skb_reserve(skb, NET_SKB_PAD);
 		adapter->rx_page_offset += adapter->rx_frag_size;
 		if (adapter->rx_page_offset >= PAGE_SIZE)
 			adapter->rx_page = NULL;

Yes, it is interesting IPv4 has code to deal with that( in ip_finish_output2()), not IPv6 :/

Reviewed-by Eric Dumazet <[email protected]>
maintainer
#2 lore raw
In reply to Florian Westphal

From: Florian Westphal <[email protected]>
Date: Fri, 20 Jul 2018 19:30:57 +0200

Quoted 18 lines

Got crash report with following backtrace:

BUG: unable to handle kernel paging request at ffff8801869daffe
RIP: 0010:[<ffffffff816429c4>]  [<ffffffff816429c4>] ip6_finish_output2+0x394/0x4c0
RSP: 0018:ffff880186c83a98  EFLAGS: 00010283
RAX: ffff8801869db00e ...
  [<ffffffff81644cdc>] ip6_finish_output+0x8c/0xf0
  [<ffffffff81644d97>] ip6_output+0x57/0x100
  [<ffffffff81643dc9>] ip6_forward+0x4b9/0x840
  [<ffffffff81645566>] ip6_rcv_finish+0x66/0xc0
  [<ffffffff81645db9>] ipv6_rcv+0x319/0x530
  [<ffffffff815892ac>] netif_receive_skb+0x1c/0x70
  [<ffffffffc0060bec>] atl1c_clean+0x1ec/0x310 [atl1c]
  ...

The bad access is in neigh_hh_output(), at skb->data - 16 (HH_DATA_MOD). atl1c driver provided skb with no headroom, so 14 bytes (ethernet header) got pulled, but then 16 are copied.

Quoted 6 lines

Reserve NET_SKB_PAD bytes headroom, like netdev_alloc_skb().

Compile tested only; I lack hardware.

Fixes 7b7017642199 ("atl1c: Fix misuse of netdev_alloc_skb in refilling rx ring")
Signed-off-by Florian Westphal <[email protected]>

Ancient bug :-/

Applied and queued up for -stable, thanks Florian.