* [PATCH 1/2] can: gw: fix OOB heap access in cgw_csum_crc8_rel()
[not found] <20260318161914.15140-1-socketcan@hartkopp.net>
@ 2026-03-18 16:19 ` Oliver Hartkopp
0 siblings, 0 replies; 16+ messages in thread
From: Oliver Hartkopp @ 2026-03-18 16:19 UTC (permalink / raw)
To: ali.norouzi, security, torvalds; +Cc: mkl, socketcan, stable
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/2] can: gw: fix OOB heap access in cgw_csum_crc8_rel()
@ 2026-03-18 16:51 Oliver Hartkopp
2026-03-18 16:51 ` [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg() Oliver Hartkopp
0 siblings, 1 reply; 16+ messages in thread
From: Oliver Hartkopp @ 2026-03-18 16:51 UTC (permalink / raw)
To: linux-can; +Cc: Ali Norouzi, stable, 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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-18 16:51 [PATCH 1/2] can: gw: fix OOB heap access in cgw_csum_crc8_rel() Oliver Hartkopp
@ 2026-03-18 16:51 ` Oliver Hartkopp
2026-03-19 13:13 ` Oliver Hartkopp
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Oliver Hartkopp @ 2026-03-18 16:51 UTC (permalink / raw)
To: linux-can; +Cc: Oliver Hartkopp, stable, Ali Norouzi
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-18 16:51 ` [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg() Oliver Hartkopp
@ 2026-03-19 13:13 ` Oliver Hartkopp
2026-03-19 13:33 ` Marc Kleine-Budde
2026-03-19 13:23 ` Marc Kleine-Budde
2026-03-19 14:58 ` Marc Kleine-Budde
2 siblings, 1 reply; 16+ messages in thread
From: Oliver Hartkopp @ 2026-03-19 13:13 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: stable, Ali Norouzi, linux-can
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)
> {
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-18 16:51 ` [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg() Oliver Hartkopp
2026-03-19 13:13 ` Oliver Hartkopp
@ 2026-03-19 13:23 ` Marc Kleine-Budde
2026-03-19 14:58 ` Marc Kleine-Budde
2 siblings, 0 replies; 16+ messages in thread
From: Marc Kleine-Budde @ 2026-03-19 13:23 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, stable, Ali Norouzi
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-19 13:13 ` Oliver Hartkopp
@ 2026-03-19 13:33 ` Marc Kleine-Budde
2026-03-19 13:34 ` Oliver Hartkopp
0 siblings, 1 reply; 16+ messages in thread
From: Marc Kleine-Budde @ 2026-03-19 13:33 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: stable, Ali Norouzi, linux-can
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-19 13:33 ` Marc Kleine-Budde
@ 2026-03-19 13:34 ` Oliver Hartkopp
0 siblings, 0 replies; 16+ messages in thread
From: Oliver Hartkopp @ 2026-03-19 13:34 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: stable, Ali Norouzi, linux-can
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-18 16:51 ` [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg() Oliver Hartkopp
2026-03-19 13:13 ` Oliver Hartkopp
2026-03-19 13:23 ` Marc Kleine-Budde
@ 2026-03-19 14:58 ` Marc Kleine-Budde
2026-03-19 15:10 ` Ali Norouzi
2026-03-19 15:12 ` Oliver Hartkopp
2 siblings, 2 replies; 16+ messages in thread
From: Marc Kleine-Budde @ 2026-03-19 14:58 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, stable, Ali Norouzi
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-19 14:58 ` Marc Kleine-Budde
@ 2026-03-19 15:10 ` Ali Norouzi
2026-03-19 15:16 ` Marc Kleine-Budde
2026-03-19 15:27 ` Oliver Hartkopp
2026-03-19 15:12 ` Oliver Hartkopp
1 sibling, 2 replies; 16+ messages in thread
From: Ali Norouzi @ 2026-03-19 15:10 UTC (permalink / raw)
To: Marc Kleine-Budde, Oliver Hartkopp
Cc: linux-can@vger.kernel.org, stable@vger.kernel.org
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 |
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-19 14:58 ` Marc Kleine-Budde
2026-03-19 15:10 ` Ali Norouzi
@ 2026-03-19 15:12 ` Oliver Hartkopp
2026-03-19 15:23 ` Marc Kleine-Budde
1 sibling, 1 reply; 16+ messages in thread
From: Oliver Hartkopp @ 2026-03-19 15:12 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, stable, Ali Norouzi
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-19 15:10 ` Ali Norouzi
@ 2026-03-19 15:16 ` Marc Kleine-Budde
2026-03-19 15:27 ` Oliver Hartkopp
1 sibling, 0 replies; 16+ messages in thread
From: Marc Kleine-Budde @ 2026-03-19 15:16 UTC (permalink / raw)
To: Ali Norouzi
Cc: Oliver Hartkopp, linux-can@vger.kernel.org,
stable@vger.kernel.org
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-19 15:12 ` Oliver Hartkopp
@ 2026-03-19 15:23 ` Marc Kleine-Budde
0 siblings, 0 replies; 16+ messages in thread
From: Marc Kleine-Budde @ 2026-03-19 15:23 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, stable, Ali Norouzi
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-19 15:10 ` Ali Norouzi
2026-03-19 15:16 ` Marc Kleine-Budde
@ 2026-03-19 15:27 ` Oliver Hartkopp
2026-03-19 15:50 ` Marc Kleine-Budde
1 sibling, 1 reply; 16+ messages in thread
From: Oliver Hartkopp @ 2026-03-19 15:27 UTC (permalink / raw)
To: Ali Norouzi, Marc Kleine-Budde
Cc: linux-can@vger.kernel.org, stable@vger.kernel.org
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 |
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-19 15:27 ` Oliver Hartkopp
@ 2026-03-19 15:50 ` Marc Kleine-Budde
2026-03-23 8:47 ` Oliver Hartkopp
0 siblings, 1 reply; 16+ messages in thread
From: Marc Kleine-Budde @ 2026-03-19 15:50 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Ali Norouzi, linux-can@vger.kernel.org, stable@vger.kernel.org
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-19 15:50 ` Marc Kleine-Budde
@ 2026-03-23 8:47 ` Oliver Hartkopp
2026-03-23 9:35 ` Marc Kleine-Budde
0 siblings, 1 reply; 16+ messages in thread
From: Oliver Hartkopp @ 2026-03-23 8:47 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Ali Norouzi, linux-can@vger.kernel.org, stable@vger.kernel.org
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
2026-03-23 8:47 ` Oliver Hartkopp
@ 2026-03-23 9:35 ` Marc Kleine-Budde
0 siblings, 0 replies; 16+ messages in thread
From: Marc Kleine-Budde @ 2026-03-23 9:35 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Ali Norouzi, linux-can@vger.kernel.org, stable@vger.kernel.org
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-03-23 9:36 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-18 16:51 [PATCH 1/2] can: gw: fix OOB heap access in cgw_csum_crc8_rel() Oliver Hartkopp
2026-03-18 16:51 ` [PATCH 2/2] can: isotp: fix tx.buf use-after-free in isotp_sendmsg() Oliver Hartkopp
2026-03-19 13:13 ` Oliver Hartkopp
2026-03-19 13:33 ` Marc Kleine-Budde
2026-03-19 13:34 ` Oliver Hartkopp
2026-03-19 13:23 ` Marc Kleine-Budde
2026-03-19 14:58 ` Marc Kleine-Budde
2026-03-19 15:10 ` Ali Norouzi
2026-03-19 15:16 ` Marc Kleine-Budde
2026-03-19 15:27 ` Oliver Hartkopp
2026-03-19 15:50 ` Marc Kleine-Budde
2026-03-23 8:47 ` Oliver Hartkopp
2026-03-23 9:35 ` Marc Kleine-Budde
2026-03-19 15:12 ` Oliver Hartkopp
2026-03-19 15:23 ` Marc Kleine-Budde
[not found] <20260318161914.15140-1-socketcan@hartkopp.net>
2026-03-18 16:19 ` [PATCH 1/2] can: gw: fix OOB heap access in cgw_csum_crc8_rel() Oliver Hartkopp
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox