Re: [PATCH net 10/14] can: dev: can_put_echo_skb(): free skb on invalid echo index
From: sashiko-bot@kernel.org
Date: 2026-08-27 12:10:53
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 [off-list ref] 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
quoted hunk ↗ jump to hunk
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