[PATCH v5 3/4] can: isotp: fix race between RX timer and consecutive frame reception

From: Oliver Hartkopp <[email protected]>

When receiving a Consecutive Frame (CF), hrtimer_cancel() is called to stop
the RX timer. However, if the timer handler (isotp_rx_timer_handler) has
already fired and is running concurrently on another CPU, hrtimer_cancel()
will wait for it to finish.

By the time it returns, the timer handler might have already reported an
ETIMEDOUT error and reset the RX state machine. If we blindly continue
processing the CF, we corrupt the newly reset or idle state.

Fix this by verifying that the state is still ISOTP_WAIT_DATA after
canceling the hrtimer. If it changed, the reception has already timed out,
so we drop the frame.

Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
Reported-by: [email protected]
Link: https://lore.kernel.org/linux-can/[email protected]/
Signed-off-by: Oliver Hartkopp <[email protected]>
---
 net/can/isotp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index e7597ca3e140..c0edbd35516c 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -575,10 +575,18 @@ static int isotp_rcv_cf(struct sock *sk, struct canfd_frame *cf, int ae,
 		so->lastrxcf_tstamp = skb->tstamp;
 	}
 
 	hrtimer_cancel(&so->rxtimer);
 
+	/* isotp_rx_timer_handler() may have raced us for so->rx.state
+	 * while hrtimer_cancel() above waited for it to finish, already
+	 * reporting ETIMEDOUT and resetting the reception; don't process
+	 * this CF into a reassembly that has already been given up on.
+	 */
+	if (so->rx.state != ISOTP_WAIT_DATA)
+		return 1;
+
 	/* CFs are never longer than the FF */
 	if (cf->len > so->rx.ll_dl)
 		return 1;
 
 	/* CFs have usually the LL_DL length */

-- 
2.53.0