From: <hotaka.miyazaki@cybertrust.co.jp>
To: <netdev@vger.kernel.org>
Subject: Potential violation of RFC793 3.9, missing challenge ACK
Date: Thu, 16 May 2024 16:12:30 +0900 [thread overview]
Message-ID: <TYAPR01MB64091EA3717FC588B4ACF045C4ED2@TYAPR01MB6409.jpnprd01.prod.outlook.com> (raw)
Hello.
I have a question about the following part of the tcp_ack function in net/ipv4/tcp_input.c.
```
/* If the ack includes data we haven't sent yet, discard
* this segment (RFC793 Section 3.9).
*/
if (after(ack, tp->snd_nxt))
return -SKB_DROP_REASON_TCP_ACK_UNSENT_DATA;
```
I think that this part violates RFC793 3.9 (and the equivalent part in RFC9293 (3.10.7.4)).
According to the RFC, “If the ACK acks something not yet sent (SEG.ACK > SND.NXT) then send an ACK, drop the segment, and return“ [1].
However, the code appears not to ack before discarding a segment.
Does anyone know whether the violation is intended and the reason if it is intended?
If that is not intended, I would like to fix it. What do you think of that?
I think that the following patch would fix the violation.
The fixed code is similar to the SKB_DROP_REASON_TCP_TOO_OLD_ACK error case.
```
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 5d874817a78d..a0ed2f671912 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3877,11 +3877,14 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
goto old_ack;
}
- /* If the ack includes data we haven't sent yet, discard
- * this segment (RFC793 Section 3.9).
+ /* If the ack includes data we haven't sent yet,
+ * send an ACK and discard this segment (RFC793 Section 3.9).
*/
- if (after(ack, tp->snd_nxt))
+ if (after(ack, tp->snd_nxt)) {
+ if (!(flag & FLAG_NO_CHALLENGE_ACK))
+ tcp_send_challenge_ack(sk);
return -SKB_DROP_REASON_TCP_ACK_UNSENT_DATA;
+ }
if (after(ack, prior_snd_una)) {
flag |= FLAG_SND_UNA_ADVANCED;
```
Thanks for reading.
---
[1]: RFC793 3.9 on page 72 (fifth check)
```
ESTABLISHED STATE
If SND.UNA < SEG.ACK =< SND.NXT then, set SND.UNA <- SEG.ACK.
Any segments on the retransmission queue which are thereby
entirely acknowledged are removed. Users should receive
positive acknowledgments for buffers which have been SENT and
fully acknowledged (i.e., SEND buffer should be returned with
"ok" response). If the ACK is a duplicate
(SEG.ACK < SND.UNA), it can be ignored. If the ACK acks
something not yet sent (SEG.ACK > SND.NXT) then send an ACK,
drop the segment, and return.
```
next reply other threads:[~2024-05-16 7:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-16 7:12 hotaka.miyazaki [this message]
2024-05-21 9:47 ` Potential violation of RFC793 3.9, missing challenge ACK Paolo Abeni
2024-05-21 10:12 ` Eric Dumazet
2024-05-21 10:33 ` Paolo Abeni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=TYAPR01MB64091EA3717FC588B4ACF045C4ED2@TYAPR01MB6409.jpnprd01.prod.outlook.com \
--to=hotaka.miyazaki@cybertrust.co.jp \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).