[PATCH net v2 0/4] tls: device: fix RX record start tracking after strparser rework

[PATCH net v2 0/4] tls: device: fix RX record start tracking after strparser rework

[Rishikesh Jethwani]

Since commit 84c61fe1a75b ("tls: rx: do not use the standard
strparser"), the record being processed stays in the TCP receive queue
in the strparser's normal mode, so tcp_sk(sk)->copied_seq points at the
start of the current record rather than past it. In copy mode it is past
the record, plus one if TCP consumed a FIN with it. Several places in
tls_device.c still derive record positions from copied_seq, and the
copy-mode header parse reports a record start that ignores where the
record begins within the first skb.

Patch 1 makes the strparser save each record's TCP sequence when it
starts parsing the record, before anything can switch it to copy mode,
and adds tls_strp_msg_seq() to return it. It uses that to fix the record
start reported by the tls_device_decrypted tracepoint.

Patch 2 uses the helper to fix the next-record TCP sequence passed to
the device by core-driven RX resync (TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT,
used only by nfp). In normal mode the device was told that the next
record starts where the current one does.

Patch 3 fixes the record start passed to tls_device_rx_resync_new_rec()
when the header is parsed in copy mode. It was the start of the first
queued skb rather than of the record, so DRIVER_REQ and
DRIVER_REQ_ASYNC resyncs could be missed and CORE_NEXT_HINT given a
wrong boundary. Commit 9466ef3ec972 ("tls: fix RX desync on overlapping
skbs") made this path more likely right around a loss of device sync.

Patch 4 fixes the checks that decide whether the next record is already
queued before a CORE_NEXT_HINT hint is sent. They used tcp_inq(), which
counts from copied_seq, and now count from the record start, so the
result no longer depends on how much of the record has been consumed.
These checks were already wrong with the original strparser, so this
patch fixes the commit that added them rather than the rework. It
depends on patches 1-3.

Testing:

tools/testing/selftests/net/tls passed 655/655 on each host over
loopback, covering TLS 1.2 and 1.3 with AES-GCM-128/256, AES-CCM
and ChaCha20-Poly1305. SM4 and ARIA variants were excluded as those
algorithms are not enabled in the test kernel configuration.

On two mlx5 hosts with TLS 1.2 AES-GCM RX offload, all 28 cases
(1,344 records) passed plaintext and sequence checks. 26 cases reached
hardware decryption, four of them after a DRIVER_REQ_ASYNC resync. In
the other two, offload was installed mid-record and later headers were
split after two bytes; the NIC never issued a resync request, so these
cases cover only the sequence argument, not recovery.

Patch 1: all 1,344 tracepoint record starts matched the TCP captures,
covering normal parsing, copy mode, and three records where the copy
also consumed a FIN.

Patch 3: in 192 copied-header calls the old expression would have
reported a wrong record start; the fixed argument matched the wire
sequence in each. Hardware recovery through one of these repaired
calls was not observed.

Patches 2 and 4: these affect only TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT,
which mlx5 does not use, so they are verified by analysis only.
Review of the device-side effect from anyone with nfp kTLS RX would be
welcome.

Changes in v2:
- Save the record start whenever the strparser starts a new record
  instead of on entry to copy mode, and keep the tracking in the
  strparser; rename tls_device_rx_rec_start() to tls_strp_msg_seq()
  (Sabrina).
- Say in each commit message how the issue was found.
- Add patch 3 (record start reported to RX resync in copy mode) and
  patch 4 (next-record queued checks for CORE_NEXT_HINT).
- Expand hardware testing on mlx5 (28 cases, tracepoint and
  resync-argument checks) and add software selftest results.

v1: https://lore.kernel.org/netdev/20261015201135.2744031-1-rjethwani@purestorage.com/

Rishikesh Jethwani (4):
  tls: device: fix record start in the tls_device_decrypted tracepoint
  tls: device: fix next-record sequence in core-driven RX resync
  tls: device: fix record start reported to RX resync in copy mode
  tls: device: fix next-record queued checks in core-driven RX resync

 include/net/tls.h    |  2 ++
 net/tls/tls.h        |  6 ++++++
 net/tls/tls_device.c | 16 +++++++++-------
 net/tls/tls_strp.c   |  3 +++
 net/tls/tls_sw.c     |  2 +-
 5 files changed, 21 insertions(+), 8 deletions(-)

-- 
2.50.1

[PATCH net v2 1/4] tls: device: fix record start in the tls_device_decrypted tracepoint

[Rishikesh Jethwani]

tls_device_decrypted() reports the TCP sequence of the record it is
classifying as tcp_sk(sk)->copied_seq - rxm->full_len. The original
strparser read every record out of the TCP receive queue before the TLS
layer looked at it, so copied_seq had already moved past the record.

The TLS-specific strparser leaves the record in the TCP receive queue in
its normal mode and only calls tcp_read_done() from
tls_strp_msg_consume(), after decryption. When the tracepoint fires,
copied_seq still points at the first byte of the record, and the
reported tcp_seq is one record length too low.

In copy mode (e.g. when the receive queue is short under memory
pressure or with a collapsed receive window, or the record spans
overlapping or mixed decrypted/undecrypted skbs), tcp_read_sock() has
already advanced copied_seq past the record. It can also consume a FIN
that ends the record's last skb, which adds one byte of sequence space
absent from the record length, so subtracting the length reports a
start one byte too high. copied_seq cannot tell whether a FIN was
consumed, so the start must be saved before the read.

Record the TCP sequence in the strparser whenever it starts parsing a
new record, before anything can switch it to copy mode, and report that
via a new tls_strp_msg_seq() helper. The value is then valid in both
modes without having to track which path entered copy mode.

No functional change beyond the tracepoint argument.

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

Fixes: 84c61fe1a75b ("tls: rx: do not use the standard strparser")
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 include/net/tls.h    | 2 ++
 net/tls/tls.h        | 6 ++++++
 net/tls/tls_device.c | 2 +-
 net/tls/tls_strp.c   | 3 +++
 4 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index e57bef58851e..e1e48a25cafa 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -123,6 +123,8 @@ struct tls_strparser {
 	bool msg_ready;
 
 	struct strp_msg stm;
+	/* TCP sequence of the first byte of the current record. */
+	u32 seq;
 
 	struct sk_buff *anchor;
 	struct work_struct work;
diff --git a/net/tls/tls.h b/net/tls/tls.h
index 60a37bdaaa25..a9f7fd79c69e 100644
--- a/net/tls/tls.h
+++ b/net/tls/tls.h
@@ -227,6 +227,12 @@ static inline bool tls_strp_msg_mixed_decrypted(struct tls_sw_context_rx *ctx)
 	return ctx->strp.mixed_decrypted;
 }
 
+static inline u32 tls_strp_msg_seq(struct tls_sw_context_rx *ctx)
+{
+	DEBUG_NET_WARN_ON_ONCE(!ctx->strp.msg_ready);
+	return ctx->strp.seq;
+}
+
 #ifdef CONFIG_TLS_DEVICE
 int tls_device_init(void);
 void tls_device_cleanup(void);
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index f11d0528fc43..fbfe6ec3df04 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -990,7 +990,7 @@ int tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx)
 		is_encrypted = 0;
 	}
 
-	trace_tls_device_decrypted(sk, tcp_sk(sk)->copied_seq - rxm->full_len,
+	trace_tls_device_decrypted(sk, tls_strp_msg_seq(sw_ctx),
 				   tls_ctx->rx.rec_seq, rxm->full_len,
 				   is_encrypted, is_decrypted);
 
diff --git a/net/tls/tls_strp.c b/net/tls/tls_strp.c
index 6cc222008d95..5d5e8c3fd5ad 100644
--- a/net/tls/tls_strp.c
+++ b/net/tls/tls_strp.c
@@ -526,6 +526,9 @@ static int tls_strp_read_sock(struct tls_strparser *strp)
 
 	tls_strp_load_anchor_with_queue(strp, inq);
 	if (!strp->stm.full_len) {
+		/* Save the record start before copy mode advances copied_seq. */
+		strp->seq = tcp_sk(strp->sk)->copied_seq;
+
 		if (inq < TLS_HEADER_SIZE)
 			return tls_strp_read_copy(strp, true);
 		if (!tls_strp_check_queue_ok(strp, TLS_HEADER_SIZE))
-- 
2.50.1

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

[Rishikesh Jethwani]

tls_device_core_ctrl_rx_resync() asks the device to resync at the next
record, passing rec_seq + 1 together with tcp_sk(sk)->copied_seq as that
record's TCP sequence. With the original strparser the current record
had already been read out of the TCP receive queue, so copied_seq was
the start of the next record.

The TLS-specific strparser leaves the record queued in its normal mode
until tls_strp_msg_consume(), so copied_seq is still the start of the
current record. The resync branch is taken when tcp_inq() does not
exceed the record length, i.e. typically when the current record is the
only data queued, and the device is told that record rec_seq + 1 begins
where record rec_seq begins. This can prevent hardware resynchronization
and keep subsequent records on the software decryption path. Further
resync attempts are made with backoff.

In copy mode copied_seq is already past the record, so the sequence is
correct there; the only exception is a FIN consumed along with the
record, after which no further record can follow.

Use the record start from tls_strp_msg_seq() plus the record length
instead. That is the start of the next record in both modes, so the
reported sequence no longer depends on which mode the strparser is in.

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: 84c61fe1a75b ("tls: rx: do not use the standard strparser")
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 net/tls/tls_device.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index fbfe6ec3df04..a6c4d0cc54c7 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -839,6 +839,7 @@ static void tls_device_core_ctrl_rx_resync(struct tls_context *tls_ctx,
 					   struct tls_offload_context_rx *ctx,
 					   struct sock *sk, struct sk_buff *skb)
 {
+	struct tls_sw_context_rx *sw_ctx = tls_sw_ctx_rx(tls_ctx);
 	struct strp_msg *rxm;
 
 	/* device will request resyncs by itself based on stream scan */
@@ -877,8 +878,9 @@ 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, tcp_sk(sk)->copied_seq,
-				     rcd_sn);
+		tls_device_resync_rx(tls_ctx, sk,
+				     tls_strp_msg_seq(sw_ctx) +
+				     rxm->full_len, rcd_sn);
 	}
 }
 
-- 
2.50.1

Re: [PATCH net v2 2/4] tls: device: fix next-record sequence in core-driven RX resync

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In tls_device_core_ctrl_rx_resync() (net/tls/tls_device.c),
  this patch makes the next-record sequence sent to the device…

Pre-existing issues:
- [Medium] The other CORE_NEXT_HINT resync path, tls_rx_msg_size() ->
  tls_device_rx_resync_new_rec() (net/tls/tls_sw.c:2234-2235 at this…

--

tls: device: fix next-record sequence in core-driven RX resync

tls_device_core_ctrl_rx_resync() used to pass tcp_sk(sk)->copied_seq to
tls_device_resync_rx() as the TCP sequence of the next record. This patch
passes tls_strp_msg_seq() + rxm->full_len instead. The commit message
says the result is the start of the next record in both strparser modes.

> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index fbfe6ec3df048..a6c4d0cc54c73 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c

[ ... ]

> @@ -877,8 +878,9 @@ 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, tcp_sk(sk)->copied_seq,
> -				     rcd_sn);
> +		tls_device_resync_rx(tls_ctx, sk,
> +				     tls_strp_msg_seq(sw_ctx) +
> +				     rxm->full_len, rcd_sn);

[Severity: Medium]
The sequence passed here no longer depends on the strparser mode. Does the
check that picks this branch still depend on it?

Earlier in tls_device_core_ctrl_rx_resync():

	/* head of next rec is already in, parser will sync for us */
	if (tcp_inq(sk) > rxm->full_len) {
		trace_tls_device_rx_resync_nh_schedule(sk);
		ctx->resync_nh_do_now = 1;
	} else {

tcp_inq() counts from copied_seq. In copy mode, tls_strp_read_copy() has
already moved copied_seq past the current record, so tcp_inq() counts only
the bytes of the next record.

Suppose between 1 and rxm->full_len bytes of the next record are already
queued. The else branch runs, and tls_dev_resync() is called for a TCP
position the device has already passed. resync_nh_do_now is never set, so
tls_device_rx_resync_new_rec() also returns early:

	case TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT:
		if (likely(!rx_ctx->resync_nh_do_now))
			return;

Would this waste the resync attempt and keep the stream on software
decryption until the backed-off retry?

This tcp_inq() check was already there before this patch. The later commit
"tls: device: fix next-record queued checks in core-driven RX resync" in
this series appears to fix it. That commit replaces the check with
after(READ_ONCE(tcp_sk(sk)->rcv_nxt), next_seq).

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. The
commit message says the reported sequence "no longer depends on which mode
the strparser is in". Does that also hold for the other CORE_NEXT_HINT
path?

tls_rx_msg_size() in net/tls/tls_sw.c still computes the record start from
the anchor skb:

	tls_device_rx_resync_new_rec(strp->sk, data_len + TLS_HEADER_SIZE,
				     TCP_SKB_CB(skb)->seq + strp->stm.offset);

In copy mode, tls_strp_read_copy() resets stm.offset to 0 but keeps the
anchor's old cb. The value above can then be lower than the real record
start, while tls_device_core_ctrl_rx_resync() now uses strp->seq.

When tls_device_core_ctrl_rx_resync() sets resync_nh_do_now, it hands the
resync to this path. tls_device_rx_resync_new_rec() also still assumes
normal mode:

		/* head of next rec is already in, note that the sock_inq will
		 * include the currently parsed message when called from parser
		 */
		sock_data = tcp_inq(sk);
		if (sock_data > rcd_len) {

In copy mode, could the device then get the wrong next-record boundary for
CORE_NEXT_HINT? Could the request sequence also fail to match for
DRIVER_REQ and DRIVER_REQ_ASYNC?

Two later commits in this series look like they fix this:

  "tls: device: fix record start reported to RX resync in copy mode"
  changes the argument to strp->seq.

  "tls: device: fix next-record queued checks in core-driven RX resync"
  changes the tcp_inq() check in tls_device_rx_resync_new_rec() to
  READ_ONCE(tcp_sk(sk)->rcv_nxt) - seq.

>  	}
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022221732.1397620-1-rjethwani%40purestorage.com

Re: [PATCH net v2 2/4] tls: device: fix next-record sequence in core-driven RX resync

[Rishikesh Jethwani]

On Mon, Oct 26, 2026 at 3:48 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] In tls_device_core_ctrl_rx_resync() (net/tls/tls_device.c),
>   this patch makes the next-record sequence sent to the device…
>
> Pre-existing issues:
> - [Medium] The other CORE_NEXT_HINT resync path, tls_rx_msg_size() ->
>   tls_device_rx_resync_new_rec() (net/tls/tls_sw.c:2234-2235 at this…
>
> --
>
> tls: device: fix next-record sequence in core-driven RX resync
>
> tls_device_core_ctrl_rx_resync() used to pass tcp_sk(sk)->copied_seq to
> tls_device_resync_rx() as the TCP sequence of the next record. This patch
> passes tls_strp_msg_seq() + rxm->full_len instead. The commit message
> says the result is the start of the next record in both strparser modes.
>
> > diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> > index fbfe6ec3df048..a6c4d0cc54c73 100644
> > --- a/net/tls/tls_device.c
> > +++ b/net/tls/tls_device.c
>
> [ ... ]
>
> > @@ -877,8 +878,9 @@ 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, tcp_sk(sk)->copied_seq,
> > -                                  rcd_sn);
> > +             tls_device_resync_rx(tls_ctx, sk,
> > +                                  tls_strp_msg_seq(sw_ctx) +
> > +                                  rxm->full_len, rcd_sn);
>
> [Severity: Medium]
> The sequence passed here no longer depends on the strparser mode. Does the
> check that picks this branch still depend on it?
>
> Earlier in tls_device_core_ctrl_rx_resync():
>
>         /* head of next rec is already in, parser will sync for us */
>         if (tcp_inq(sk) > rxm->full_len) {
>                 trace_tls_device_rx_resync_nh_schedule(sk);
>                 ctx->resync_nh_do_now = 1;
>         } else {
>
> tcp_inq() counts from copied_seq. In copy mode, tls_strp_read_copy() has
> already moved copied_seq past the current record, so tcp_inq() counts only
> the bytes of the next record.
>
> Suppose between 1 and rxm->full_len bytes of the next record are already
> queued. The else branch runs, and tls_dev_resync() is called for a TCP
> position the device has already passed. resync_nh_do_now is never set, so
> tls_device_rx_resync_new_rec() also returns early:
>
>         case TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT:
>                 if (likely(!rx_ctx->resync_nh_do_now))
>                         return;
>
> Would this waste the resync attempt and keep the stream on software
> decryption until the backed-off retry?

Right, but that's pre-existing and not changed by this patch. In copy
mode copied_seq was already the next record's start, so the sequence
passed there is the same before and after; this patch only fixes the
normal mode, where the tcp_inq() check was correct but the sequence was
not.

The tcp_inq() check itself is wrong in copy mode independently of this
change (and since f953d33ba122 with the original strparser), which is
why it's a separate patch with its own Fixes: tag: patch 4/4
("tls: device: fix next-record queued checks in
core-driven RX resync") replaces it with after(rcv_nxt, next_seq).

> This tcp_inq() check was already there before this patch. The later commit
> "tls: device: fix next-record queued checks in core-driven RX resync" in
> this series appears to fix it. That commit replaces the check with
> after(READ_ONCE(tcp_sk(sk)->rcv_nxt), next_seq).
>
> [Severity: Medium]
> This is a pre-existing issue and was not introduced by this patch. The
> commit message says the reported sequence "no longer depends on which mode
> the strparser is in". Does that also hold for the other CORE_NEXT_HINT
> path?
>
> tls_rx_msg_size() in net/tls/tls_sw.c still computes the record start from
> the anchor skb:
>
>         tls_device_rx_resync_new_rec(strp->sk, data_len + TLS_HEADER_SIZE,
>                                      TCP_SKB_CB(skb)->seq + strp->stm.offset);
>
> In copy mode, tls_strp_read_copy() resets stm.offset to 0 but keeps the
> anchor's old cb. The value above can then be lower than the real record
> start, while tls_device_core_ctrl_rx_resync() now uses strp->seq.
>
> When tls_device_core_ctrl_rx_resync() sets resync_nh_do_now, it hands the
> resync to this path. tls_device_rx_resync_new_rec() also still assumes
> normal mode:
>
>                 /* head of next rec is already in, note that the sock_inq will
>                  * include the currently parsed message when called from parser
>                  */
>                 sock_data = tcp_inq(sk);
>                 if (sock_data > rcd_len) {
>
> In copy mode, could the device then get the wrong next-record boundary for
> CORE_NEXT_HINT? Could the request sequence also fail to match for
> DRIVER_REQ and DRIVER_REQ_ASYNC?
>
> Two later commits in this series look like they fix this:
>
>   "tls: device: fix record start reported to RX resync in copy mode"
>   changes the argument to strp->seq.
>
>   "tls: device: fix next-record queued checks in core-driven RX resync"
>   changes the tcp_inq() check in tls_device_rx_resync_new_rec() to
>   READ_ONCE(tcp_sk(sk)->rcv_nxt) - seq.
>
> >       }
> >  }

The tls_rx_msg_size() path is pre-existing and handled separately:
patch 3/4 reports strp->seq instead of the anchor cb seq (covering
DRIVER_REQ / DRIVER_REQ_ASYNC as well as CORE_NEXT_HINT),

> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022221732.1397620-1-rjethwani%40purestorage.com

[PATCH net v2 3/4] tls: device: fix record start reported to RX resync in copy mode

[Rishikesh Jethwani]

tls_rx_msg_size() tells the device layer where each new record starts,
via tls_device_rx_resync_new_rec(), as TCP_SKB_CB(skb)->seq +
strp->stm.offset. skb is the strparser anchor, whose cb is copied from
the first queued skb by tls_strp_load_anchor_with_queue(), and
stm.offset is where the record starts within that skb.

When the header cannot be parsed in place, tls_strp_read_copy() moves
the strparser to copy mode and resets stm.offset to 0, but the anchor
keeps the first skb's cb. The header is then parsed from the copied
data, and the reported start is the first byte of that skb rather than
of the record. If the record does not begin at an skb boundary, the
sequence is too low by the record's offset into the skb.

Every resync type compares or advances that sequence. With
TLS_OFFLOAD_SYNC_TYPE_DRIVER_REQ and DRIVER_REQ_ASYNC the device's
request no longer matches and the resync is missed until a later
record. With TLS_OFFLOAD_SYNC_TYPE_CORE_NEXT_HINT the device is given a
next-record boundary that is off by the same amount.

The header is parsed in copy mode when fewer bytes than the TLS header
plus explicit IV, where applicable, are queued under memory pressure or
with a collapsed receive window, or when the header straddles skbs that
overlap or differ in decrypted status. The latter is most likely right
around a loss of device sync, when resync is needed. Commit 9466ef3ec972
("tls: fix RX desync on overlapping skbs") added that queue-consistency
check before the header is parsed; before it only a short queue led
here.

The strparser now records the start of each record before it can enter
copy mode, so report strp->seq instead.

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

Fixes: 84c61fe1a75b ("tls: rx: do not use the standard strparser")
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 net/tls/tls_sw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index d1ad31986cf2..5db4a6897b29 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2232,7 +2232,7 @@ int tls_rx_msg_size(struct tls_strparser *strp, struct sk_buff *skb)
 	}
 
 	tls_device_rx_resync_new_rec(strp->sk, data_len + TLS_HEADER_SIZE,
-				     TCP_SKB_CB(skb)->seq + strp->stm.offset);
+				     strp->seq);
 	return data_len + TLS_HEADER_SIZE;
 
 read_failure:
-- 
2.50.1

[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@purestorage.com>
---
 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