All of lore.kernel.org
 help / color / mirror / Atom feed
From: Liu Chao <liuc63@xiaopeng.com>
To: Robin van der Gracht <robin@protonic.nl>,
	Oleksij Rempel <o.rempel@pengutronix.de>,
	Oliver Hartkopp <socketcan@hartkopp.net>,
	Marc Kleine-Budde <mkl@pengutronix.de>
Cc: kernel@pengutronix.de, linux-can@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, Liu Chao <liuc63@xiaopeng.com>
Subject: [PATCH net v2 2/2] can: j1939: check received packet count before completing session
Date: Thu, 10 Sep 2026 01:31:04 +0800	[thread overview]
Message-ID: <20260909173105.158202-3-liuc63@xiaopeng.com> (raw)
In-Reply-To: <20260909173105.158202-1-liuc63@xiaopeng.com>

j1939_xtp_rx_eoma_one() marks a session complete as soon as it sees
an EOMA without verifying that all data packets arrived.  Add a pkt.rx
check so a session with missing packets gets aborted instead of
delivering a short message to userspace.

Also tighten the existing EOMA size-mismatch warning to actually abort
for receive sessions instead of just logging.  The warning itself is
kept for both rx and tx paths so transmitter-side protocol violations
are still logged.

Only unicast receive sessions are gated:

 - Transmitter sessions track pkt.rx via loopback confirmations which
   may legitimately lag behind the real transmit count, so the check
   would cause false aborts on the tx path.

 - BAM (broadcast) sessions never go through EOMA -- they complete via
   the final flag in j1939_xtp_rx_dat_one() when pkt.rx reaches
   pkt.total directly.

The error path skips sessions already in J1939_SESSION_WAITING_ABORT:
cancelling the deactivation timer there would leave the session on
active_session_list forever, since j1939_session_cancel() only re-arms
that timer for sessions which are not yet aborting.  The same failure
mode was previously fixed for the second-RTS path by 1809c82aa073
("net: can: j1939: j1939_xtp_rx_rts_session_active(): deactivate
session upon receiving the second rts").

Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
Cc: stable@vger.kernel.org
Signed-off-by: Liu Chao <liuc63@xiaopeng.com>
---
 net/can/j1939/transport.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index c40db4337..fe447077b 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -1396,6 +1396,7 @@ static void
 j1939_xtp_rx_eoma_one(struct j1939_session *session, struct sk_buff *skb)
 {
 	struct j1939_sk_buff_cb *skcb = j1939_skb_to_cb(skb);
+	unsigned int expected_total;
 	const u8 *dat;
 	int len;
 
@@ -1411,9 +1412,22 @@ j1939_xtp_rx_eoma_one(struct j1939_session *session, struct sk_buff *skb)
 
 	if (session->total_message_size != len) {
 		netdev_warn_once(session->priv->ndev,
-				 "%s: 0x%p: Incorrect size. Expected: %i; got: %i.\n",
+				 "%s: 0x%p: EOMA size mismatch, expected %u got %d\n",
 				 __func__, session, session->total_message_size,
 				 len);
+		if (!session->transmission)
+			goto out_session_cancel;
+	}
+
+	if (!session->transmission) {
+		expected_total = (session->total_message_size + 6) / 7;
+		if (session->pkt.rx < expected_total) {
+			netdev_warn_once(session->priv->ndev,
+					 "%s: 0x%p: EOMA but only %u/%u data packets rx'd\n",
+					 __func__, session,
+					 session->pkt.rx, expected_total);
+			goto out_session_cancel;
+		}
 	}
 
 	netdev_dbg(session->priv->ndev, "%s: 0x%p\n", __func__, session);
@@ -1422,6 +1436,16 @@ j1939_xtp_rx_eoma_one(struct j1939_session *session, struct sk_buff *skb)
 	j1939_session_timers_cancel(session);
 	/* transmitted without problems */
 	j1939_session_completed(session);
+	return;
+
+ out_session_cancel:
+	/* Don't touch timers if the session is already aborting;
+	 * cancelling the deactivation timer here would leak the session.
+	 */
+	if (session->state >= J1939_SESSION_WAITING_ABORT)
+		return;
+	j1939_session_timers_cancel(session);
+	j1939_session_cancel(session, J1939_XTP_ABORT_FAULT);
 }
 
 static void
-- 
2.50.1


  parent reply	other threads:[~2026-09-09 17:36 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-09 17:31 [PATCH net v2 0/2] can: j1939: tighten TP receive-path checks Liu Chao
2026-09-09 17:31 ` [PATCH net v2 1/2] can: j1939: reject TP RTS with wrong packet count Liu Chao
2026-09-09 17:31 ` Liu Chao [this message]
2026-09-10 17:37   ` [PATCH net v2 2/2] can: j1939: check received packet count before completing session sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260909173105.158202-3-liuc63@xiaopeng.com \
    --to=liuc63@xiaopeng.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=robin@protonic.nl \
    --cc=socketcan@hartkopp.net \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.