Re: [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The variable `ca_ops` is used but never declared in the `TCP_CONGESTION` block.
--

commit 7bf16b98f97797b307198d9ae8e078d4cbb59158
Author: Cen Zhang (Microsoft Security FORGE Labs) <blbllhy@gmail.com>

tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION)

do_tcp_getsockopt() reads icsk->icsk_ca_ops->name without holding
rcu_read_lock(). Since commit 0baf26b0fcd7, icsk_ca_ops can point to
dynamically allocated BPF struct_ops memory that may be freed concurrently via
setsockopt(TCP_CONGESTION), leading to a use-after-free.

Link: https://lore.kernel.org/all/20260821182449.79785-2-blbllhy@gmail.com/

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index b4237d0e994d6..4918f48ba76d5 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -4577,16 +4577,24 @@ int do_tcp_getsockopt(struct sock *sk, int level,
>  		val = !inet_csk_in_pingpong_mode(sk);
>  		break;
>  
> -	case TCP_CONGESTION:
> +	case TCP_CONGESTION: {
> +		char ca_name[TCP_CA_NAME_MAX] = {};
> +
>  		if (copy_from_sockptr(&len, optlen, sizeof(int)))
>  			return -EFAULT;
>  		len = min_t(unsigned int, len, TCP_CA_NAME_MAX);
>  		if (copy_to_sockptr(optlen, &len, sizeof(int)))
>  			return -EFAULT;
> -		if (copy_to_sockptr(optval, icsk->icsk_ca_ops->name, len))
> +
> +		rcu_read_lock();
> +		ca_ops = READ_ONCE(icsk->icsk_ca_ops);

[Severity: Low]
Does this introduce a compilation error in do_tcp_getsockopt() because ca_ops
is used but never declared in the TCP_CONGESTION block?

It appears ca_ops is scoped only inside the adjacent TCP_CC_INFO case block
and is not declared at the function level or within this block, which will
cause a build failure.

> +		memcpy(ca_name, ca_ops->name, sizeof(ca_name));
> +		rcu_read_unlock();
> +
> +		if (copy_to_sockptr(optval, ca_name, len))
>  			return -EFAULT;
>  		return 0;
> -
> +	}
>  	case TCP_ULP:
>  		if (copy_from_sockptr(&len, optlen, sizeof(int)))
>  			return -EFAULT;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826171344.4133-1-blbllhy@gmail.com?part=1