netdev - [PATCH net] tcp_cubic: Fix for bug in HyStart implementation in the Linux kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <408334417.4436448.1736139157134@mail.yahoo.com>
Date: Mon, 6 Jan 2025 04:52:37 +0000 (UTC)
From: Mahdi Arghavani <ma.arghavani@...oo.com>
To: "edumazet@...gle.com" <edumazet@...gle.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, 
	Haibo Zhang <haibo.zhang@...go.ac.nz>, 
	David Eyers <david.eyers@...go.ac.nz>, 
	Abbas Arghavani <abbas.arghavani@....se>
Subject: [PATCH net] tcp_cubic: Fix for bug in HyStart implementation in the
 Linux kernel

Hi,

While refining the source code for our project (SUSS), I discovered a bug in the implementation of HyStart in the Linux kernel, starting from version v5.15.6. The issue, caused by incorrect marking of round starts, results in inaccurate measurement of the length of each ACK train. Since HyStart relies on the length of ACK trains as one of two key criteria to stop exponential cwnd growth during Slow-Start, this inaccuracy renders the criterion ineffective, potentially degrading TCP performance.

Issue Description: The problem arises because the hystart_reset function is not called upon receiving the first ACK (when cwnd=iw=10, see the attached figure). Instead, its invocation is delayed until the condition cwnd >= hystart_low_window is satisfied. In each round, this delay causes:

1) A postponed marking of the start of a new round.

2) An incorrect update of ca->end_seq, leading to incorrect marking of the subsequent round.

As a result, the ACK train length is underestimated, which adversely affects HyStart’s first criterion for stopping cwnd exponential growth.

Proposed Solution: Below is a tested patch that addresses the issue by ensuring hystart_reset is triggered appropriately:

diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c

index 5dbed91c6178..78d9cf493ace 100644

--- a/net/ipv4/tcp_cubic.c

+++ b/net/ipv4/tcp_cubic.c

@@ -392,6 +392,9 @@ static void hystart_update(struct sock *sk, u32 delay)

        if (after(tp->snd_una, ca->end_seq))

                bictcp_hystart_reset(sk);

+       if (tcp_snd_cwnd(tp) < hystart_low_window)

+               return;

+

        if (hystart_detect & HYSTART_ACK_TRAIN) {

                u32 now = bictcp_clock_us(sk);

@@ -468,8 +471,7 @@ __bpf_kfunc static void cubictcp_acked(struct sock *sk, const struct ack_sample

                ca->delay_min = delay;

        /* hystart triggers when cwnd is larger than some threshold */

-       if (!ca->found && tcp_in_slow_start(tp) && hystart &&

-           tcp_snd_cwnd(tp) >= hystart_low_window)

+       if (!ca->found && tcp_in_slow_start(tp) && hystart)

                hystart_update(sk, delay);
 }
Best wishes,
Mahdi Arghavani
Content of type "text/html" skipped

Download attachment "HyStart_bug.jpeg" of type "image/jpeg" (79382 bytes)