On 19.01.21 18:07, Vincent MAILHOL wrote:

On Wed. 20 Jan 2021 at 01:25, Vincent Mailhol
[off-list ref] wrote:

quoted

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...

not tested -> compile tested -> runtime tested

Choose your level!

:-D

quoted

         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