Re: [PATCH net v2 0/2] tcp/mptcp: prevent blackhole timeout arithmetic wraparound

[Eric Dumazet <edumazet@kernel.org>]

Le sam. 14 nov. 2026 à 17:12, Yingjie Wang <x-professor@outlook.com> a écrit :
>
> The TCP Fast Open and MPTCP blackhole handlers multiply writable timeout
> sysctl values before adding the result to a jiffies timestamp. The
> multiplication is performed in 32-bit unsigned arithmetic, so a large
> value can wrap into a short delay even on a 64-bit kernel.
>
> With HZ=1000, a timeout of 6912213 seconds and the maximum multiplier of
> 64 should produce 442,381,632,000 jiffies, but the current calculations
> truncate the result to 512 jiffies. Both patches compute the delay in u64
> and cap it at MAX_JIFFY_OFFSET before adding it to the saved timestamp.
>
> The issues were found using a static analysis tool. In the v1 arithmetic
> A/B tests, loopback reproducers induced 12 blackholes, waited 1.2 seconds,
> and then made a new connection:
>
>   TFO:   base probe 1.026 s, TCPFastOpenActiveFail +1;
>          fixed probe 0.000 s, TCPFastOpenActiveFail +0.
>   MPTCP: base probe 3.087 s, MPCapableSYNTXDisabled +0;
>          fixed probe 0.000 s, MPCapableSYNTXDisabled +1.
>
> The TFO counter records another failed active Fast Open attempt. The
> MPTCP counter records a connection that skips MP_CAPABLE and falls back to
> TCP while active MPTCP is disabled.
>
> Changes since v1:
>
> - Add a reasonable upper bound for the TCP Fast Open sysctl, as requested
>   by Kuniyuki Iwashima. This version proposes one day (86400 seconds),
>   using the existing IPv4 one_day_secs bound, and documents that limit.
>   Keep the existing MPTCP sysctl input range.

The one day limit, the u64 computation and the clamp look fine to me
for the TCP side: with HZ=1000, 64 * 86400 * HZ still does not fit in
32 bits, and 32-bit kernels need the interval to stay below 2^31 jiffies
for time_before().

Why is MPTCP left alone? Kuniyuki's remark applies equally to
net.mptcp.blackhole_timeout: nobody needs a multi-day initial timeout
there either. I will let MPTCP maintainers decide, but having the two
sysctls behave differently looks odd.

> - Clarify that the multiplier is limited to 64 and the final delay is
>   separately capped for safe jiffies comparisons on 32-bit systems,
>   addressing Gang Yan's comment. Keep the READ_ONCE/WRITE_ONCE pairing
>   annotations next to the timestamp reads.
> - Identify the original runtime results as v1 evidence in both commits.

Please do not do that. The changelog of each patch should describe the
problem and the fix. Version history and test notes ("In the v1
arithmetic-only test...", "On the v2 kernel...") belong in the cover
letter or after the --- marker, not in the commit message.

> - Retain Gang Yan's Reviewed-by tag on the unchanged MPTCP arithmetic fix.
>
> v1: https://lore.kernel.org/r/cover.1794325665.git.x-professor@outlook.com
>
> On the final v2 x86-64 HZ=1000 kernel, TCP accepts 86400 and rejects 86401;
> MPTCP still accepts 86401 and INT_MAX. Boundary tests also cover zero,
> negative and oversized values, failed-write preservation, defaults, and
> network namespace isolation. The large timeout used in the v1 TFO test
> is now rejected by the TCP sysctl limit.
>
> The MPTCP blackhole test was rerun on this v2 kernel: all 12 clients
> completed, and the probe used TCP fallback directly (0.000 s,
> MPCapableSYNTXDisabled +1). The full x86-64 kernel build and affected
> object builds on x86-64 and i386 passed. The upstream TCP and MPTCP
> selftest suites were not run. Full allyesconfig/allmodconfig W=1 builds
> stopped at warnings promoted to
> errors in unchanged lockdep, coda, and x86 callthunks code;
> the affected objects passed W=1 KCFLAGS=-Werror in both configurations.
>
> The patches are independent and have separate Fixes tags.

About the Fixes/stable tags:

This needs an admin to set a timeout nobody would use, and the only
consequence is that active TFO (or MPTCP) is retried sooner than
requested.

Also patch 1 now makes the TCP sysctl reject values that were accepted
before. I do not think we want that in stable trees.

Please drop the Cc: stable and target net-next.

Thanks.

>
> Yingjie Wang (2):
>   tcp: avoid wrapping Fast Open blackhole timeout
>   mptcp: avoid wrapping blackhole timeout
>
>  Documentation/networking/ip-sysctl.rst |  1 +
>  net/ipv4/sysctl_net_ipv4.c             |  1 +
>  net/ipv4/tcp_fastopen.c                | 12 +++++++++---
>  net/mptcp/ctrl.c                       | 12 +++++++++---
>  4 files changed, 20 insertions(+), 6 deletions(-)
>
>
> base-commit: 30c71476d842b094651bd7712f6c8ee8716147d0
> --
> 2.43.0
>