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()

Posted by Baul Lee 1 month, 1 week ago

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()

Posted by Marc Kleine-Budde 1 month, 1 week ago

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   |

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

Posted by Max Staudt 1 month, 1 week ago

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>