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

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

[Baul Lee]

can327_parse_frame() assigns the CAN payload length from the DLC nibble
of the adapter's ASCII frame line, hex_to_bin(elm->rxbuf[datastart - 2]),
without validating it. A standard-format line only has to satisfy
rxbuf[3] == ' ' and rxbuf[5] == ' ', so the DLC nibble rxbuf[4] can be a
space, for which hex_to_bin() returns -1, and that becomes 255 in the u8
frame->len. A hex nibble of 9 to f is not rejected either, while
CAN_MAX_DLEN is 8.

frame->data[] is the 8-byte payload of the 16-byte struct can_frame
returned by alloc_can_skb(), so the data-nibble loop writes up to 255
device-controlled bytes, 247 of them past the frame and over the
trailing skb_shared_info. The length check before the loop only requires
the line to be frame->len * 3 + datastart bytes, which a long enough
line of hex and spaces satisfies. Freeing the corrupted skb then faults:

  pc : skb_release_data+0xf4/0x200
  Call trace:
   skb_release_data+0xf4/0x200 (P)
   sk_skb_reason_drop+0x40/0xa4
   can_rcv+0x6c/0xbc
   __netif_receive_skb_one_core+0x40/0x4c
   can327_ldisc_rx+0xc8/0x140
   tty_ldisc_receive_buf+0x48/0x60
   flush_to_ldisc+0xdc/0x1b0
  Kernel panic - not syncing: Oops: Fatal exception in interrupt

Reject the line when the nibble is not a hex digit or exceeds
CAN_MAX_DLEN, as the parser already does for other malformed lines.

Attaching the N_CAN327 line discipline requires CAP_NET_ADMIN, but the
frame lines then come from the ELM327 device, so a malicious adapter
reaches this path with device-controlled data.

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

Fixes: 43da2f07622f ("can: can327: CAN/ldisc driver for ELM327 based OBD-II adapters")
Cc: stable@vger.kernel.org
Signed-off-by: Baul Lee <baul.lee@xbow.com>
Reviewed-by: Max Staudt <max@enpas.org>
---
v2: add Cc: stable and Max's Reviewed-by, as requested in review of v1
    (https://lore.kernel.org/linux-can/20260818215029.47758-1-baul.lee@xbow.com/).
    No change to the code.

 drivers/net/can/can327.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/can327.c b/drivers/net/can/can327.c
index 90f5e35f3c8f..c76a6378d4d6 100644
--- a/drivers/net/can/can327.c
+++ b/drivers/net/can/can327.c
@@ -395,6 +395,7 @@ static int can327_parse_frame(struct can327 *elm, size_t len)
 	struct sk_buff *skb;
 	int hexlen;
 	int datastart;
+	int dlc;
 	int i;
 
 	lockdep_assert_held(&elm->lock);
@@ -460,7 +461,13 @@ static int can327_parse_frame(struct can327 *elm, size_t len)
 	 */
 
 	/* Read CAN data length */
-	frame->len = (hex_to_bin(elm->rxbuf[datastart - 2]) << 0);
+	dlc = hex_to_bin(elm->rxbuf[datastart - 2]);
+	if (dlc < 0 || dlc > CAN_MAX_DLEN) {
+		/* Not a hex digit, or more than CAN_MAX_DLEN bytes. */
+		kfree_skb(skb);
+		return -ENODATA;
+	}
+	frame->len = dlc;
 
 	/* Read CAN ID */
 	if (frame->can_id & CAN_EFF_FLAG) {
-- 
2.50.1

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

[Max Staudt]

It seems like your patch sending tool didn't pick up the Cc: line in the 
commit message :(

Can you please re-send it just like v2, but also ensure that 
stable@vger.kernel.org is on the email's CC list?


Max

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Injecting a CAN XL frame causes a stack buffer overflow in `can327_handle_prompt()` due to an unvalidated `frame->len`.
- [High] Valid Classical CAN frames with a Data Length Code (DLC) greater than 8 are improperly rejected, leading to a state machine reset loop (DoS).
--

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

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

This patch fixes an out-of-bounds write in can327_parse_frame() by
validating the CAN payload length from the adapter. It adds checks to
reject frames if the DLC is not a valid hex digit or exceeds CAN_MAX_DLEN.

Link: https://lore.kernel.org/linux-can/20260818215029.47758-1-baul.lee@xbow.com/

> diff --git a/drivers/net/can/can327.c b/drivers/net/can/can327.c
> index 90f5e35f3c8f9..c76a6378d4d62 100644
> --- a/drivers/net/can/can327.c
> +++ b/drivers/net/can/can327.c

[ ... ]

> @@ -460,7 +461,13 @@ static int can327_parse_frame(struct can327 *elm, size_t len)
>  	 */
>  
>  	/* Read CAN data length */
> -	frame->len = (hex_to_bin(elm->rxbuf[datastart - 2]) << 0);
> +	dlc = hex_to_bin(elm->rxbuf[datastart - 2]);
> +	if (dlc < 0 || dlc > CAN_MAX_DLEN) {
> +		/* Not a hex digit, or more than CAN_MAX_DLEN bytes. */
> +		kfree_skb(skb);
> +		return -ENODATA;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but does returning -ENODATA for
frames with a DLC greater than 8 lead to a state machine reset loop?

Classical CAN frames can have a DLC between 9 and 15 on the bus. The ELM327
adapter outputs the raw DLC nibble. If this check fails and returns -ENODATA,
can327_parse_line() treats it as an unrecoverable error and calls
can327_kick_into_cmd_mode() to reset the adapter:

drivers/net/can/can327.c:can327_parse_line() {
    ...
    if (ret == -ENODATA) {
        /* Start afresh. */
        can327_kick_into_cmd_mode(elm);
    ...
}

Will this cause a denial of service by continuously resetting the CAN
interface whenever a valid frame with DLC > 8 is received?

> +	frame->len = dlc;
>  
>  	/* Read CAN ID */
>  	if (frame->can_id & CAN_EFF_FLAG) {

[Severity: Critical]
This is a pre-existing issue, but is there a stack buffer overflow in
can327_handle_prompt() when handling CAN XL frames?

If can_dev_dropped_skb() in include/linux/can/dev.h fails to drop ETH_P_CANXL
frames for devices lacking CAN XL support, an attacker could send a CAN XL
frame via a PF_PACKET or PF_CAN raw socket.

When can327_netdev_start_xmit() copies the first 16 bytes into
elm->can_frame_to_send, the CAN XL flags field aliases with frame->len,
setting it to 128 or more (since CANXL_XLF is 0x80).

In can327_handle_prompt(), the loop iterates over this attacker-controlled
length:

drivers/net/can/can327.c:can327_handle_prompt() {
    ...
    for (i = 0; i < frame->len; i++) {
        snprintf(&local_txbuf[2 * i],
                 sizeof(local_txbuf), "%02X",
                 frame->data[i]);
    }
    ...
}

Since sizeof(local_txbuf) is only 18, will this loop overflow local_txbuf[]
by writing hex bytes far past the end of the stack buffer, overwriting the
stack frame and return address?

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

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

[Marc Kleine-Budde]

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

On 19.08.2026 07:18:18, Baul Lee wrote:
> can327_parse_frame() assigns the CAN payload length from the DLC nibble
> of the adapter's ASCII frame line, hex_to_bin(elm->rxbuf[datastart - 2]),
> without validating it. A standard-format line only has to satisfy
> rxbuf[3] == ' ' and rxbuf[5] == ' ', so the DLC nibble rxbuf[4] can be a
> space, for which hex_to_bin() returns -1, and that becomes 255 in the u8
> frame->len. A hex nibble of 9 to f is not rejected either, while
> CAN_MAX_DLEN is 8.
>
> frame->data[] is the 8-byte payload of the 16-byte struct can_frame
> returned by alloc_can_skb(), so the data-nibble loop writes up to 255
> device-controlled bytes, 247 of them past the frame and over the
> trailing skb_shared_info. The length check before the loop only requires
> the line to be frame->len * 3 + datastart bytes, which a long enough
> line of hex and spaces satisfies. Freeing the corrupted skb then faults:
>
>   pc : skb_release_data+0xf4/0x200
>   Call trace:
>    skb_release_data+0xf4/0x200 (P)
>    sk_skb_reason_drop+0x40/0xa4
>    can_rcv+0x6c/0xbc
>    __netif_receive_skb_one_core+0x40/0x4c
>    can327_ldisc_rx+0xc8/0x140
>    tty_ldisc_receive_buf+0x48/0x60
>    flush_to_ldisc+0xdc/0x1b0
>   Kernel panic - not syncing: Oops: Fatal exception in interrupt
>
> Reject the line when the nibble is not a hex digit or exceeds
> CAN_MAX_DLEN, as the parser already does for other malformed lines.
>
> Attaching the N_CAN327 line discipline requires CAP_NET_ADMIN, but the
> frame lines then come from the ELM327 device, so a malicious adapter
> reaches this path with device-controlled data.
>
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
>
> Fixes: 43da2f07622f ("can: can327: CAN/ldisc driver for ELM327 based OBD-II adapters")
> Cc: stable@vger.kernel.org
> Signed-off-by: Baul Lee <baul.lee@xbow.com>
> Reviewed-by: Max Staudt <max@enpas.org>

Nitpick, your S-o-b must come last.

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 v2] can: can327: Fix out-of-bounds write in can327_parse_frame()

[Marc Kleine-Budde]

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

On 19.08.2026 07:28:44, Max Staudt wrote:
> It seems like your patch sending tool didn't pick up the Cc: line in the
> commit message :(
>
> Can you please re-send it just like v2, but also ensure that
> stable@vger.kernel.org is on the email's CC list?

After I pick up the patch and send a PR, my tooling will automatically
send it to stable (if on Cc).

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 --]