On 09/12/2019 17.05, Marc Kleine-Budde wrote:
From: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
KMSAN sysbot detected a read access to an untinitialized value in the headroom
of an outgoing CAN related sk_buff. When using CAN sockets this area is filled
appropriately - but when using a packet socket this initialization is missing.
The problematic read access occurs in the CAN receive path which can only be
triggered when the sk_buff is sent through a (virtual) CAN interface. So we
check in the sending path whether we need to perform the missing
initializations.
Fixes: d3b58c47d330d ("can: replace timestamp as unique skb attribute")
Reported-by: syzbot+b02ff0707a97e4e79ebb@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
---
Changes since v1
- rename to can_skb_headroom_valid()
- reverse logic
- move to dev.c
Applying this patch and also my v1 patch to Linux 4.1 fails in both cases due to cosmetic changes (int to bool, comment style clean-up).
So we can not create a patch which is more convenient to be applied to stable Linux 4.1+ anyway.
Feel free to add Cc: linux-stable <stable@xxxxxxxxxxxxxxx> # >= v4.1 Thanks, Oliver
drivers/net/can/dev.c | 30 ++++++++++++++++++++++++++++++ include/linux/can/dev.h | 5 +++++ 2 files changed, 35 insertions(+) diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c index 6ee06a49fb4c..14f1d9ee28a5 100644 --- a/drivers/net/can/dev.c +++ b/drivers/net/can/dev.c @@ -403,6 +403,36 @@ void can_change_state(struct net_device *dev, struct can_frame *cf, } EXPORT_SYMBOL_GPL(can_change_state);+/* Check for outgoing skbs that have not been created by the CAN subsystem */+bool can_skb_headroom_valid(struct net_device *dev, struct sk_buff *skb) +{ + /* af_packet creates a headroom of HH_DATA_MOD bytes which is fine */ + if (WARN_ON_ONCE(skb_headroom(skb) < sizeof(struct can_skb_priv))) + return false; + + /* af_packet does not apply CAN skb specific settings */ + if (skb->ip_summed == CHECKSUM_NONE) { + /* init headroom */ + can_skb_prv(skb)->ifindex = dev->ifindex; + can_skb_prv(skb)->skbcnt = 0; + + skb->ip_summed = CHECKSUM_UNNECESSARY; + + /* preform proper loopback on capable devices */ + if (dev->flags & IFF_ECHO) + skb->pkt_type = PACKET_LOOPBACK; + else + skb->pkt_type = PACKET_HOST; + + skb_reset_mac_header(skb); + skb_reset_network_header(skb); + skb_reset_transport_header(skb); + } + + return true; +} +EXPORT_SYMBOL_GPL(can_skb_headroom_valid); + /* Local echo of CAN messages * * CAN network devices *should* support a local echo functionality diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h index 9b3c720a31b1..ca087894458b 100644 --- a/include/linux/can/dev.h +++ b/include/linux/can/dev.h @@ -91,6 +91,8 @@ struct can_priv { #define get_can_dlc(i) (min_t(__u8, (i), CAN_MAX_DLC)) #define get_canfd_dlc(i) (min_t(__u8, (i), CANFD_MAX_DLC))+bool can_skb_headroom_valid(struct net_device *dev, struct sk_buff *skb);+ /* Drop a given socketbuffer if it does not contain a valid CAN frame. */ static inline bool can_dropped_invalid_skb(struct net_device *dev, struct sk_buff *skb) @@ -108,6 +110,9 @@ static inline bool can_dropped_invalid_skb(struct net_device *dev, } else goto inval_skb;+ if (!can_skb_headroom_valid(dev, skb))+ goto inval_skb; + return false;inval_skb:
