Re: [PATCH net-next 1/2] tcp: don't set window_clamp from SO_RCVBUF before the socket is connected

On Mon, Oct 12, 2026, netdev-bot+sashiko@kernel.org wrote:

[High] Does this check cover every case where SO_RCVBUF is applied
before tcp_connect_init() runs?

You are right, it does not. The socket is already in TCP_SYN_SENT before
tcp_connect_init() runs, so SO_RCVBUF applied in that gap - via
TCP_FASTOPEN_CONNECT before the first write, or via bpf_setsockopt() in a
BPF_SOCK_OPS_TCP_CONNECT_CB program (as samples/bpf/tcp_clamp_kern.c and
friends do) - still writes window_clamp and still overrides the route
metric, and a state-based check cannot tell that SYN_SENT apart from a
post-init one. v3 will instead distinguish a user TCP_WINDOW_CLAMP from an
sk_rcvbuf-derived clamp, so tcp_connect_init() and tcp_openreq_init_rwin()
apply the route metric whenever the clamp was not set via
TCP_WINDOW_CLAMP; the selftest will cover the fastopen and sock_ops paths.

[Medium] Is the part about the case without a route metric accurate?

No, you are correct. With no RTAX_WINDOW metric both the old and the
patched kernel end at tcp_full_space() - the old kernel through the
"window_clamp > tcp_full_space()" branch, the patched one through
"window_clamp == 0" - so the advertised window and wscale are identical
and derived from sk_rcvbuf, not from the defaults; the patch changes
nothing there. The real impact is only (1) routes with an RTAX_WINDOW
metric and (2) a TCP_WINDOW_CLAMP that a later SO_RCVBUF on an unconnected
socket or listener overwrites. v3's changelog will say exactly that and
drop the "any application" / "without a metric" claims.

pw-bot: cr

Thanks for the careful review.
Shashank

On Mon, Oct 12, 2026 at 4:48 PM [off-list ref] wrote:

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete.
- [Medium] The commit message misstates and overstates the impact.

--

tcp: don't set window_clamp from SO_RCVBUF before the socket is connected

This patch makes tcp_set_rcvbuf() skip the window_clamp update while the
socket is in TCP_CLOSE or TCP_LISTEN. The aim is to stop a clamp derived
from SO_RCVBUF from overriding the RTAX_WINDOW route metric in
tcp_connect_init() and tcp_openreq_init_rwin().

quoted

tcp_connect_init() and tcp_openreq_init_rwin() treat a non-zero
window_clamp as a user clamp and, in that case, skip the route window
metric (RTAX_WINDOW, "ip route ... window N"). A clamp left behind by an
SO_RCVBUF call made before connect(), or on a listener, therefore
changes the window and window scale advertised in the SYN and SYN-ACK:
the route metric is no longer applied, and even without a route metric
the advertised window and scale come from SO_RCVBUF instead of the
defaults. This affects any application that sets SO_RCVBUF before
connect() or listen(), which is more common than using the route metric.

[Severity: Medium]
Is the part about the case without a route metric accurate?

__sock_set_rcvbuf() always sets SOCK_RCVBUF_LOCK and then calls
tcp_set_rcvbuf(sk, sk->sk_rcvbuf). On the old kernel, the clamp left
behind is therefore tcp_win_from_space(sk, sk_rcvbuf), which equals
tcp_full_space(), or SOCK_MIN_RCVBUF / 2 if that is larger.
scaling_ratio cannot change before connect.

With no RTAX_WINDOW metric, the patched kernel sees window_clamp == 0 and
reads a zero metric. Then this block runs:

net/ipv4/tcp_output.c:tcp_connect_init() {
    ...
        if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
            (tp->window_clamp > tcp_full_space(sk) || tp->window_clamp == 0))
                WRITE_ONCE(tp->window_clamp, tcp_full_space(sk));
    ...
}

The old kernel reaches the same tcp_full_space() value through the
"window_clamp > tcp_full_space(sk)" branch. tcp_openreq_init_rwin() does
the same for req->rsk_window_clamp.

Wouldn't both kernels then pass the same window_clamp to
tcp_select_initial_window(), and advertise the same window and wscale in
the SYN and SYN-ACK?

If so, the visible change is limited to two cases:

- routes with an RTAX_WINDOW metric
- TCP_WINDOW_CLAMP followed by SO_RCVBUF on an unconnected socket or
  listener, where SO_RCVBUF no longer overwrites the clamp

The message does not mention the second case. Neither case is "any
application that sets SO_RCVBUF before connect() or listen()".

The message also seems to contradict itself further down:

quoted

The "after" values match a kernel with b025461303d8 reverted; with no
route metric the SYN window and scale are the defaults in both cases.

In both cases, wouldn't the values come from SO_RCVBUF via
tcp_full_space() rather than from the defaults?

quoted

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index f4c9101e5996..e523cb71b905 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c

@@ -1858,6 +1858,14 @@ int tcp_set_rcvlowat(struct sock *sk, int val)

 void tcp_set_rcvbuf(struct sock *sk, int val)
 {
+     /* Without a connection there is no clamp to update: it is chosen
+      * from the route's window metric and the buffer size when the
+      * connection is set up, see tcp_connect_init() and
+      * tcp_openreq_init_rwin().
+      */
+     if ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))
+             return;

[Severity: High]
Does this check cover every case where SO_RCVBUF is applied before
tcp_connect_init() runs?

tcp_v4_connect() and tcp_v6_connect() switch the socket to TCP_SYN_SENT
before tcp_connect()->tcp_connect_init(). A clamp written in that gap
still passes this check, and there seem to be two ways to get there.

The first is TCP_FASTOPEN_CONNECT with a cached cookie, where connect()
returns early:

net/ipv4/tcp_ipv4.c:tcp_v4_connect() {
    ...
        tcp_set_state(sk, TCP_SYN_SENT);
    ...
        if (tcp_fastopen_defer_connect(sk, &err))
                return err;
    ...
}

If the application calls setsockopt(SO_RCVBUF) before its first write(),
tcp_set_rcvbuf() sees TCP_SYN_SENT and stores a non-zero window_clamp.
tcp_sendmsg_fastopen() later calls tcp_connect()->tcp_connect_init().

The second is a sock_ops BPF program. tcp_connect() runs the callback
before tcp_connect_init():

net/ipv4/tcp_output.c:tcp_connect() {
    ...
        tcp_call_bpf(sk, BPF_SOCK_OPS_TCP_CONNECT_CB, 0, NULL);
    ...
        tcp_connect_init(sk);
    ...
}

sol_socket_sockopt() allows SO_RCVBUF for bpf_setsockopt(). That call
reaches __sock_set_rcvbuf()->tcp_set_rcvbuf() while the socket is in
TCP_SYN_SENT. samples/bpf/tcp_bufs_kern.c, tcp_clamp_kern.c and
tcp_iw_kern.c all use this pattern.

In both cases tcp_connect_init() then skips the route metric:

        if (!tp->window_clamp)
                WRITE_ONCE(tp->window_clamp, dst_metric(dst, RTAX_WINDOW));

The SOCK_RCVBUF_LOCK block afterwards can only lower the clamp to
tcp_full_space().

Doesn't that leave the b025461303d8 regression in place on these paths,
even though this patch carries the Fixes: tag for it? The selftest in this
series only covers SO_RCVBUF set before connect().

Would it work to base the check on whether tcp_connect_init() has run yet?
Another option would be to let tcp_connect_init() and
tcp_openreq_init_rwin() tell a TCP_WINDOW_CLAMP value apart from one
derived from sk_rcvbuf.

quoted

+
      tcp_set_window_clamp(sk, tcp_win_from_space(sk, val));
 }

--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261011111606.14753-1-jain.sm%40gmail.com