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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Modifying SKB payload in can_skb_init_valid() without unsharing causes data corruption for concurrent observers.
--

commit 6a48fe2d96b6f1ca706da34b2f7bdd872d06941f
Author: Cunhao Lu <1579567540@qq.com>
can: dev: can_put_echo_skb(): free skb on invalid echo index

This patch fixes a potential leak in can_put_echo_skb() when the echo index
is out of bounds. It ensures the socket buffer is freed before returning
-EINVAL so that all error paths correctly consume the SKB.

Link: https://patch.msgid.link/tencent_683AA16E643DE00211CD2FB62991264DC605@qq.com

> diff --git a/drivers/net/can/dev/skb.c b/drivers/net/can/dev/skb.c
> index c7be21ed70ddf..faafb1033d423 100644
> --- a/drivers/net/can/dev/skb.c
> +++ b/drivers/net/can/dev/skb.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but already existed in the
codebase. Does can_skb_init_valid() in this same file modify shared memory
without unsharing it first?

drivers/net/can/dev/skb.c:can_skb_init_valid() {
        ...
        /* set CANFD_FDF flag for CAN FD frames */
        if (can_is_canfd_skb(skb)) {
            struct canfd_frame *cfd;

            cfd = (struct canfd_frame *)skb->data;
            cfd->flags |= CANFD_FDF;
        }
        ...
}

If a user with CAP_NET_RAW injects a CAN FD frame via an AF_PACKET socket,
the packet goes to dev_queue_xmit() where it may be cloned for packet
sniffers like tcpdump.

When the original SKB reaches the driver's transmit function and calls
can_dev_dropped_skb() -> can_dropped_invalid_skb() -> can_skb_init_valid(),
it modifies the payload (cfd->flags |= CANFD_FDF) without verifying
skb_cloned(skb) or calling skb_unshare().

Can this cause data corruption for concurrent observers receiving the clones?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826121036.2706424-1-mkl@pengutronix.de?part=10