can: restore skb header initialisations in init_can_skb()
Subsystems (2)
PATCH 1/1 can: restore skb header initialisations in init_can_skb()
Commit 9f10374bb024 ("can: remove private CAN skb headroom infrastructure") removed the skb_reset_mac_header()/skb_reset_network_header()/ skb_reset_transport_header() calls from init_can_skb(). As a result, RX skbs from alloc_can_skb() and friends again carry mac_header = 0xFFFF. When such an skb reaches packet_rcv_spkt() (SOCK_PACKET), the push length calculation overflows and triggers skb_under_panic -> kernel BUG -> full machine panic.
The same issue was originally reported in 2014 on linux-can and fixed by commit 969439016d2c ("can: add missing initialisations in CAN related skbuffs"). packet_rcv_spkt() itself has never been hardened: only packet_rcv() and tpacket_rcv() gained dev_has_header() checks in commit d549699048b4 ("net/packet: fix packet receive on L3 devices without visible hard header").
--- drivers/net/can/dev/skb.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/net/can/dev/skb.c +8 -0
@@ -210,6 +210,14 @@ static void init_can_skb(struct sk_buff *skb) { skb->pkt_type =PACKET_BROADCAST ; skb->ip_summed =CHECKSUM_UNNECESSARY ; + +/* Restore the initialisations that were present before +* 9f10374bb024 to prevent skb_under_panic in packet_rcv_spkt +* and similar consumers. +*/ +skb_reset_mac_header (skb); +skb_reset_network_header (skb); +skb_reset_transport_header (skb); }struct sk_buff *alloc_can_skb (struct net_device *dev,struct can_frame **cf)
Signature · 1 line
2.53.0
Hello zjamg!
Many thanks for the patch!
Quoted 8 lines · zjamg
Commit
9f10374bb024("can: remove private CAN skb headroom infrastructure") removed the skb_reset_mac_header()/skb_reset_network_header()/skb_reset_transport_header()calls frominit_can_skb(). As a result, RX skbs fromalloc_can_skb()and friends again carrymac_header=0xFFFF. When such an skb reachespacket_rcv_spkt()(SOCK_PACKET), the push length calculation overflows and triggersskb_under_panic-> kernel BUG -> full machine panic.
Quoted 6 lines
The same issue was originally reported in 2014 on linux-can and fixed by commit
969439016d2c("can: add missing initialisations in CAN related skbuffs").packet_rcv_spkt()itself has never been hardened: onlypacket_rcv()andtpacket_rcv()gaineddev_has_header()checks in commitd549699048b4("net/packet: fix packet receive on L3 devices without visible hard header").
Right. My bad.
I looked into netdev_alloc_skb() to see the the values were initialized and therefore thought I can leave it out.
But there are initialized in a way that the header settings do not present valid settings, e.g. mac_header = 0xFFFF
So the functions definitely need to be called as you correctly remarked.
Quoted 3 lines
Fixes 9f10374bb024 ("can: remove private CAN skb headroom infrastructure")Signed-off-by zjamg <[email protected]>
You can add my
in your v2 patch.
Quoted 9 lines
--- drivers/net/can/dev/skb.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/net/can/dev/skb.c +0
-0
drivers/net/can/dev/skb.c static void init_can_skb(struct sk_buff *skb) · line 217
Quoted 8 lines
drivers/net/can/dev/skb.c +5
-0@@ -210,6 +210,14 @@ static void init_can_skb(struct sk_buff *skb) { skb->pkt_type =PACKET_BROADCAST ; skb->ip_summed =CHECKSUM_UNNECESSARY ; + +/* Restore the initialisations that were present before +* 9f10374bb024 to prevent skb_under_panic in packet_rcv_spkt +* and similar consumers. +*/
Please remove this comment in v2 - just add the code.
This is not usual and the patch description perfectly serves the documentation needs.
Thanks & best regards,
Oliver
Quoted 6 lines
drivers/net/can/dev/skb.c +3
-0+skb_reset_mac_header (skb); +skb_reset_network_header (skb); +skb_reset_transport_header (skb); }struct sk_buff *alloc_can_skb (struct net_device *dev,struct can_frame **cf)