On Wed. 20 Jan 2021 at 01:25, Vincent Mailhol
<mailhol.vincent@xxxxxxxxxx> wrote:
>
> 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 <mailhol.vincent@xxxxxxxxxx>
> ---
> 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
>
