[PATCH net] can: can327: Fix stack out-of-bounds write in can327_handle_prompt()

[PATCH net] can: can327: Fix stack out-of-bounds write in can327_handle_prompt()

[Baul Lee]

can327_handle_prompt() hexdumps the frame queued for transmission, two
hex characters per payload byte, into an 18-byte on-stack buffer sized
for a classical 8-byte payload. The loop runs frame->len times without a
clamp, and frame is a struct can_frame copied verbatim from the skb
passed to can327_netdev_start_xmit().

A CAN XL frame aliases struct can_frame such that canxl_frame.flags,
which must carry CANXL_XLF, overlaps can_frame.len, so it arrives with
frame->len == 0x80. can327 is a classical CAN device, but
can_dev_dropped_skb() only rejects CAN FD for a non-FD device; for
ETH_P_CANXL, can_dropped_invalid_skb() checks only that the frame is a
well-formed CAN XL frame, not that the device supports one. The smallest
such frame, CANXL_HDR_SIZE + CANXL_MIN_DLEN, is 13 bytes, so it also
passes the MTU check on a CAN_MTU device. CAN_RAW rejects it, as
raw_check_txframe() requires CAN_CAP_XL, but an AF_PACKET SOCK_RAW
socket does not go through that check.

The loop then writes 128 hex pairs over the buffer and past the end of
the stack frame:

  BUG: KASAN: stack-out-of-bounds in vsnprintf+0x550/0x83c
  Write of size 1 at addr ffff800085407a32 by task kworker/u18:0/36
  Call trace:
   vsnprintf+0x550/0x83c
   snprintf+0xa4/0xe0
   can327_handle_prompt+0x5c4/0x69c
   can327_parse_rxbuf+0x618/0xb50
   can327_ldisc_rx+0x118/0x1c0
   tty_ldisc_receive_buf+0xc0/0xe0
   flush_to_ldisc+0x11c/0x278
  This frame has 1 object:
   [32, 50) 'local_txbuf'
  Kernel panic - not syncing: stack-protector: Kernel stack is corrupted
  in: can327_handle_prompt+0x66c/0x69c

Clamp the dump to CAN_MAX_DLEN, which is what the buffer is sized for.

Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>

Fixes: fb08cba12b52 ("can: canxl: update CAN infrastructure for CAN XL frames")
Signed-off-by: Baul Lee <baul.lee@xbow.com>
---
 drivers/net/can/can327.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/can327.c b/drivers/net/can/can327.c
index 90f5e35f3c8f..396db56775a6 100644
--- a/drivers/net/can/can327.c
+++ b/drivers/net/can/can327.c
@@ -624,9 +624,10 @@ static void can327_handle_prompt(struct can327 *elm)
 			snprintf(local_txbuf, sizeof(local_txbuf), "ATRTR\r");
 		} else {
 			/* Send a regular CAN data frame */
+			int dlc = min_t(int, frame->len, CAN_MAX_DLEN);
 			int i;
 
-			for (i = 0; i < frame->len; i++) {
+			for (i = 0; i < dlc; i++) {
 				snprintf(&local_txbuf[2 * i],
 					 sizeof(local_txbuf), "%02X",
 					 frame->data[i]);
-- 
2.50.1

Re: [PATCH net] can: can327: Fix stack out-of-bounds write in can327_handle_prompt()

[sashiko-bot]

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

New issues:
- [Medium] The patch prevents a stack crash but results in transmitting an invalid CAN XL frame as garbage classical CAN data.

Pre-existing issues:
- [High] The patch addresses a local stack out-of-bounds write in can327 but fails to fix the subsystem-wide root cause: can_dev_dropped_skb() does not reject CAN XL frames for non-XL devices.
- [High] Sending a short (13-byte) CAN XL frame via an AF_PACKET RAW socket leaks 3 bytes of uninitialized kernel heap memory over UART.
- [High] Missing bounds check on parsed frame->len causes a heap array out-of-bounds write in can327_parse_frame().
--

--- Patch [1]: [PATCH net] can: can327: Fix stack out-of-bounds write in can327_handle_prompt() ---
commit 07e6406926056d156de760bd7d0a03a7acd9ad40
Author: Baul Lee <baul.lee@xbow.com>

can: can327: Fix stack out-of-bounds write in can327_handle_prompt()

This commit addresses a stack out-of-bounds write in can327_handle_prompt()
caused by an aliased CAN XL frame length. It clamps the hexdump loop to
CAN_MAX_DLEN to prevent writing past the 18-byte local_txbuf.

>    A CAN XL frame aliases struct can_frame such that canxl_frame.flags,
>    which must carry CANXL_XLF, overlaps can_frame.len, so it arrives with
>    frame->len == 0x80. can327 is a classical CAN device, but
>    can_dev_dropped_skb() only rejects CAN FD for a non-FD device; for
>    ETH_P_CANXL, can_dropped_invalid_skb() checks only that the frame is a
>    well-formed CAN XL frame, not that the device supports one.

[Severity: High]
This isn't a bug introduced by this patch, but does can_dev_dropped_skb() in
include/linux/can/dev.h need to drop CAN XL frames for non-XL hardware to
prevent a subsystem-wide issue?

Because canxl_frame.flags aliases with can_frame.len (resulting in a length
of 128), classical CAN drivers misinterpret the length. For example, does
this allow slcan_encaps() in drivers/net/can/slcan/slcan-core.c to hex-dump
128 bytes into a 33-byte heap buffer, or sja1000_start_xmit() in
drivers/net/can/sja1000/sja1000.c to write 128 out-of-bounds bytes to
hardware registers?

> diff --git a/drivers/net/can/can327.c b/drivers/net/can/can327.c
> index 90f5e35f3c8f9..396db56775a64 100644
> --- a/drivers/net/can/can327.c
> +++ b/drivers/net/can/can327.c
> @@ -624,9 +624,10 @@ static void can327_handle_prompt(struct can327 *elm)
>  			snprintf(local_txbuf, sizeof(local_txbuf), "ATRTR\r");
>  		} else {
>  			/* Send a regular CAN data frame */
> +			int dlc = min_t(int, frame->len, CAN_MAX_DLEN);

[Severity: Medium]
Instead of artificially clamping the length to 8, should we drop the
unsupported CAN XL frame entirely? 

By clamping it, it looks like we might transmit internal CAN XL headers as
classical CAN payload bytes. Does this cause a CAN XL frame to be
transmitted as garbage classical CAN data over the bus?

>  			int i;
>  
> -			for (i = 0; i < frame->len; i++) {
> +			for (i = 0; i < dlc; i++) {
>  				snprintf(&local_txbuf[2 * i],
>  					 sizeof(local_txbuf), "%02X",
>  					 frame->data[i]);

[Severity: High]
This is a pre-existing issue, but does sending a short (13-byte) CAN XL
frame via an AF_PACKET RAW socket leak uninitialized kernel heap memory
over UART?

In can327_netdev_start_xmit(), the driver passes a 13-byte SKB to
can327_send_frame(), which performs a 16-byte by-value struct copy:

can327_send_frame()
   elm->can_frame_to_send = *frame;

This captures 3 bytes of uninitialized heap memory. Then here in
can327_handle_prompt(), the loop formats offsets 8 through 15 of the
struct and queues them to be sent over the serial line.

[ ... ]

[Severity: High]
This is another pre-existing issue, but does can327_parse_frame() lack
bounds checking on the parsed frame->len?

If a malicious or faulty ELM327 device sends a malformed CAN data line
with a parsed length up to 15, frame->len is set directly:

drivers/net/can/can327.c:can327_parse_frame() {
    ...
    /* Read CAN data length */
    frame->len = (hex_to_bin(elm->rxbuf[datastart - 2]) << 0);
    ...
    /* Parse the data nibbles. */
    for (i = 0; i < frame->len; i++) {
        frame->data[i] =
            (hex_to_bin(elm->rxbuf[datastart + 3 * i]) << 4) |
            (hex_to_bin(elm->rxbuf[datastart + 3 * i + 1]));
    }
    ...
}

Since the skb is allocated via alloc_can_skb() (which allocates exactly
16 bytes for a classical CAN frame), does writing 15 bytes starting at
offset 8 corrupt adjacent heap memory like skb_shared_info?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819054201.67294-1-baul.lee@xbow.com?part=1

Re: [PATCH net] can: can327: Fix stack out-of-bounds write in can327_handle_prompt()

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On 19.08.2026 14:42:01, Baul Lee wrote:
> can327_handle_prompt() hexdumps the frame queued for transmission, two
> hex characters per payload byte, into an 18-byte on-stack buffer sized
> for a classical 8-byte payload. The loop runs frame->len times without a
> clamp, and frame is a struct can_frame copied verbatim from the skb
> passed to can327_netdev_start_xmit().
>
> A CAN XL frame aliases struct can_frame such that canxl_frame.flags,

I've a patch pending, that drops CAN_XL frames on non CAN_XL interfaces:

https://lore.kernel.org/all/20260731-drop_canxl_frames-v1-1-7387b70353b3@kernel.org/

So I think this patch is not needed.

regards,
Marc

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net] can: can327: Fix stack out-of-bounds write in can327_handle_prompt()

[Max Staudt]

On 8/19/26 3:23 PM, Marc Kleine-Budde wrote:
> On 19.08.2026 14:42:01, Baul Lee wrote:
>> can327_handle_prompt() hexdumps the frame queued for transmission, two
>> hex characters per payload byte, into an 18-byte on-stack buffer sized
>> for a classical 8-byte payload. The loop runs frame->len times without a
>> clamp, and frame is a struct can_frame copied verbatim from the skb
>> passed to can327_netdev_start_xmit().
>>
>> A CAN XL frame aliases struct can_frame such that canxl_frame.flags,
> 
> I've a patch pending, that drops CAN_XL frames on non CAN_XL interfaces:
> 
> https://lore.kernel.org/all/20260731-drop_canxl_frames-v1-1-7387b70353b3@kernel.org/
> 
> So I think this patch is not needed.

Agreed, thanks!

Just in case you decide to take Baul's patch anyway, here is my

Reviewed-by: Max Staudt <max@enpas.org>