[PATCH 1/2] can: gw: fix OOB heap access in cgw_csum_crc8_rel()

[PATCH 1/2] can: gw: fix OOB heap access in cgw_csum_crc8_rel()

[Oliver Hartkopp]

From: Ali Norouzi <ali.norouzi@keysight.com>

cgw_csum_crc8_rel() correctly computes bounds-safe indices via calc_idx():

    int from = calc_idx(crc8->from_idx, cf->len);
    int to   = calc_idx(crc8->to_idx,   cf->len);
    int res  = calc_idx(crc8->result_idx, cf->len);

    if (from < 0 || to < 0 || res < 0)
        return;

However, the loop and the result write then use the raw s8 fields directly
instead of the computed variables:

    for (i = crc8->from_idx; ...)        /* BUG: raw negative index */
    cf->data[crc8->result_idx] = ...;    /* BUG: raw negative index */

With from_idx = to_idx = result_idx = -64 on a 64-byte CAN FD frame,
calc_idx(-64, 64) = 0 so the guard passes, but the loop iterates with
i = -64, reading cf->data[-64], and the write goes to cf->data[-64].
This write might end up to 56 (7.0-rc) or 40 (<= 6.19) bytes before the
start of the canfd_frame on the heap.

The companion function cgw_csum_xor_rel() uses `from`/`to`/`res`
correctly throughout; fix cgw_csum_crc8_rel() to match.

Confirmed with KASAN on linux-7.0-rc2:
  BUG: KASAN: slab-out-of-bounds in cgw_csum_crc8_rel+0x515/0x5b0
  Read of size 1 at addr ffff8880076619c8 by task poc_cgw_oob/62

To configure the can-gw crc8 checksums CAP_NET_ADMIN is needed.

Fixes: 456a8a646b25 ("can: gw: add support for CAN FD frames")
Cc: stable@vger.kernel.org
Reported-by: Ali Norouzi <ali.norouzi@keysight.com>
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Ali Norouzi <ali.norouzi@keysight.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/gw.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/can/gw.c b/net/can/gw.c
index 8ee4d67a07d3..0ec99f68aa45 100644
--- a/net/can/gw.c
+++ b/net/can/gw.c
@@ -373,14 +373,14 @@ static void cgw_csum_crc8_rel(struct canfd_frame *cf,
 
 	if (from < 0 || to < 0 || res < 0)
 		return;
 
 	if (from <= to) {
-		for (i = crc8->from_idx; i <= crc8->to_idx; i++)
+		for (i = from; i <= to; i++)
 			crc = crc8->crctab[crc ^ cf->data[i]];
 	} else {
-		for (i = crc8->from_idx; i >= crc8->to_idx; i--)
+		for (i = from; i >= to; i--)
 			crc = crc8->crctab[crc ^ cf->data[i]];
 	}
 
 	switch (crc8->profile) {
 	case CGW_CRC8PRF_1U8:
@@ -395,11 +395,11 @@ static void cgw_csum_crc8_rel(struct canfd_frame *cf,
 		crc = crc8->crctab[crc ^ (cf->can_id & 0xFF) ^
 				   (cf->can_id >> 8 & 0xFF)];
 		break;
 	}
 
-	cf->data[crc8->result_idx] = crc ^ crc8->final_xor_val;
+	cf->data[res] = crc ^ crc8->final_xor_val;
 }
 
 static void cgw_csum_crc8_pos(struct canfd_frame *cf,
 			      struct cgw_csum_crc8 *crc8)
 {
-- 
2.51.0

[PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Oliver Hartkopp]

isotp_sendmsg() uses only cmpxchg() on so->tx.state to serialize access
to so->tx.buf. isotp_release() waits for ISOTP_IDLE via
wait_event_interruptible() and then calls kfree(so->tx.buf).

If a signal interrupts the wait_event_interruptible() inside close()
while tx.state is ISOTP_SENDING, the loop exits early and release
proceeds to force ISOTP_SHUTDOWN and continues to kfree(so->tx.buf)
while sendmsg may still be reading so->tx.buf for the final CAN frame
in isotp_fill_dataframe().

The so->tx.buf can be allocated once when the standard tx.buf length needs
to be extended. Move the kfree() of this potentially extended tx.buf to
sk_destruct time when either isotp_sendmsg() and isotp_release() are done.

Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
Cc: stable@vger.kernel.org
Reported-by: Ali Norouzi <ali.norouzi@keysight.com>
Co-developed-by: Ali Norouzi <ali.norouzi@keysight.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/isotp.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index da3b72e7afcc..2770f43f4951 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1246,16 +1246,10 @@ static int isotp_release(struct socket *sock)
 	hrtimer_cancel(&so->rxtimer);
 
 	so->ifindex = 0;
 	so->bound = 0;
 
-	if (so->rx.buf != so->rx.sbuf)
-		kfree(so->rx.buf);
-
-	if (so->tx.buf != so->tx.sbuf)
-		kfree(so->tx.buf);
-
 	sock_orphan(sk);
 	sock->sk = NULL;
 
 	release_sock(sk);
 	sock_prot_inuse_add(net, sk->sk_prot, -1);
@@ -1620,10 +1614,25 @@ static int isotp_notifier(struct notifier_block *nb, unsigned long msg,
 	isotp_busy_notifier = NULL;
 	spin_unlock(&isotp_notifier_lock);
 	return NOTIFY_DONE;
 }
 
+static void isotp_sock_destruct(struct sock *sk)
+{
+	struct isotp_sock *so = isotp_sk(sk);
+
+	/* do the standard CAN sock destruct work */
+	can_sock_destruct(sk);
+
+	/* free potential extended PDU buffers */
+	if (so->rx.buf != so->rx.sbuf)
+		kfree(so->rx.buf);
+
+	if (so->tx.buf != so->tx.sbuf)
+		kfree(so->tx.buf);
+}
+
 static int isotp_init(struct sock *sk)
 {
 	struct isotp_sock *so = isotp_sk(sk);
 
 	so->ifindex = 0;
@@ -1664,10 +1673,13 @@ static int isotp_init(struct sock *sk)
 
 	spin_lock(&isotp_notifier_lock);
 	list_add_tail(&so->notifier, &isotp_notifier_list);
 	spin_unlock(&isotp_notifier_lock);
 
+	/* re-assign default can_sock_destruct() reference */
+	sk->sk_destruct = isotp_sock_destruct;
+
 	return 0;
 }
 
 static __poll_t isotp_poll(struct file *file, struct socket *sock, poll_table *wait)
 {
-- 
2.51.0

Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Oliver Hartkopp]

Hallo Marc,

the AI bot correctly remarked that the Fixes tag points to the wrong 
commit I took from Alis original patch.

Indeed it has to be

Fixes: 96d1c81e6a04 ("can: isotp: add module parameter for maximum pdu 
size")

Can you correct that while applying the patch or should I resend it?

Best regards,
Oliver


On 18.03.26 17:51, Oliver Hartkopp wrote:
> isotp_sendmsg() uses only cmpxchg() on so->tx.state to serialize access
> to so->tx.buf. isotp_release() waits for ISOTP_IDLE via
> wait_event_interruptible() and then calls kfree(so->tx.buf).
> 
> If a signal interrupts the wait_event_interruptible() inside close()
> while tx.state is ISOTP_SENDING, the loop exits early and release
> proceeds to force ISOTP_SHUTDOWN and continues to kfree(so->tx.buf)
> while sendmsg may still be reading so->tx.buf for the final CAN frame
> in isotp_fill_dataframe().
> 
> The so->tx.buf can be allocated once when the standard tx.buf length needs
> to be extended. Move the kfree() of this potentially extended tx.buf to
> sk_destruct time when either isotp_sendmsg() and isotp_release() are done.
> 
> Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
> Cc: stable@vger.kernel.org
> Reported-by: Ali Norouzi <ali.norouzi@keysight.com>
> Co-developed-by: Ali Norouzi <ali.norouzi@keysight.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
>   net/can/isotp.c | 24 ++++++++++++++++++------
>   1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index da3b72e7afcc..2770f43f4951 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -1246,16 +1246,10 @@ static int isotp_release(struct socket *sock)
>   	hrtimer_cancel(&so->rxtimer);
>   
>   	so->ifindex = 0;
>   	so->bound = 0;
>   
> -	if (so->rx.buf != so->rx.sbuf)
> -		kfree(so->rx.buf);
> -
> -	if (so->tx.buf != so->tx.sbuf)
> -		kfree(so->tx.buf);
> -
>   	sock_orphan(sk);
>   	sock->sk = NULL;
>   
>   	release_sock(sk);
>   	sock_prot_inuse_add(net, sk->sk_prot, -1);
> @@ -1620,10 +1614,25 @@ static int isotp_notifier(struct notifier_block *nb, unsigned long msg,
>   	isotp_busy_notifier = NULL;
>   	spin_unlock(&isotp_notifier_lock);
>   	return NOTIFY_DONE;
>   }
>   
> +static void isotp_sock_destruct(struct sock *sk)
> +{
> +	struct isotp_sock *so = isotp_sk(sk);
> +
> +	/* do the standard CAN sock destruct work */
> +	can_sock_destruct(sk);
> +
> +	/* free potential extended PDU buffers */
> +	if (so->rx.buf != so->rx.sbuf)
> +		kfree(so->rx.buf);
> +
> +	if (so->tx.buf != so->tx.sbuf)
> +		kfree(so->tx.buf);
> +}
> +
>   static int isotp_init(struct sock *sk)
>   {
>   	struct isotp_sock *so = isotp_sk(sk);
>   
>   	so->ifindex = 0;
> @@ -1664,10 +1673,13 @@ static int isotp_init(struct sock *sk)
>   
>   	spin_lock(&isotp_notifier_lock);
>   	list_add_tail(&so->notifier, &isotp_notifier_list);
>   	spin_unlock(&isotp_notifier_lock);
>   
> +	/* re-assign default can_sock_destruct() reference */
> +	sk->sk_destruct = isotp_sock_destruct;
> +
>   	return 0;
>   }
>   
>   static __poll_t isotp_poll(struct file *file, struct socket *sock, poll_table *wait)
>   {

Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Marc Kleine-Budde]

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

On 19.03.2026 14:13:55, Oliver Hartkopp wrote:
> Hallo Marc,
>
> the AI bot correctly remarked that the Fixes tag points to the wrong commit
> I took from Alis original patch.
>
> Indeed it has to be
>
> Fixes: 96d1c81e6a04 ("can: isotp: add module parameter for maximum pdu
> size")
>
> Can you correct that while applying the patch or should I resend it?

I'll do it. Please ignore my mail stating the same send 10 Minutes after
yours :)

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 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Oliver Hartkopp]

On 19.03.26 14:33, Marc Kleine-Budde wrote:
> On 19.03.2026 14:13:55, Oliver Hartkopp wrote:
>> Hallo Marc,
>>
>> the AI bot correctly remarked that the Fixes tag points to the wrong commit
>> I took from Alis original patch.
>>
>> Indeed it has to be
>>
>> Fixes: 96d1c81e6a04 ("can: isotp: add module parameter for maximum pdu
>> size")
>>
>> Can you correct that while applying the patch or should I resend it?
> 
> I'll do it. Please ignore my mail stating the same send 10 Minutes after
> yours :)
> 

Mail scatter today :-D

Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Marc Kleine-Budde]

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

On 18.03.2026 17:51:20, Oliver Hartkopp wrote:
> isotp_sendmsg() uses only cmpxchg() on so->tx.state to serialize access
> to so->tx.buf. isotp_release() waits for ISOTP_IDLE via
> wait_event_interruptible() and then calls kfree(so->tx.buf).
>
> If a signal interrupts the wait_event_interruptible() inside close()
> while tx.state is ISOTP_SENDING, the loop exits early and release
> proceeds to force ISOTP_SHUTDOWN and continues to kfree(so->tx.buf)
> while sendmsg may still be reading so->tx.buf for the final CAN frame
> in isotp_fill_dataframe().
>
> The so->tx.buf can be allocated once when the standard tx.buf length needs
> to be extended. Move the kfree() of this potentially extended tx.buf to
> sk_destruct time when either isotp_sendmsg() and isotp_release() are done.
>
> Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
> Cc: stable@vger.kernel.org
> Reported-by: Ali Norouzi <ali.norouzi@keysight.com>
> Co-developed-by: Ali Norouzi <ali.norouzi@keysight.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

AI-bot

| https://netdev-ai.bots.linux.dev/ai-review.html?id=19f0e15f-5d42-43a4-a9e1-632cf3d9182c

suggested:

Fixes: 96d1c81e6a04 ("can: isotp: add module parameter for maximum pdu size")

which makes sense.

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 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Marc Kleine-Budde]

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

On 18.03.2026 17:51:20, Oliver Hartkopp wrote:
> isotp_sendmsg() uses only cmpxchg() on so->tx.state to serialize access
> to so->tx.buf. isotp_release() waits for ISOTP_IDLE via
> wait_event_interruptible() and then calls kfree(so->tx.buf).
>
> If a signal interrupts the wait_event_interruptible() inside close()
> while tx.state is ISOTP_SENDING, the loop exits early and release
> proceeds to force ISOTP_SHUTDOWN and continues to kfree(so->tx.buf)
> while sendmsg may still be reading so->tx.buf for the final CAN frame
> in isotp_fill_dataframe().
>
> The so->tx.buf can be allocated once when the standard tx.buf length needs
> to be extended. Move the kfree() of this potentially extended tx.buf to
> sk_destruct time when either isotp_sendmsg() and isotp_release() are done.
>
> Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
> Cc: stable@vger.kernel.org
> Reported-by: Ali Norouzi <ali.norouzi@keysight.com>
> Co-developed-by: Ali Norouzi <ali.norouzi@keysight.com>

I'm missing Ali Norouzi's S-o-b. It was in the Mail that Linus Torvalds
forwarded us:

mid:CAHk-=wheQ2o0B_-EV5H3w=ZZS2huESOxrvTaub_EbrbAMbgi4A@mail.gmail.com

Ali can I add you S-o-b here?

> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

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 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Ali Norouzi]

Hi Marc,

Sure

Best,
Ali


________________________________________
From: Marc Kleine-Budde
Sent: Thursday, March 19, 2026 15:58
To: Oliver Hartkopp
Cc: linux-can@vger.kernel.org; stable@vger.kernel.org; Ali Norouzi
Subject: Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()


On 18.03.2026 17:51:20, Oliver Hartkopp wrote:

> isotp_sendmsg() uses only cmpxchg() on so->tx.state to serialize access

> to so->tx.buf. isotp_release() waits for ISOTP_IDLE via

> wait_event_interruptible() and then calls kfree(so->tx.buf).

>

> If a signal interrupts the wait_event_interruptible() inside close()

> while tx.state is ISOTP_SENDING, the loop exits early and release

> proceeds to force ISOTP_SHUTDOWN and continues to kfree(so->tx.buf)

> while sendmsg may still be reading so->tx.buf for the final CAN frame

> in isotp_fill_dataframe().

>

> The so->tx.buf can be allocated once when the standard tx.buf length needs

> to be extended. Move the kfree() of this potentially extended tx.buf to

> sk_destruct time when either isotp_sendmsg() and isotp_release() are done.

>

> Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")

> Cc: stable@vger.kernel.org

> Reported-by: Ali Norouzi <ali.norouzi@keysight.com>

> Co-developed-by: Ali Norouzi <ali.norouzi@keysight.com>



I'm missing Ali Norouzi's S-o-b. It was in the Mail that Linus Torvalds

forwarded us:



mid:CAHk-=wheQ2o0B_-EV5H3w=ZZS2huESOxrvTaub_EbrbAMbgi4A@mail.gmail.com



Ali can I add you S-o-b here?



> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>



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 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Marc Kleine-Budde]

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

On 19.03.2026 15:10:34, Ali Norouzi wrote:
> Hi Marc,
>
> Sure

Thanks,
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 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Oliver Hartkopp]

On 19.03.26 16:10, Ali Norouzi wrote:
> Hi Marc,
> 
> Sure

Btw. I'm fine with it too.

Thanks,
Ali

> 
> Best,
> Ali
> 
> 
> ________________________________________
> From: Marc Kleine-Budde
> Sent: Thursday, March 19, 2026 15:58
> To: Oliver Hartkopp
> Cc: linux-can@vger.kernel.org; stable@vger.kernel.org; Ali Norouzi
> Subject: Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
> 
> 
> On 18.03.2026 17:51:20, Oliver Hartkopp wrote:
> 
>> isotp_sendmsg() uses only cmpxchg() on so->tx.state to serialize access
> 
>> to so->tx.buf. isotp_release() waits for ISOTP_IDLE via
> 
>> wait_event_interruptible() and then calls kfree(so->tx.buf).
> 
>>
> 
>> If a signal interrupts the wait_event_interruptible() inside close()
> 
>> while tx.state is ISOTP_SENDING, the loop exits early and release
> 
>> proceeds to force ISOTP_SHUTDOWN and continues to kfree(so->tx.buf)
> 
>> while sendmsg may still be reading so->tx.buf for the final CAN frame
> 
>> in isotp_fill_dataframe().
> 
>>
> 
>> The so->tx.buf can be allocated once when the standard tx.buf length needs
> 
>> to be extended. Move the kfree() of this potentially extended tx.buf to
> 
>> sk_destruct time when either isotp_sendmsg() and isotp_release() are done.
> 
>>
> 
>> Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
> 
>> Cc: stable@vger.kernel.org
> 
>> Reported-by: Ali Norouzi <ali.norouzi@keysight.com>
> 
>> Co-developed-by: Ali Norouzi <ali.norouzi@keysight.com>
> 
> 
> 
> I'm missing Ali Norouzi's S-o-b. It was in the Mail that Linus Torvalds
> 
> forwarded us:
> 
> 
> 
> mid:CAHk-=wheQ2o0B_-EV5H3w=ZZS2huESOxrvTaub_EbrbAMbgi4A@mail.gmail.com
> 
> 
> 
> Ali can I add you S-o-b here?
> 
> 
> 
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> 
> 
> 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 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Marc Kleine-Budde]

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

On 19.03.2026 16:27:35, Oliver Hartkopp wrote:
> On 19.03.26 16:10, Ali Norouzi wrote:
> > Hi Marc,
> >
> > Sure
>
> Btw. I'm fine with it too.

Thanks Oliver, thanks Ali!

To keep confusion at a minimum, I've send a v2 of this series with Ali's
S-o-b and the corrected Fixes tag

| https://lore.kernel.org/20260319-fix-can-gw-and-can-isotp-v2-0-c45d52c6d2d8@pengutronix.de

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 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Oliver Hartkopp]

On 19.03.26 16:50, Marc Kleine-Budde wrote:
> On 19.03.2026 16:27:35, Oliver Hartkopp wrote:
>> On 19.03.26 16:10, Ali Norouzi wrote:
>>> Hi Marc,
>>>
>>> Sure
>>
>> Btw. I'm fine with it too.
> 
> Thanks Oliver, thanks Ali!
> 
> To keep confusion at a minimum, I've send a v2 of this series with Ali's
> S-o-b and the corrected Fixes tag
> 
> | https://lore.kernel.org/20260319-fix-can-gw-and-can-isotp-v2-0-c45d52c6d2d8@pengutronix.de
> 

Thanks Marc!

Do you expect some additional Acked-by or was this just an informative 
message?

I'm definitely fine with it. It can be up-streamed as-is.

Best regards,
Oliver

Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Marc Kleine-Budde]

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

On 23.03.2026 09:47:44, Oliver Hartkopp wrote:
> > Thanks Oliver, thanks Ali!
> >
> > To keep confusion at a minimum, I've send a v2 of this series with Ali's
> > S-o-b and the corrected Fixes tag
> >
> > | https://lore.kernel.org/20260319-fix-can-gw-and-can-isotp-v2-0-c45d52c6d2d8@pengutronix.de

> Do you expect some additional Acked-by or was this just an informative
> message?

I'm going to upstream this series, unless I get negative feedback.
Positive feedback is always welcome :)

> I'm definitely fine with it. It can be up-streamed as-is.

Thanks :)

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 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Oliver Hartkopp]

On 19.03.26 15:58, Marc Kleine-Budde wrote:
> On 18.03.2026 17:51:20, Oliver Hartkopp wrote:
>> isotp_sendmsg() uses only cmpxchg() on so->tx.state to serialize access
>> to so->tx.buf. isotp_release() waits for ISOTP_IDLE via
>> wait_event_interruptible() and then calls kfree(so->tx.buf).
>>
>> If a signal interrupts the wait_event_interruptible() inside close()
>> while tx.state is ISOTP_SENDING, the loop exits early and release
>> proceeds to force ISOTP_SHUTDOWN and continues to kfree(so->tx.buf)
>> while sendmsg may still be reading so->tx.buf for the final CAN frame
>> in isotp_fill_dataframe().
>>
>> The so->tx.buf can be allocated once when the standard tx.buf length needs
>> to be extended. Move the kfree() of this potentially extended tx.buf to
>> sk_destruct time when either isotp_sendmsg() and isotp_release() are done.
>>
>> Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
>> Cc: stable@vger.kernel.org
>> Reported-by: Ali Norouzi <ali.norouzi@keysight.com>
>> Co-developed-by: Ali Norouzi <ali.norouzi@keysight.com>
> 
> I'm missing Ali Norouzi's S-o-b. It was in the Mail that Linus Torvalds
> forwarded us:
> 
> mid:CAHk-=wheQ2o0B_-EV5H3w=ZZS2huESOxrvTaub_EbrbAMbgi4A@mail.gmail.com
> 
> Ali can I add you S-o-b here?

Is this correct?

I picked some of his commit message but the code is entirely my product.

Best regards,
Oliver

> 
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> regards,
> Marc
> 

Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Marc Kleine-Budde]

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

On 19.03.2026 16:12:21, Oliver Hartkopp wrote:
>
>
> On 19.03.26 15:58, Marc Kleine-Budde wrote:
> > On 18.03.2026 17:51:20, Oliver Hartkopp wrote:
> > > isotp_sendmsg() uses only cmpxchg() on so->tx.state to serialize access
> > > to so->tx.buf. isotp_release() waits for ISOTP_IDLE via
> > > wait_event_interruptible() and then calls kfree(so->tx.buf).
> > >
> > > If a signal interrupts the wait_event_interruptible() inside close()
> > > while tx.state is ISOTP_SENDING, the loop exits early and release
> > > proceeds to force ISOTP_SHUTDOWN and continues to kfree(so->tx.buf)
> > > while sendmsg may still be reading so->tx.buf for the final CAN frame
> > > in isotp_fill_dataframe().
> > >
> > > The so->tx.buf can be allocated once when the standard tx.buf length needs
> > > to be extended. Move the kfree() of this potentially extended tx.buf to
> > > sk_destruct time when either isotp_sendmsg() and isotp_release() are done.
> > >
> > > Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
> > > Cc: stable@vger.kernel.org
> > > Reported-by: Ali Norouzi <ali.norouzi@keysight.com>
> > > Co-developed-by: Ali Norouzi <ali.norouzi@keysight.com>
      ^^^^^^^^^^^^^^^
> >
> > I'm missing Ali Norouzi's S-o-b. It was in the Mail that Linus Torvalds
> > forwarded us:
> >
> > mid:CAHk-=wheQ2o0B_-EV5H3w=ZZS2huESOxrvTaub_EbrbAMbgi4A@mail.gmail.com
> >
> > Ali can I add you S-o-b here?
>
> Is this correct?
>
> I picked some of his commit message but the code is entirely my product.

You added "Co-developed-by: Ali Norouzi <ali.norouzi@keysight.com>" to
this patch. According to Documentation/process/submitting-patches.rst,
we need Ali's S-o-b then.

Please discuss authorship with Ali, consult submitting-patches.rst, use
"b4 prep --check", then send a v2 :)

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

[PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()

[Oliver Hartkopp]

isotp_sendmsg() uses only cmpxchg() on so->tx.state to serialize access
to so->tx.buf. isotp_release() waits for ISOTP_IDLE via
wait_event_interruptible() and then calls kfree(so->tx.buf).

If a signal interrupts the wait_event_interruptible() inside close()
while tx.state is ISOTP_SENDING, the loop exits early and release
proceeds to force ISOTP_SHUTDOWN and continues to kfree(so->tx.buf)
while sendmsg may still be reading so->tx.buf for the final CAN frame
in isotp_fill_dataframe().

The so->tx.buf can be allocated once when the standard tx.buf length needs
to be extended. Move the kfree() of this potentially extended tx.buf to
sk_destruct time when either isotp_sendmsg() and isotp_release() are done.

Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
Cc: stable@vger.kernel.org
Reported-by: Ali Norouzi <ali.norouzi@keysight.com>
Co-developed-by: Ali Norouzi <ali.norouzi@keysight.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/isotp.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index da3b72e7afcc..2770f43f4951 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1246,16 +1246,10 @@ static int isotp_release(struct socket *sock)
 	hrtimer_cancel(&so->rxtimer);
 
 	so->ifindex = 0;
 	so->bound = 0;
 
-	if (so->rx.buf != so->rx.sbuf)
-		kfree(so->rx.buf);
-
-	if (so->tx.buf != so->tx.sbuf)
-		kfree(so->tx.buf);
-
 	sock_orphan(sk);
 	sock->sk = NULL;
 
 	release_sock(sk);
 	sock_prot_inuse_add(net, sk->sk_prot, -1);
@@ -1620,10 +1614,25 @@ static int isotp_notifier(struct notifier_block *nb, unsigned long msg,
 	isotp_busy_notifier = NULL;
 	spin_unlock(&isotp_notifier_lock);
 	return NOTIFY_DONE;
 }
 
+static void isotp_sock_destruct(struct sock *sk)
+{
+	struct isotp_sock *so = isotp_sk(sk);
+
+	/* do the standard CAN sock destruct work */
+	can_sock_destruct(sk);
+
+	/* free potential extended PDU buffers */
+	if (so->rx.buf != so->rx.sbuf)
+		kfree(so->rx.buf);
+
+	if (so->tx.buf != so->tx.sbuf)
+		kfree(so->tx.buf);
+}
+
 static int isotp_init(struct sock *sk)
 {
 	struct isotp_sock *so = isotp_sk(sk);
 
 	so->ifindex = 0;
@@ -1664,10 +1673,13 @@ static int isotp_init(struct sock *sk)
 
 	spin_lock(&isotp_notifier_lock);
 	list_add_tail(&so->notifier, &isotp_notifier_list);
 	spin_unlock(&isotp_notifier_lock);
 
+	/* re-assign default can_sock_destruct() reference */
+	sk->sk_destruct = isotp_sock_destruct;
+
 	return 0;
 }
 
 static __poll_t isotp_poll(struct file *file, struct socket *sock, poll_table *wait)
 {
-- 
2.51.0