[PATCH net] can: j1939: transport: validate the DPO packet offset

[PATCH net] can: j1939: transport: validate the DPO packet offset

[Sun Yiqi]

j1939_xtp_rx_dpo_one() stores the packet offset from a received ETP DPO
command without checking it against the session's total packet count.
An offset greater than or equal to that count makes
j1939_session_skb_get() return NULL. A subsequent EOMA can then cause
j1939_session_completed() to pass the NULL skb to j1939_sk_recv(),
resulting in a NULL pointer dereference.

Reject an out-of-range offset before storing it and abort the session
with J1939_XTP_ABORT_BAD_EDPO_OFFSET. Keeping the previous valid offset
also prevents this lookup failure if an EOMA arrives while the session
is waiting for the abort to complete.

Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
Cc: stable@vger.kernel.org
Signed-off-by: Sun Yiqi <sunyiqi2@huawei.com>
---
 net/can/j1939/transport.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index 8fcfd13e5e6f..bda50432a72e 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -1823,14 +1823,22 @@ static void j1939_xtp_rx_dpo_one(struct j1939_session *session,
 				 struct sk_buff *skb)
 {
 	const u8 *dat = skb->data;
+	unsigned int packet;
 
 	if (j1939_xtp_rx_cmd_bad_pgn(session, skb))
 		return;
 
 	netdev_dbg(session->priv->ndev, "%s: 0x%p\n", __func__, session);
 
+	packet = j1939_etp_ctl_to_packet(skb->data);
+	if (packet >= session->pkt.total) {
+		j1939_session_timers_cancel(session);
+		j1939_session_cancel(session, J1939_XTP_ABORT_BAD_EDPO_OFFSET);
+		return;
+	}
+
 	/* transmitted without problems */
-	session->pkt.dpo = j1939_etp_ctl_to_packet(skb->data);
+	session->pkt.dpo = packet;
 	session->last_cmd = dat[0];
 	j1939_tp_set_rxtimeout(session, 750);
 
-- 
2.51.0.windows.1

Re: [PATCH net] can: j1939: transport: validate the DPO packet offset

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.