On 16/12/2019 18.17, Oliver Hartkopp wrote:
On 10/12/2019 11.31, 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>
When do you want to push it upstream?
... together with the hint for the stable trees
Cc: linux-stable <stable@xxxxxxxxxxxxxxx> # >= v4.1
The patch is ok for me.
Do you need another Acked-by or Tested-by tag?
Tested-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
Just for the record:
With this patch for vcan.c
diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
index 39ca14b0585d..eb7023043385 100644
--- a/drivers/net/can/vcan.c
+++ b/drivers/net/can/vcan.c
@@ -83,15 +83,32 @@ static void vcan_rx(struct sk_buff *skb, struct
net_device *dev)
netif_rx_ni(skb);
}
+static void vcan_print_skb(struct sk_buff *skb)
+{
+ printk("len: %d, end-data: %ld, tail-data %ld, data-head %ld,
ifindex = %d, skbcnt = %d, pkt_type = %d, ip_summed = %d\n",
+ skb->len,
+ skb_end_pointer(skb) - skb->data,
+ skb_tail_pointer(skb) - skb->data,
+ skb->data - skb->head,
+ can_skb_prv(skb)->ifindex,
+ can_skb_prv(skb)->skbcnt,
+ skb->pkt_type,
+ skb->ip_summed);
+}
+
static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev)
{
struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
struct net_device_stats *stats = &dev->stats;
int loop;
+ vcan_print_skb(skb);
+
if (can_dropped_invalid_skb(dev, skb))
return NETDEV_TX_OK;
+ vcan_print_skb(skb);
+
stats->tx_packets++;
stats->tx_bytes += cfd->len;
And then starting
cansend vcan0 12345678#112233
dmesg:
[ 95.567858] len: 16, end-data: 184, tail-data 16, data-head 8,
ifindex = 8, skbcnt = 0, pkt_type = 5, ip_summed = 1
[ 95.567862] len: 16, end-data: 184, tail-data 16, data-head 8,
ifindex = 8, skbcnt = 0, pkt_type = 5, ip_summed = 1
Alternatively
linux-can/can-tests/netlayer# ./tst-packet -s
dmesg:
[ 124.740592] len: 16, end-data: 176, tail-data 16, data-head 16,
ifindex = 0, skbcnt = 0, pkt_type = 0, ip_summed = 0
[ 124.740596] len: 16, end-data: 176, tail-data 16, data-head 16,
ifindex = 10, skbcnt = 0, pkt_type = 5, ip_summed = 1
So ifindex, skbcnt, pkt_type, ip_summed have been set correctly - even
for skb's that have not been created by the CAN subsystem (e.g. packet
socket).
Regards,
Oliver
