Re: [PATCH net v3] can: j1939: validate the EDPO packet number

[Marc Kleine-Budde]

On 09.10.2026 18:55:59, Quchaosheng wrote:
> From: Chaosheng Qu <quchaosheng000406@xxxxxxx>
>
> ETP.CM_DPO is peer controlled and its packet number is accepted without
> any validation.  It names the packet a transmission window starts at, and
> j1939_xtp_rx_dat_one() places incoming data at "dat[0] - 1 + pkt.dpo", so
> a value past the end of the message moves pkt.dpo beyond the reassembled
> buffer.
>
> The transfer still completes normally in that case.  With
> pkt.dpo == pkt.total, a closing ETP.DT frame with dat[0] == 0 evaluates to
> packet pkt.total - 1, which is the last in-order packet, so pkt.rx reaches
> pkt.total and the receiver emits its own EOMA.  The local loopback of that
> EOMA runs j1939_session_completed(), which asks
> j1939_session_skb_get() for offset "pkt.dpo * 7" -- one full packet past
> the end of the queued skb -- gets NULL, and passes it straight to
> j1939_sk_recv(), which dereferences oskb->sk.
>
> Reaching the dereference needs two things, and this patch keeps them as
> two separate changes because neither is sufficient alone:
>
>   - the range check in j1939_xtp_rx_dpo_one(), which stops pkt.dpo from
>     leaving the message
>   - the NULL guard in j1939_session_completed(), which covers the offset
>     lookup coming up empty for any other reason
>
> The reproducer in the link was run three times in each configuration: the
> unpatched kernel panics 3/3, a kernel with only the NULL guard still
> panics 3/3, and this patch gives 0/3.  The middle result is the point --
> it shows the guard does not close this hole on its own, so the range check
> is not redundant with it.
>
> Nothing on the path checks a capability: net/can has no CAP_NET_RAW check
> at all, CAN_J1939 does not require one, and the only capability gate
> nearby is SO_J1939_SEND_PRIO < 2, which asks for CAP_NET_ADMIN and is not
> needed here.  The vcan interface used by the reproducer has to be created
> first, which normally needs privileges; given the interface, any user can
> reach this.
>
>   ETP.CM_RTS (size 1786)  -> total = 256 packets
>   ETP.CM_DPO (packet 0)
>   ETP.DT seq 1..255       -> packets 0..254, pkt.rx = 255
>   ETP.CM_DPO (packet 256) -> pkt.dpo = 256, unvalidated
>   ETP.DT seq 0            -> packet 255 == pkt.rx, completes the transfer
>
> J1939_XTP_ABORT_BAD_EDPO_OFFSET and its "Bad EDPO offset" string are
> already in the tree and have no caller; the range check uses that abort.
>
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Link: https://sashiko.dev/#/patchset/20260928193312.553632-1-mkl@xxxxxxxxxxxxxx?part=3
> Assisted-by: LLM
> Signed-off-by: Chaosheng Qu <quchaosheng000406@xxxxxxx>

I you send another version of this patch, please use v5, as the
automated tools are confused.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Attachment: signature.asc
Description: PGP signature