tcp_sequence() rejects an incoming segment when end_seq is before rcv_wup. Since end_seq is one past the last sequence number consumed by the segment, this misses the boundary case where end_seq is equal to rcv_wup. A segment that consumes sequence space and has end_seq equal to rcv_wup is therefore allowed to reach later processing, including ACK handling, even though it should be rejected as a completely old segment. Reject this boundary case while retaining the existing behavior for segments that consume no sequence space. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Reported-by: Michael Cohen Reported-by: Tamir Shahar Reported-by: Amit Klein Signed-off-by: Michael Cohen --- Packetdrill reproducer: // Off by one bug in tcp_sequence() // the negative test before(end_seq, tp->rcv_wup) has off by one error, since end_seq is SEG.SEQ+SEG.LEN, // whereas the RFCs require SEG.SEQ+SEG.LEN-1 (their positive test is RCV.NXT =< SEG.SEQ+SEG.LEN-1) 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 +0 < S 0:0(0) win 12345 +0 > S. 0:0(0) ack 1 <...> +0 < . 1:1(0) ack 1 win 12345 +0 accept(3, ..., ...) = 4 // This is not mandatory for the phenomenon, we just do this to increment SND.NXT (set SND.NXT=101, retain SND.UNA=1) so we can show // later that the problematic segment is actually accepted (via the tcpi_accepted_bytes count). +0 send(4, ..., 100, 0) = 100 +0 > P. 1:101(100) ack 1 +0 < P. 1:1001(1000) ack 1 win 12345 +0 > . 101:101(0) ack 1001 // Now RCV.NXT=1001, so according to the RFC, a subsequent 1:1001 should be discarded. // But in Linux, 1:1001 is accepted(!). // Note that bytes_acked is incremented to the packet's ack number, which shows the packet is accepted. +0 < P. 1:1001(1000) ack 23 win 12345 // +0 < P. 1:1000(999) ack 23 win 12345 // if you use this instead, you get an assertion error, as expected. // this assert will succeed in the presence of the bug, but per the RFCs, it should fail because the packet should have been discarded +0 %{ assert(tcpi_bytes_acked==22) }% net/ipv4/tcp_input.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index daff93d51..3bfa0f07d 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -4844,7 +4844,8 @@ static enum skb_drop_reason tcp_sequence(const struct sock *sk, const struct tcp_sock *tp = tcp_sk(sk); u32 seq_limit; - if (before(end_seq, tp->rcv_wup)) + if (before(end_seq, tp->rcv_wup) || + (end_seq == tp->rcv_wup && seq != end_seq)) return SKB_DROP_REASON_TCP_OLD_SEQUENCE; seq_limit = tp->rcv_nxt + tcp_max_receive_window(tp); -- 2.43.0