[PATCH] tcp: validate old ACKs before fast path data processing

[PATCH] tcp: validate old ACKs before fast path data processing

[Inbal Schussheim]

For incoming TCP segments processed in the fast path,
Linux does not enforce the RFC5961 requirement:

   The ACK value is considered acceptable only if
   it is in the range of ((SND.UNA - MAX.SND.WND) <= SEG.ACK <=
   SND.NXT).  All incoming segments whose ACK value doesn't satisfy the
   above condition MUST be discarded and an ACK sent back.

Meaning the ack of incoming segments is no earlier than a window back
from the first unacknowledged sent byte.

Later work showed that the condition (SND.UNA - MAX.SND.WND) <= SEG.ACK
can be further tightened, eliminating some demonstrated TCP data
injection attacks, resulting in CVE-2023-52881 assigned by Linux and
the 2023 patch:
Commit 3d501dd326fb1c7 ("tcp: do not accept ACK of bytes we never sent")
that rejects ACKs for bytes so far back that were never sent.

Link: https://www.cve.org/CVERecord?id=CVE-2023-52881

Both RFC5961 and the later patch were only applied to the slow path,
leaving the fast path vulnerable and noncompliant with RFC5961.

Enforce a validation test for the SEG.ACK in the fast path, before the data
is processed. Failure to pass the validation will result in a challenge ACK
and the packet will be discarded in compliance with RFC5961.

Some details:
RFC5961 (and the 2023 patch) is enforced in tcp_ack()
(./net/ipv4/tcp_input.c).
Incoming segments to a socket in ESTABLISHED state are processed in
tcp_rcv_established() (./net/ipv4/tcp_input.c).
Consider a packet that violates RFC5961 (meaning the SEG.ACK is too early).
In the slow path (starting at the label "slow_path"), tcp_ack() is invoked,
well before processing the segment data.
A challenge ACK is sent there, tcp_ack() returns
-SKB_DROP_REASON_TCP_TOO_OLD_ACK,
and slow path discards the segment as expected.
In the fast path, tcp_ack() is also called,
but only after the data from the segment is processed.
Furthermore, the return value from tcp_ack() is not checked.
De-facto, the data from the segment is accepted
(and an ACK is generated), even though the segment violates RFC5961.

The following packetdrill script shows the issue at hand.
Linux (as a server) accepts data segment processed in the fast path with
an ack that is far too low.

// BASED ON PACKETDRILL SCRIPT FROM:
// Commit 3d501dd326fb1c7 ("tcp: do not accept ACK of bytes we never sent")
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1024) = 0

// ---------------- Handshake ------------------- //
+0 < S 0:0(0) win 65535
+0 > S. 0:0(0) ack 1 <...>
+0 < . 1:1(0) ack 1 win 65535
+0 accept(3, ..., ...) = 4

// Data must be first sent/received on the socket
// so that memory is allocated (sk_forward_alloc should be > 0)
// and later data will be proccessed in the fast path
+0 < P. 1:501(500) ack 1 win 65535 //valid packet forcing memory allocation
+0 > . 1:1(0) ack 501

// incoming segment, ack way in the past... (101 + 2^32 - 1500000000)
// Oops, unpatched kernels happily accept this packet
+0 < P. 501:1501(1000) ack 2794967397 win 65535

// On unpatched kernels, this ACK will match,
// showing that the segment is accepted
+0 > . 1:1(0) ack 1501

Reported-by: Amit Klein <amit.klein@mail.huji.ac.il>
Reported-by: Tamir Shahar <tamir.shahar1@mail.huji.ac.il>
Reported-by: Inbal Schussheim <inbal.lipshtat@mail.huji.ac.il>
Signed-off-by: Inbal Schussheim <inbal.lipshtat@mail.huji.ac.il>
---
 net/ipv4/tcp_input.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index daff93d51342..2474871e80ec 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4272,6 +4272,17 @@ static void tcp_rack_update_reo_wnd(struct sock *sk, struct rate_sample *rs)
 	}
 }
 
+/* Validates that the ACK is older than the acceptable historical ACK window*/
+static inline bool tcp_ack_too_old(const struct tcp_sock *tp, u32 ack,
+				   u32 snd_una)
+{
+	u32 max_window;
+
+	max_window = min_t(u64, tp->max_window, tp->bytes_acked);
+
+	return before(ack, snd_una - max_window);
+}
+
 /* This routine deals with incoming acks, but not outgoing ones. */
 static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 {
@@ -4303,12 +4314,8 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 	 * then we can probably ignore it.
 	 */
 	if (before(ack, prior_snd_una)) {
-		u32 max_window;
-
-		/* do not accept ACK for bytes we never sent. */
-		max_window = min_t(u64, tp->max_window, tp->bytes_acked);
 		/* RFC 5961 5.2 [Blind Data Injection Attack].[Mitigation] */
-		if (before(ack, prior_snd_una - max_window)) {
+		if (tcp_ack_too_old(tp, ack, prior_snd_una)) {
 			if (!(flag & FLAG_NO_CHALLENGE_ACK))
 				tcp_send_challenge_ack(sk, false);
 			return -SKB_DROP_REASON_TCP_TOO_OLD_ACK;
@@ -6614,6 +6621,15 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
 			if ((int)skb->truesize > sk->sk_forward_alloc)
 				goto step5;
 
+			if (unlikely(before(TCP_SKB_CB(skb)->ack_seq, tp->snd_una))) {
+				if (tcp_ack_too_old(tp, TCP_SKB_CB(skb)->ack_seq,
+						    tp->snd_una)) {
+					tcp_send_challenge_ack(sk, false);
+					reason = SKB_DROP_REASON_TCP_TOO_OLD_ACK;
+					goto discard;
+				}
+			}
+
 			/* Predicted packet is in window by definition.
 			 * seq == rcv_nxt and rcv_wup <= rcv_nxt.
 			 * Hence, check seq<=rcv_wup reduces to:
-- 
2.43.0

Re: [PATCH] tcp: validate old ACKs before fast path data processing

[Eric Dumazet]

On Sun, Sep 6, 2026 at 2:32 PM Inbal Schussheim
<inbal.lipshtat@mail.huji.ac.il> wrote:
>
> For incoming TCP segments processed in the fast path,
> Linux does not enforce the RFC5961 requirement:
>
>    The ACK value is considered acceptable only if
>    it is in the range of ((SND.UNA - MAX.SND.WND) <= SEG.ACK <=
>    SND.NXT).  All incoming segments whose ACK value doesn't satisfy the
>    above condition MUST be discarded and an ACK sent back.
>
> Meaning the ack of incoming segments is no earlier than a window back
> from the first unacknowledged sent byte.
>
> Later work showed that the condition (SND.UNA - MAX.SND.WND) <= SEG.ACK
> can be further tightened, eliminating some demonstrated TCP data
> injection attacks, resulting in CVE-2023-52881 assigned by Linux and
> the 2023 patch:
> Commit 3d501dd326fb1c7 ("tcp: do not accept ACK of bytes we never sent")
> that rejects ACKs for bytes so far back that were never sent.
>
> Link: https://www.cve.org/CVERecord?id=CVE-2023-52881
>
> Both RFC5961 and the later patch were only applied to the slow path,
> leaving the fast path vulnerable and noncompliant with RFC5961.
>
> Enforce a validation test for the SEG.ACK in the fast path, before the data
> is processed. Failure to pass the validation will result in a challenge ACK
> and the packet will be discarded in compliance with RFC5961.
>
> Some details:
> RFC5961 (and the 2023 patch) is enforced in tcp_ack()
> (./net/ipv4/tcp_input.c).
> Incoming segments to a socket in ESTABLISHED state are processed in
> tcp_rcv_established() (./net/ipv4/tcp_input.c).
> Consider a packet that violates RFC5961 (meaning the SEG.ACK is too early).
> In the slow path (starting at the label "slow_path"), tcp_ack() is invoked,
> well before processing the segment data.
> A challenge ACK is sent there, tcp_ack() returns
> -SKB_DROP_REASON_TCP_TOO_OLD_ACK,
> and slow path discards the segment as expected.
> In the fast path, tcp_ack() is also called,
> but only after the data from the segment is processed.
> Furthermore, the return value from tcp_ack() is not checked.
> De-facto, the data from the segment is accepted
> (and an ACK is generated), even though the segment violates RFC5961.
>
> The following packetdrill script shows the issue at hand.
> Linux (as a server) accepts data segment processed in the fast path with
> an ack that is far too low.
>
> // BASED ON PACKETDRILL SCRIPT FROM:
> // Commit 3d501dd326fb1c7 ("tcp: do not accept ACK of bytes we never sent")
> 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> +0 bind(3, ..., ...) = 0
> +0 listen(3, 1024) = 0
>
> // ---------------- Handshake ------------------- //
> +0 < S 0:0(0) win 65535
> +0 > S. 0:0(0) ack 1 <...>
> +0 < . 1:1(0) ack 1 win 65535
> +0 accept(3, ..., ...) = 4
>
> // Data must be first sent/received on the socket
> // so that memory is allocated (sk_forward_alloc should be > 0)
> // and later data will be proccessed in the fast path
> +0 < P. 1:501(500) ack 1 win 65535 //valid packet forcing memory allocation
> +0 > . 1:1(0) ack 501
>
> // incoming segment, ack way in the past... (101 + 2^32 - 1500000000)
> // Oops, unpatched kernels happily accept this packet
> +0 < P. 501:1501(1000) ack 2794967397 win 65535
>
> // On unpatched kernels, this ACK will match,
> // showing that the segment is accepted
> +0 > . 1:1(0) ack 1501
>
> Reported-by: Amit Klein <amit.klein@mail.huji.ac.il>
> Reported-by: Tamir Shahar <tamir.shahar1@mail.huji.ac.il>
> Reported-by: Inbal Schussheim <inbal.lipshtat@mail.huji.ac.il>
> Signed-off-by: Inbal Schussheim <inbal.lipshtat@mail.huji.ac.il>
> ---
>  net/ipv4/tcp_input.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index daff93d51342..2474871e80ec 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4272,6 +4272,17 @@ static void tcp_rack_update_reo_wnd(struct sock *sk, struct rate_sample *rs)
>         }
>  }
>
> +/* Validates that the ACK is older than the acceptable historical ACK window*/
> +static inline bool tcp_ack_too_old(const struct tcp_sock *tp, u32 ack,
> +                                  u32 snd_una)
> +{
> +       u32 max_window;
> +
> +       max_window = min_t(u64, tp->max_window, tp->bytes_acked);
> +
> +       return before(ack, snd_una - max_window);
> +}
> +
>  /* This routine deals with incoming acks, but not outgoing ones. */
>  static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
>  {
> @@ -4303,12 +4314,8 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
>          * then we can probably ignore it.
>          */
>         if (before(ack, prior_snd_una)) {
> -               u32 max_window;
> -
> -               /* do not accept ACK for bytes we never sent. */
> -               max_window = min_t(u64, tp->max_window, tp->bytes_acked);
>                 /* RFC 5961 5.2 [Blind Data Injection Attack].[Mitigation] */
> -               if (before(ack, prior_snd_una - max_window)) {
> +               if (tcp_ack_too_old(tp, ack, prior_snd_una)) {
>                         if (!(flag & FLAG_NO_CHALLENGE_ACK))
>                                 tcp_send_challenge_ack(sk, false);
>                         return -SKB_DROP_REASON_TCP_TOO_OLD_ACK;
> @@ -6614,6 +6621,15 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
>                         if ((int)skb->truesize > sk->sk_forward_alloc)
>                                 goto step5;
>
> +                       if (unlikely(before(TCP_SKB_CB(skb)->ack_seq, tp->snd_una))) {
> +                               if (tcp_ack_too_old(tp, TCP_SKB_CB(skb)->ack_seq,
> +                                                   tp->snd_una)) {
> +                                       tcp_send_challenge_ack(sk, false);
> +                                       reason = SKB_DROP_REASON_TCP_TOO_OLD_ACK;
> +                                       goto discard;
> +                               }
> +                       }
> +


Hi Inbal

I would suggest :

1) reduce the changelog to the problem at hand, not the old ones. You
do not have to tell a story.

2) Add a selftests (in a separate patch) instead of including it in a changelog.

3) Make sure to handle pure ACK as well so that we avoid another CVE
in ~3 years, presumably from your team.

Segments with old ACKs (ack_seq < tp->snd_una) do not belong in the
fast path in the first place,
whether they carry payload or are pure ACKs.
In the pure ACK fast path, an ancient ACK would also bypass drop
accounting and update
timestamps/state before tcp_ack() returns an error.

Instead of duplicating the challenge ACK and drop logic in
tcp_rcv_established(),
we can simply tighten the header prediction check.

The so-called 'slow-path' is already taking care of this.

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0f60a1dbf927468741f6f163e8d1b07687452c39..89f9f3913b52ff468f3bd4b098cb5d0f3c45a9d2
100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6539,7 +6539,7 @@ void tcp_rcv_established(struct sock *sk, struct
sk_buff *skb)

        if ((tcp_flag_word(th) & TCP_HP_BITS) == tp->pred_flags &&
            TCP_SKB_CB(skb)->seq == tp->rcv_nxt &&
-           !after(TCP_SKB_CB(skb)->ack_seq, tp->snd_nxt)) {
+           between(TCP_SKB_CB(skb)->ack_seq, tp->snd_una, tp->snd_nxt)) {
                int tcp_header_len = tp->tcp_header_len;
                s32 delta = 0;
                int flag = 0;


Thanks!

pw-bot: cr

Re: [PATCH] tcp: validate old ACKs before fast path data processing

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 3

- [Medium] Incomplete RFC5961 enforcement: the new ACK-acceptability
  check is only added to the data-carrying fast-path branch of…
- [Low] The patch fixes a remotely triggerable, security-relevant defect
  (the ESTABLISHED fast path accepting payload from a segment whose…
- [Low] No in-tree test covers the newly added enforcement.
- [Low] The rationale comment for the tp->bytes_acked term is lost.

--

tcp: validate old ACKs before fast path data processing

This adds an RFC5961 5.2 SEG.ACK acceptability test to the ESTABLISHED
fast path in tcp_rcv_established(), before the segment payload is queued,
sending a challenge ACK and dropping the segment when the ACK is too old.
The existing test in tcp_ack() is factored into a new tcp_ack_too_old()
helper shared by both call sites.

Link: https://www.cve.org/CVERecord?id=CVE-2023-52881

A few questions below.

[Severity: Low]
The changelog names both CVE-2023-52881 and commit 3d501dd326fb1c7 ("tcp:
do not accept ACK of bytes we never sent"), but the tag block has no
Fixes: line and no stable routing indication.

Should this carry a Fixes: tag so stable selection and CVE correlation
have something to key on?  The candidates implied by the changelog are the
original RFC5961 5.2 mitigation (which only touched tcp_ack()) and/or
3d501dd326fb1c7.

[Severity: Low]
The changelog carries a working reproducer, but nothing in-tree exercises
the new hunk.  The existing script
tools/testing/selftests/net/packetdrill/tcp_rfc5961_ack-out-of-window.pkt
injects only zero-length probes for both edges:

  +0 < . 1:1(0) ack 2 win 1000
  ...
  +0 < . 1:1(0) ack 1000 win 1000

Those take the len == tcp_header_len pure-ACK branch (or the slow path),
so that test passes identically with and without this patch.

Could the packetdrill script from the changelog (500-byte priming segment
followed by the 1000-byte segment with ack 2794967397) be added next to
the existing rfc5961 case, so a later refactor of the fast path cannot
silently drop the check?

> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 0f60a1dbf9274..6ebf72cd9a755 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4272,6 +4272,17 @@ static void tcp_rack_update_reo_wnd(struct sock *sk, struct rate_sample *rs)
>  	}
>  }
>  
> +/* Validates that the ACK is older than the acceptable historical ACK window*/
> +static inline bool tcp_ack_too_old(const struct tcp_sock *tp, u32 ack,
> +				   u32 snd_una)
> +{
> +	u32 max_window;
> +
> +	max_window = min_t(u64, tp->max_window, tp->bytes_acked);
> +
> +	return before(ack, snd_una - max_window);
> +}
> +

[Severity: Low]
The helper comment describes a plain MAX.SND.WND window, but the code also
bounds by tp->bytes_acked, which is strictly tighter than RFC5961.  The
comment explaining that term is deleted in the hunk below and not
reproduced here:

-		/* do not accept ACK for bytes we never sent. */
-		max_window = min_t(u64, tp->max_window, tp->bytes_acked);

Since the helper now has two call sites, could the "do not accept ACK for
bytes we never sent" rationale be moved into tcp_ack_too_old() alongside
the RFC5961 note?

>  /* This routine deals with incoming acks, but not outgoing ones. */
>  static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
>  {
> @@ -4303,12 +4314,8 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
>  	 * then we can probably ignore it.
>  	 */
>  	if (before(ack, prior_snd_una)) {
> -		u32 max_window;
> -
> -		/* do not accept ACK for bytes we never sent. */
> -		max_window = min_t(u64, tp->max_window, tp->bytes_acked);
>  		/* RFC 5961 5.2 [Blind Data Injection Attack].[Mitigation] */
> -		if (before(ack, prior_snd_una - max_window)) {
> +		if (tcp_ack_too_old(tp, ack, prior_snd_una)) {
>  			if (!(flag & FLAG_NO_CHALLENGE_ACK))
>  				tcp_send_challenge_ack(sk, false);
>  			return -SKB_DROP_REASON_TCP_TOO_OLD_ACK;

[ ... ]

> @@ -6614,6 +6621,15 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
>  			if ((int)skb->truesize > sk->sk_forward_alloc)
>  				goto step5;
>  
> +			if (unlikely(before(TCP_SKB_CB(skb)->ack_seq, tp->snd_una))) {
> +				if (tcp_ack_too_old(tp, TCP_SKB_CB(skb)->ack_seq,
> +						    tp->snd_una)) {
> +					tcp_send_challenge_ack(sk, false);
> +					reason = SKB_DROP_REASON_TCP_TOO_OLD_ACK;
> +					goto discard;
> +				}
> +			}
> +

[Severity: Medium]
The new check sits inside the else branch of if (len <= tcp_header_len),
so it only covers data-bearing segments.  Was the sibling pure-ACK branch
intentionally left out?

That branch still does this, in this order, in tcp_rcv_established():

	if (len <= tcp_header_len) {
		if (len == tcp_header_len) {
			if (tcp_header_len ==
			    (sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED) &&
			    tp->rcv_nxt == tp->rcv_wup)
				flag |= __tcp_replace_ts_recent(tp, delta);

			tcp_ecn_received_counters(sk, skb, 0);
			...
			tcp_ack(sk, skb, flag);
			__kfree_skb(skb);
			tcp_data_snd_check(sk);
			tp->rcv_rtt_last_tsecr = tp->rx_opt.rcv_tsecr;
			return;

__tcp_replace_ts_recent() stores unconditionally:

net/ipv4/tcp_input.c:__tcp_replace_ts_recent() {
	tcp_store_ts_recent(tp);
	return tstamp_delta > 0 ? FLAG_TS_PROGRESS : 0;
}

So for a bare ACK matching tp->pred_flags with seq == tp->rcv_nxt,
!after(ack_seq, tp->snd_nxt), and before(ack_seq, tp->snd_una -
min(max_window, bytes_acked)), tp->rx_opt.ts_recent is advanced to the
segment's TSval, the ECN receive counters are bumped, and
tp->rcv_rtt_last_tsecr is taken from that segment.

tcp_ack() then reaches the same tcp_ack_too_old() test, sends the
challenge ACK and returns -SKB_DROP_REASON_TCP_TOO_OLD_ACK, but the
return value is still discarded here and nothing is rolled back or
accounted as a drop.

This is the same condition the changelog describes:

    "In the fast path, tcp_ack() is also called,
    but only after the data from the segment is processed.
    Furthermore, the return value from tcp_ack() is not checked."

The slow path does not have this ordering problem, since tcp_ack() runs
the too-old test before the FLAG_UPDATE_TS_RECENT handling, and step5
checks (int)reason < 0.

Would hoisting the new check up to just after the header prediction test,
before if (len <= tcp_header_len), cover both branches with a single call
site instead of leaving the pure-ACK branch as is?

>  			/* Predicted packet is in window by definition.
>  			 * seq == rcv_nxt and rcv_wup <= rcv_nxt.
>  			 * Hence, check seq<=rcv_wup reduces to:

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260906123151.1391349-1-inbal.lipshtat%40mail.huji.ac.il