public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/5] can: netlink: can_changelink(): add missing error handling to call can_ctrlmode_changelink()
       [not found] <20260323103224.218099-1-mkl@pengutronix.de>
@ 2026-03-23 10:27 ` Marc Kleine-Budde
  2026-03-24 11:30   ` patchwork-bot+netdevbpf
  2026-03-23 10:28 ` [PATCH net 4/5] can: gw: fix OOB heap access in cgw_csum_crc8_rel() Marc Kleine-Budde
  2026-03-23 10:28 ` [PATCH net 5/5] can: isotp: fix tx.buf use-after-free in isotp_sendmsg() Marc Kleine-Budde
  2 siblings, 1 reply; 4+ messages in thread
From: Marc Kleine-Budde @ 2026-03-23 10:27 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde, stable

In commit e1a5cd9d6665 ("can: netlink: add can_ctrlmode_changelink()") the
CAN Control Mode (IFLA_CAN_CTRLMODE) handling was factored out into the
can_ctrlmode_changelink() function. But the call to
can_ctrlmode_changelink() is missing the error handling.

Add the missing error handling and propagation to the call
can_ctrlmode_changelink().

Cc: stable@vger.kernel.org
Fixes: e1a5cd9d6665 ("can: netlink: add can_ctrlmode_changelink()")
Link: https://patch.msgid.link/20260310-can_ctrlmode_changelink-add-error-handling-v1-1-0daf63d85922@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/dev/netlink.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 0498198a4696..766d455950f5 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -601,7 +601,9 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 	/* We need synchronization with dev->stop() */
 	ASSERT_RTNL();
 
-	can_ctrlmode_changelink(dev, data, extack);
+	err = can_ctrlmode_changelink(dev, data, extack);
+	if (err)
+		return err;
 
 	if (data[IFLA_CAN_BITTIMING]) {
 		struct can_bittiming bt;

base-commit: 8a63baadf08453f66eb582fdb6dd234f72024723
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH net 4/5] can: gw: fix OOB heap access in cgw_csum_crc8_rel()
       [not found] <20260323103224.218099-1-mkl@pengutronix.de>
  2026-03-23 10:27 ` [PATCH net 1/5] can: netlink: can_changelink(): add missing error handling to call can_ctrlmode_changelink() Marc Kleine-Budde
@ 2026-03-23 10:28 ` Marc Kleine-Budde
  2026-03-23 10:28 ` [PATCH net 5/5] can: isotp: fix tx.buf use-after-free in isotp_sendmsg() Marc Kleine-Budde
  2 siblings, 0 replies; 4+ messages in thread
From: Marc Kleine-Budde @ 2026-03-23 10:28 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Ali Norouzi, stable,
	Oliver Hartkopp, Marc Kleine-Budde

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>
Link: https://patch.msgid.link/20260319-fix-can-gw-and-can-isotp-v2-1-c45d52c6d2d8@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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
@@ -375,10 +375,10 @@ static void cgw_csum_crc8_rel(struct canfd_frame *cf,
 		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]];
 	}
 
@@ -397,7 +397,7 @@ static void cgw_csum_crc8_rel(struct canfd_frame *cf,
 		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,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH net 5/5] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
       [not found] <20260323103224.218099-1-mkl@pengutronix.de>
  2026-03-23 10:27 ` [PATCH net 1/5] can: netlink: can_changelink(): add missing error handling to call can_ctrlmode_changelink() Marc Kleine-Budde
  2026-03-23 10:28 ` [PATCH net 4/5] can: gw: fix OOB heap access in cgw_csum_crc8_rel() Marc Kleine-Budde
@ 2026-03-23 10:28 ` Marc Kleine-Budde
  2 siblings, 0 replies; 4+ messages in thread
From: Marc Kleine-Budde @ 2026-03-23 10:28 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp, stable,
	Ali Norouzi, Marc Kleine-Budde

From: Oliver Hartkopp <socketcan@hartkopp.net>

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: 96d1c81e6a04 ("can: isotp: add module parameter for maximum pdu size")
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: Ali Norouzi <ali.norouzi@keysight.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260319-fix-can-gw-and-can-isotp-v2-2-c45d52c6d2d8@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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
@@ -1248,12 +1248,6 @@ static int isotp_release(struct socket *sock)
 	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;
 
@@ -1622,6 +1616,21 @@ static int isotp_notifier(struct notifier_block *nb, unsigned long msg,
 	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);
@@ -1666,6 +1675,9 @@ static int isotp_init(struct sock *sk)
 	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;
 }
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net 1/5] can: netlink: can_changelink(): add missing error handling to call can_ctrlmode_changelink()
  2026-03-23 10:27 ` [PATCH net 1/5] can: netlink: can_changelink(): add missing error handling to call can_ctrlmode_changelink() Marc Kleine-Budde
@ 2026-03-24 11:30   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-03-24 11:30 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: netdev, davem, kuba, linux-can, kernel, stable

Hello:

This series was applied to netdev/net.git (main)
by Marc Kleine-Budde <mkl@pengutronix.de>:

On Mon, 23 Mar 2026 11:27:57 +0100 you wrote:
> In commit e1a5cd9d6665 ("can: netlink: add can_ctrlmode_changelink()") the
> CAN Control Mode (IFLA_CAN_CTRLMODE) handling was factored out into the
> can_ctrlmode_changelink() function. But the call to
> can_ctrlmode_changelink() is missing the error handling.
> 
> Add the missing error handling and propagation to the call
> can_ctrlmode_changelink().
> 
> [...]

Here is the summary with links:
  - [net,1/5] can: netlink: can_changelink(): add missing error handling to call can_ctrlmode_changelink()
    https://git.kernel.org/netdev/net/c/cadf6019231b
  - [net,2/5] can: mcp251x: add error handling for power enable in open and resume
    https://git.kernel.org/netdev/net/c/7a57354756c7
  - [net,3/5] can: statistics: add missing atomic access in hot path
    https://git.kernel.org/netdev/net/c/46eee1661aa9
  - [net,4/5] can: gw: fix OOB heap access in cgw_csum_crc8_rel()
    https://git.kernel.org/netdev/net/c/b9c310d72783
  - [net,5/5] can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
    https://git.kernel.org/netdev/net/c/424e95d62110

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-24 11:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260323103224.218099-1-mkl@pengutronix.de>
2026-03-23 10:27 ` [PATCH net 1/5] can: netlink: can_changelink(): add missing error handling to call can_ctrlmode_changelink() Marc Kleine-Budde
2026-03-24 11:30   ` patchwork-bot+netdevbpf
2026-03-23 10:28 ` [PATCH net 4/5] can: gw: fix OOB heap access in cgw_csum_crc8_rel() Marc Kleine-Budde
2026-03-23 10:28 ` [PATCH net 5/5] can: isotp: fix tx.buf use-after-free in isotp_sendmsg() Marc Kleine-Budde

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox