Re: [PATCH 2/3] can: vxcan: vxcan_xmit: fix use after free bug
From: Vincent MAILHOL <hidden>
Date: 2021-01-19 18:29:25
On Wed. 20 Jan 2021 at 01:25, Vincent Mailhol [off-list ref] wrote:
quoted hunk ↗ jump to hunk
After calling netif_rx_ni(skb), dereferencing skb is unsafe. Especially, the canfd_frame cfd which aliases skb memory is accessed after the netif_rx_ni(). fixes: a8f820a380a2 ("can: add Virtual CAN Tunnel driver (vxcan)") Signed-off-by: Vincent Mailhol <redacted> --- drivers/net/can/vxcan.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)diff --git a/drivers/net/can/vxcan.c b/drivers/net/can/vxcan.c index fa47bab510bb..a525ef8d19b0 100644 --- a/drivers/net/can/vxcan.c +++ b/drivers/net/can/vxcan.c@@ -39,6 +39,7 @@ static netdev_tx_t vxcan_xmit(struct sk_buff *skb, struct net_device *dev) struct net_device *peer; struct canfd_frame *cfd = (struct canfd_frame *)skb->data; struct net_device_stats *peerstats, *srcstats = &dev->stats; + u8 len; if (can_dropped_invalid_skb(dev, skb)) return NETDEV_TX_OK;@@ -61,12 +62,13 @@ static netdev_tx_t vxcan_xmit(struct sk_buff *skb, struct net_device *dev) skb->dev = peer; skb->ip_summed = CHECKSUM_UNNECESSARY; + u8 len = cfd->len;
len = cfd->len; Silly mistake: u8 not needed twice of course...
if (netif_rx_ni(skb) == NET_RX_SUCCESS) {
srcstats->tx_packets++;
- srcstats->tx_bytes += cfd->len;
+ srcstats->tx_bytes += len;
peerstats = &peer->stats;
peerstats->rx_packets++;
- peerstats->rx_bytes += cfd->len;
+ peerstats->rx_bytes += len;
}
out_unlock:
--
2.26.2