Skip to content

Commit 424e95d

Browse files
hartkoppmarckleinebudde
authored andcommitted
can: isotp: fix tx.buf use-after-free in isotp_sendmsg()
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: 96d1c81 ("can: isotp: add module parameter for maximum pdu size") Cc: [email protected] Reported-by: Ali Norouzi <[email protected]> Co-developed-by: Ali Norouzi <[email protected]> Signed-off-by: Ali Norouzi <[email protected]> Signed-off-by: Oliver Hartkopp <[email protected]> Link: https://patch.msgid.link/20260319-fix-can-gw-and-can-isotp-v2-2-c45d52c6d2d8@pengutronix.de Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent b9c310d commit 424e95d

1 file changed

Lines changed: 18 additions & 6 deletions

File tree

net/can/isotp.c

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,12 +1248,6 @@ static int isotp_release(struct socket *sock)
12481248
so->ifindex = 0;
12491249
so->bound = 0;
12501250

1251-
if (so->rx.buf != so->rx.sbuf)
1252-
kfree(so->rx.buf);
1253-
1254-
if (so->tx.buf != so->tx.sbuf)
1255-
kfree(so->tx.buf);
1256-
12571251
sock_orphan(sk);
12581252
sock->sk = NULL;
12591253

@@ -1622,6 +1616,21 @@ static int isotp_notifier(struct notifier_block *nb, unsigned long msg,
16221616
return NOTIFY_DONE;
16231617
}
16241618

1619+
static void isotp_sock_destruct(struct sock *sk)
1620+
{
1621+
struct isotp_sock *so = isotp_sk(sk);
1622+
1623+
/* do the standard CAN sock destruct work */
1624+
can_sock_destruct(sk);
1625+
1626+
/* free potential extended PDU buffers */
1627+
if (so->rx.buf != so->rx.sbuf)
1628+
kfree(so->rx.buf);
1629+
1630+
if (so->tx.buf != so->tx.sbuf)
1631+
kfree(so->tx.buf);
1632+
}
1633+
16251634
static int isotp_init(struct sock *sk)
16261635
{
16271636
struct isotp_sock *so = isotp_sk(sk);
@@ -1666,6 +1675,9 @@ static int isotp_init(struct sock *sk)
16661675
list_add_tail(&so->notifier, &isotp_notifier_list);
16671676
spin_unlock(&isotp_notifier_lock);
16681677

1678+
/* re-assign default can_sock_destruct() reference */
1679+
sk->sk_destruct = isotp_sock_destruct;
1680+
16691681
return 0;
16701682
}
16711683

0 commit comments

Comments
 (0)