]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
authorOliver Hartkopp <socketcan@hartkopp.net>
Thu, 19 Mar 2026 15:47:45 +0000 (16:47 +0100)
committerMarc Kleine-Budde <mkl@pengutronix.de>
Thu, 19 Mar 2026 16:16:02 +0000 (17:16 +0100)
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

index da3b72e7afccb731734b525ce2136f469ad889aa..2770f43f4951884658d54ac90bd1e0ae21c24102 100644 (file)
@@ -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;
 }