[PATCH net 0/2] can: j1939: fix NULL deref on ETP completion with out-of-range DPO

[Xue Boyang]

Hello,

while auditing the J1939 transport layer of v7.3-rc3 we found a NULL
pointer dereference in j1939_session_completed() that is reachable by
an unprivileged user through a virtual CAN interface (all required
capabilities are obtainable inside a user namespace, no hardware
needed).

Root cause: ETP.CM_DPO is accepted without validation, and the final
message distribution looks up the receive queue at pkt.dpo * 7.  With
pkt.dpo moved past the end of the reassembled buffer the lookup
returns NULL, which is passed to j1939_sk_recv() and dereferenced.
The interesting part is that the transfer still *completes normally*
beforehand: TP.DT placement uses "dat[0] - 1 + pkt.dpo" arithmetic,
so with pkt.dpo == pkt.total a final DT frame with dat[0] == 0 is
accepted as the last in-order packet.  The trigger is therefore fully
deterministic - no race, no memory pressure:

    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

    KASAN: null-ptr-deref in range [0x18-0x1f]
    RIP: 0010:j1939_sk_recv+0xd8/0x4b0
    Call Trace:
     j1939_xtp_rx_eoma+0x43d/0x500
     j1939_tp_recv+0x930/0xca0
     j1939_can_recv+0x696/0x900

Both patches verified on v7.3.0-rc3-00313-gdaf677c2c644 + KASAN under
QEMU: the oops reproduces on vanilla, disappears with the series
applied, and the reproducer then completes normally (session
completes, message delivered once with correct dpo).  Self-contained
C reproducer available on request.

Patch 1 fixes the oops by checking the return value of
j1939_session_skb_get() like the only other caller
(j1939_simple_txnext()) already does, while still running
j1939_session_deactivate_activate_next() so the session is not left
stuck.

Patch 2 is a small hardening on top: reject DPO packet numbers that
point past the end of the transfer (pkt.dpo > pkt.total) with an
abort, mirroring the error handling of j1939_xtp_rx_dat_one().  Note
that pkt.dpo == pkt.total combined with dat[0] == 0 still encodes the
final packet and is intentionally kept working - patch 1 is what
actually covers that case.  Happy to drop or rework patch 2 if you
prefer a different boundary.

One design question for the maintainers: the DT placement offset
(dat[0] - 1 + pkt.dpo) and the completion lookup offset (pkt.dpo * 7)
use different semantics for the same field.  It works because window
rebases advance pkt.dpo in lockstep with the packet counter, but it
is the source of this bug class.  A future cleanup could store the
byte offset once, or clamp at DPO-receive time.

Xue Boyang (2):
  can: j1939: fix null-ptr-deref in j1939_session_completed()
  can: j1939: validate the DPO packet number

 net/can/j1939/transport.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
2.53.0