Re: [PATCH v5 1/3] can: rockchip_canfd: prevent TX stall on echo skb failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds read and hardware register out-of-bounds write due to CAN XL frame mishandling.
--

--- Patch [1]: [PATCH v5 1/3] can: rockchip_canfd: prevent TX stall on echo skb failure ---
commit 8df8ef3a7e1e7414d8117605f2f14ed295ef9ad1
Author: Cunhao Lu <1579567540@qq.com>

can: rockchip_canfd: prevent TX stall on echo skb failure

This patch prevents a TX stall by installing the echo skb before loading
the hardware TX buffer. If installation fails, it drops the frame and leaves
the hardware FIFO and software TX state unchanged. It correctly relies on the
echo skb as the source for hardware frame data after it is installed.

Link: https://lore.kernel.org/linux-can/tencent_944DADCC4B42C8484EC01DA2B15F42132906@qq.com

> diff --git a/drivers/net/can/rockchip/rockchip_canfd-tx.c b/drivers/net/can/rockchip/rockchip_canfd-tx.c
> index 12200dcfd3389..86fa8f2e1c8ba 100644
> --- a/drivers/net/can/rockchip/rockchip_canfd-tx.c
> +++ b/drivers/net/can/rockchip/rockchip_canfd-tx.c
[ ... ]
> @@ -114,7 +123,6 @@ netdev_tx_t rkcanfd_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  					    cfd->len);
>  	}
>  
> -	tx_head = rkcanfd_get_tx_head(priv);
>  	reg_cmd = RKCANFD_REG_CMD_TX_REQ(tx_head);
>  
>  	rkcanfd_write(priv, RKCANFD_REG_FD_TXFRAMEINFO, reg_frameinfo);
> @@ -123,10 +131,7 @@ netdev_tx_t rkcanfd_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  		rkcanfd_write(priv, RKCANFD_REG_FD_TXDATA0 + i,
>  			      *(u32 *)(cfd->data + i));
>  

[Severity: High]
This is a pre-existing issue, but does this code properly handle CAN XL frames?

If an ETH_P_CANXL frame is sent via AF_PACKET, can_dev_dropped_skb() currently
lacks a check to drop CAN XL frames for devices that don't support CAN XL.
When such a frame enters rkcanfd_start_xmit(), can_is_canfd_skb() returns
false, causing the driver to treat it as a Classic CAN frame.

The driver reads cfd->len from offset 4, which in a canxl_frame corresponds to
the user-controlled flags field. Because valid CAN XL frames require CANXL_XLF
(0x80) to be set, cfd->len is evaluated as at least 128.

Could a minimally sized CAN XL frame trigger a loop of 32 iterations here,
leading to an out-of-bounds read from cfd->data and an out-of-bounds write
to device memory at RKCANFD_REG_FD_TXDATA0 + i?

> -	frame_len = can_skb_get_frame_len(skb);
> -	err = can_put_echo_skb(skb, ndev, tx_head, frame_len);
> -	if (!err)
> -		netdev_sent_queue(priv->ndev, frame_len);
> +	netdev_sent_queue(priv->ndev, frame_len);
>  
>  	WRITE_ONCE(priv->tx_head, priv->tx_head + 1);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-master-v5-0-5b27029dee20@qq.com?part=1