[PATCH net 1/2] can: j1939: fix null-ptr-deref in j1939_session_completed()

[Xue Boyang]

j1939_session_completed() distributes the reassembled message to all
matching receivers via j1939_sk_recv().  The skb is obtained from
j1939_session_skb_get(), which looks up the receive queue starting at
session->pkt.dpo * 7 and returns NULL if no queued skb covers that
offset.

The DPO (Data Packet Offset) command is accepted without validation in
j1939_xtp_rx_dpo_one(), so a peer can move pkt.dpo to a packet number
beyond the end of the reassembled buffer.  The TP.DT placement uses
"dat[0] - 1 + session->pkt.dpo" arithmetic, so with pkt.dpo ==
pkt.total a final DT frame with dat[0] == 0 is still accepted as the
last in-order packet (packet == rx) and the transfer completes
normally.  j1939_session_completed() then looks up the offset
pkt.total * 7, which is outside the receive buffer, so
j1939_session_skb_get() returns NULL and it is passed to
j1939_sk_recv(), which dereferences it via j1939_skb_to_cb().

This is reachable by an unprivileged user through a virtual CAN
interface (all required capabilities are obtainable in a user
namespace) with the frame sequence:

    ETP.CM_RTS (size 1786)  -> total = 256 packets
    ETP.CM_DPO (packet 0)
    ETP.DT x255             -> packets 0..254, rx = 255
    ETP.CM_DPO (packet 256) -> dpo = 256, unvalidated
    ETP.DT (dat[0] = 0)     -> packet 255 == rx, completes transfer

resulting in:

    KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
    RIP: 0010:j1939_sk_recv+0xd8/0x4b0
    Call Trace:
     <IRQ>
     ? j1939_session_skb_get_by_offset+0x14f/0x2a0
     j1939_xtp_rx_eoma+0x43d/0x500
     j1939_tp_recv+0x930/0xca0
     j1939_can_recv+0x696/0x900
     can_rcv_filter+0x223/0x760
     can_receive+0x222/0x310
     can_rcv+0x204/0x370

(j1939_session_completed() is inlined into j1939_xtp_rx_eoma() in
this build; trace taken from v7.3.0-rc3-00313-gdaf677c2c644 + KASAN.)

The only other caller of j1939_session_skb_get(),
j1939_simple_txnext(), already checks for NULL; do the same here and
skip the distribution while still running
j1939_session_deactivate_activate_next() so the session is not left
stuck.

Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
Signed-off-by: Xue Boyang <m18335910246@xxxxxxx>
---
 net/can/j1939/transport.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index 8fcfd13e5e6f..ecf7f245f837 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -1244,8 +1244,10 @@ static void j1939_session_completed(struct j1939_session *session)
 	if (!session->transmission) {
 		se_skb = j1939_session_skb_get(session);
 		/* distribute among j1939 receivers */
-		j1939_sk_recv(session->priv, se_skb);
-		consume_skb(se_skb);
+		if (se_skb) {
+			j1939_sk_recv(session->priv, se_skb);
+			consume_skb(se_skb);
+		}
 	}
 
 	j1939_session_deactivate_activate_next(session);
-- 
2.53.0