[PATCH net v7 3/4] can: j1939: drop the message when the reassembled skb is gone

[Chaosheng Qu]

j1939_session_completed() takes the skb for the completed message and hands
it to j1939_sk_recv() without checking the result.  j1939_session_skb_get()
returns NULL when the offset it computes lands past the end of the queue.

The offset is "pkt.dpo * 7", and pkt.dpo is bounded when an EDPO is
received,
so this should not happen.  Guard it anyway: a NULL dereference in the
receive path is a kernel panic, while a dropped message is a lost datagram.
j1939_simple_txnext(), the only other caller of j1939_session_skb_get(),
already checks for NULL.

Signed-off-by: Chaosheng Qu <quchaosheng000406@xxxxxxx>
---
 net/can/j1939/transport.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index b0e62822b56e..5f05c15114ce 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -1269,9 +1269,19 @@ 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);
+
+		/* Defense in depth: pkt.dpo is range checked when an EDPO is
+		 * received, so the lookup should always find the skb.  Drop the
+		 * message rather than hand a NULL skb to j1939_sk_recv() if the
+		 * offset arithmetic ever regresses.  The only other caller of
+		 * j1939_session_skb_get(), j1939_simple_txnext(), already does
+		 * this.
+		 */
+		if (se_skb) {
+			/* distribute among j1939 receivers */
+			j1939_sk_recv(session->priv, se_skb);
+			consume_skb(se_skb);
+		}
 	}
 
 	j1939_session_deactivate_activate_next(session);
-- 
2.43.0