Re: [PATCH RFC -next 2/5] tcp: sync memcg budget on protocol buffer updates

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tcp: memcg limit bypass via unhandled budget sync failures
--

commit e6770a5d905e4be1d779ff6b117516aca572b367
Author: Cai Xinchen <caixinchen1@huawei.com>
Date:   Thu Sep 24 08:02:16 2026 +0000

tcp: sync memcg budget on protocol buffer updates

This commit updates the memcg budget tracker during various TCP autotuning
and buffer adjustment operations by adding calls to sk_memcg_budget_sync()
after sk_sndbuf and sk_rcvbuf are written.

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 1e2074f733f43..629ce7dc3032d 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -471,6 +471,10 @@ void tcp_init_sock(struct sock *sk)
>  
>  	WRITE_ONCE(sk->sk_sndbuf, READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_wmem[1]));
>  	WRITE_ONCE(sk->sk_rcvbuf, READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_rmem[1]));
> +	/* The default buffers grew from the generic sock_init_data()
> +	 * values: charge the difference to the memcg.
> +	 */
> +	sk_memcg_budget_sync(sk, gfp_memcg_charge());

[Severity: High]
In tcp_init_sock(), if sk_memcg_budget_sync() returns false here (e.g., if
the memcg limit is reached), does this leave the socket buffers expanded
without being properly charged?

Could this allow a socket to consume uncharged memory up to the expanded
limits, bypassing memcg resource isolation?

[ ... ]
> @@ -1850,6 +1854,7 @@ int tcp_set_rcvlowat(struct sock *sk, int val)
>  	space = tcp_space_from_win(sk, val);
>  	if (space > sk->sk_rcvbuf) {
>  		WRITE_ONCE(sk->sk_rcvbuf, space);
> +		sk_memcg_budget_sync(sk, gfp_memcg_charge());

[Severity: High]
In tcp_set_rcvlowat(), if sk_memcg_budget_sync() fails to charge the memcg,
shouldn't sk_rcvbuf be rolled back to its previous value?

[ ... ]
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 892ff256e2352..9fdb2c66821b7 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -629,9 +629,11 @@ static void tcp_sndbuf_expand(struct sock *sk)
>  	sndmem = ca_ops->sndbuf_expand ? ca_ops->sndbuf_expand(sk) : 2;
>  	sndmem *= nr_segs * per_mss;
>  
> -	if (sk->sk_sndbuf < sndmem)
> +	if (sk->sk_sndbuf < sndmem) {
>  		WRITE_ONCE(sk->sk_sndbuf,
>  			   min(sndmem, READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_wmem[2])));
> +		sk_memcg_budget_sync(sk, gfp_memcg_charge());
> +	}

[Severity: High]
In tcp_sndbuf_expand(), since this runs in softirq and uses GFP_ATOMIC,
memcg charges are more likely to fail under pressure.

If sk_memcg_budget_sync() returns false, does the expanded sk_sndbuf need
to be reverted to prevent uncharged memory consumption up to
sysctl_tcp_wmem[2]?

[ ... ]
> @@ -791,6 +793,7 @@ static void tcp_clamp_window(struct sock *sk)
>  	    sk_memory_allocated(sk) < sk_prot_mem_limits(sk, 0)) {
>  		WRITE_ONCE(sk->sk_rcvbuf,
>  			   min(atomic_read(&sk->sk_rmem_alloc), rmem2));
> +		sk_memcg_budget_sync(sk, gfp_memcg_charge());
>  	}

[Severity: High]
Is it safe to ignore the return value of sk_memcg_budget_sync() here after
expanding sk_rcvbuf in tcp_clamp_window()?

[ ... ]
> @@ -948,6 +951,7 @@ void tcp_rcvbuf_grow(struct sock *sk, u32 newval)
>  	rcvbuf = min_t(u32, tcp_space_from_win(sk, rcvwin), cap);
>  	if (rcvbuf > sk->sk_rcvbuf) {
>  		WRITE_ONCE(sk->sk_rcvbuf, rcvbuf);
> +		sk_memcg_budget_sync(sk, gfp_memcg_charge());

[Severity: High]
Similar to the other autotuning functions, if the charge fails in
tcp_rcvbuf_grow(), does this leave sk_rcvbuf increased without charging
the memcg?

[ ... ]
> @@ -6055,8 +6059,10 @@ static bool tcp_should_expand_sndbuf(struct sock *sk)
>  		 * it never goes below SOCK_MIN_SNDBUF.
>  		 * See sk_stream_moderate_sndbuf() for more details.
>  		 */
> -		if (unused_mem > SOCK_MIN_SNDBUF)
> +		if (unused_mem > SOCK_MIN_SNDBUF) {
>  			WRITE_ONCE(sk->sk_sndbuf, unused_mem);
> +			sk_memcg_budget_sync(sk, gfp_memcg_charge());
> +		}

[Severity: High]
Should sk_sndbuf be restored in tcp_should_expand_sndbuf() if the budget
sync fails here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924080219.1036588-1-caixinchen1@huawei.com?part=2