Re: [PATCH] can: dev: can_put_echo_skb(): free skb on invalid echo index

[Marc Kleine-Budde]

On 30.07.2026 23:03:12, Cunhao Lu wrote:
> can_put_echo_skb() consumes the skb on all paths except when the echo index
> is out of bounds. This leaves ownership with the caller on -EINVAL, unlike
> the other error paths, and can leak the skb if the caller expects
> consistent semantics.
>
> Free the skb before returning -EINVAL so that all return paths consume it.
>
> Fixes: 6411959c10fe ("can: dev: can_put_echo_skb(): don't crash kernel if can_priv::echo_skb is accessed out of bounds")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Cunhao Lu <1579567540@xxxxxx>
> ---
>  drivers/net/can/dev/skb.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c
> index 95fcdc1026f8..44ebeba99837 100644
> --- a/drivers/net/can/dev/skb.c
> +++ b/drivers/net/can/dev/skb.c
> @@ -54,6 +54,7 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
>  	if (idx >= priv->echo_skb_max) {
>  		netdev_err(dev, "%s: BUG! Trying to access can_priv::echo_skb out of bounds (%u/max %u)\n",
>  			   __func__, idx, priv->echo_skb_max);
> +		kfree_skb(skb);

I think sashiko's comment is valid. Can you use a dev_kfree_skb_any()
here. Can you add another patch to this series and convert the existing
kfree_skb() to dev_kfree_skb_any()? The new patch should be the first
one.

regards,
Marc

>  		return -EINVAL;
>  	}
>
>
> ---
> base-commit: 11028ab62899e4191e074ee364c712b77823a9c4
> change-id: 20260730-master-abf3e4067e47
>
> Best regards,
> --
> Cunhao Lu <1579567540@xxxxxx>
>
>
>

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Attachment: signature.asc
Description: PGP signature