[PATCH net v2 4/4] tls: device: fix next-record queued checks in core-driven RX resync

[Rishikesh Jethwani]

TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT resync tells the device where the
next record starts. That is only useful if the device has not yet seen
the next record's header, so both places that send the hint first check
whether any of the next record is already queued on the socket, using
tcp_inq(). tcp_inq() counts from copied_seq, so the checks are only
right if they know how much of the current record has been consumed.

tls_device_core_ctrl_rx_resync() runs when a record is entirely
encrypted and checks tcp_inq() > rxm->full_len. With the original
strparser the record had already been read out, so this asked for more
than a whole record beyond the current one instead of any data at all,
contrary to the intent stated in the commit that added it. Since the
TLS-specific strparser the check is correct in its normal mode, where
the record is still queued, but copy mode has consumed the record and
keeps the original error. The hint can then be sent immediately for a
boundary the device has already passed.

tls_device_rx_resync_new_rec() runs when a new record header is parsed
and checks tcp_inq() > rcd_len, relying on the queue still holding the
record being parsed. That does not hold once part of the record has
been consumed: in copy mode, and with the original strparser, which
also consumed incomplete headers before parsing them. The check then
underestimates what is queued and can send the hint after the next
header has arrived, too late for the device to use it.

In both cases the attempt is wasted and the next try is made only after
backoff, keeping the stream on software decryption longer.

Count from the record start instead of copied_seq. With the record
start now tracked by the strparser in both modes, rcv_nxt minus that
start is the number of bytes received for this record and beyond,
regardless of how much has been consumed. The
tls_device_rx_resync_nh_delay tracepoint now reports received sequence
space from the record start, including data already consumed in copy
mode. A FIN right after the record now counts as queued data, which
only defers a hint that could not be used, since no record can follow
it.

Only drivers using TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT are affected.

Found by code inspection while working on TLS 1.3 hardware offload
support.

Fixes: f953d33ba122 ("net/tls: add kernel-driven TLS RX resync")
Signed-off-by: Rishikesh Jethwani <rjethwani@xxxxxxxxxxxxxxx>
---
 net/tls/tls_device.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index a6c4d0cc54c7..8181564600c1 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -805,10 +805,10 @@ void tls_device_rx_resync_new_rec(struct sock *sk, u32 rcd_len, u32 seq)
 		if (likely(!rx_ctx->resync_nh_do_now))
 			return;
 
-		/* head of next rec is already in, note that the sock_inq will
-		 * include the currently parsed message when called from parser
+		/* head of next rec is already in, count from the record start
+		 * since copy mode may have consumed part of it already
 		 */
-		sock_data = tcp_inq(sk);
+		sock_data = READ_ONCE(tcp_sk(sk)->rcv_nxt) - seq;
 		if (sock_data > rcd_len) {
 			trace_tls_device_rx_resync_nh_delay(sk, sock_data,
 							    rcd_len);
@@ -841,6 +841,7 @@ static void tls_device_core_ctrl_rx_resync(struct tls_context *tls_ctx,
 {
 	struct tls_sw_context_rx *sw_ctx = tls_sw_ctx_rx(tls_ctx);
 	struct strp_msg *rxm;
+	u32 next_seq;
 
 	/* device will request resyncs by itself based on stream scan */
 	if (ctx->resync_type != TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT)
@@ -866,9 +867,10 @@ static void tls_device_core_ctrl_rx_resync(struct tls_context *tls_ctx,
 		ctx->resync_nh.decrypted_tgt += TLS_DEVICE_RESYNC_NH_MAX_IVAL;
 
 	rxm = strp_msg(skb);
+	next_seq = tls_strp_msg_seq(sw_ctx) + rxm->full_len;
 
 	/* head of next rec is already in, parser will sync for us */
-	if (tcp_inq(sk) > rxm->full_len) {
+	if (after(READ_ONCE(tcp_sk(sk)->rcv_nxt), next_seq)) {
 		trace_tls_device_rx_resync_nh_schedule(sk);
 		ctx->resync_nh_do_now = 1;
 	} else {
@@ -878,9 +880,7 @@ static void tls_device_core_ctrl_rx_resync(struct tls_context *tls_ctx,
 		memcpy(rcd_sn, tls_ctx->rx.rec_seq, prot->rec_seq_size);
 		tls_bigint_increment(rcd_sn, prot->rec_seq_size);
 
-		tls_device_resync_rx(tls_ctx, sk,
-				     tls_strp_msg_seq(sw_ctx) +
-				     rxm->full_len, rcd_sn);
+		tls_device_resync_rx(tls_ctx, sk, next_seq, rcd_sn);
 	}
 }
 
-- 
2.50.1