Re: [PATCH net] soreuseport: Fix use-after-free when a socket is added to socks[] twice

[Norbert Szetei]

On Sep 28, 2026, at 19:45, Kuniyuki Iwashima <kuniyu@xxxxxxxxxx> wrote:
> This has long been a known problem, and I think it's time
> to fix it instead of working around it:
> 
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 2948dffcc3e1..a33cdf99368d 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1324,6 +1324,8 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
> case SO_REUSEPORT:
> if (valbool && !sk_is_inet(sk))
> ret = -EOPNOTSUPP;
> + else if (!valbool && rcu_access_pointer(sk->sk_reuseport_cb))
> + ret = -EBUSY;
> else
> sk->sk_reuseport = valbool;
> break;

Thanks for the suggestion. I tested it with my sock_reuseport.c change dropped
and I was not able to reproduce the issue, and migration and resurrect still
work.

One question before I send it as v2 with Suggested-by: you, unless you
would rather post it yourself.

Should the check be restricted to TCP?

	else if (!valbool && sk->sk_protocol == IPPROTO_TCP &&
		 rcu_access_pointer(sk->sk_reuseport_cb))
		ret = -EBUSY;


The closed section only exists for TCP, so TCP is the only protocol where
clearing the flag leads to the double add. With the check as it is, a bound
UDP socket also starts getting EBUSY from setsockopt(SO_REUSEPORT, 0).

N.

>> Clearing it between shutdown() and listen() makes that listen() skip
>> reuseport_add_sock(), and with it reuseport_resurrect(), so the socket is
>> hashed as a listener while it is still in the closed section. On the next
>> shutdown() __reuseport_detach_sock() does not find it in the listening
>> section, returns false, and __reuseport_add_closed_sock() adds a second
>> copy of it to socks[].
>> 
>> sk_destruct() calls reuseport_detach_sock(), which removes one of the two
>> entries. reuseport_grow() then dereferences the other one, because its
>> loop runs over every slot up to reuse->max_socks:
>> 
>>  BUG: KASAN: slab-use-after-free in reuseport_grow (net/core/sock_reuseport.c:291)
>>  Write of size 8 at addr ffff888132359988 by task poc/621
>> 
>>   reuseport_grow (net/core/sock_reuseport.c:291)
>>   reuseport_add_sock (net/core/sock_reuseport.c:350)
>>   inet_hash (net/ipv4/inet_hashtables.c:810)
>>   inet_csk_listen_start (net/ipv4/inet_connection_sock.c:1359)
>>   __inet_listen_sk (net/ipv4/af_inet.c:225)
>>   inet_listen (net/ipv4/af_inet.c:247)
>>   __sys_listen (net/socket.c:2014)
>> 
>>  Allocated by task 621:
>>   sk_prot_alloc (net/core/sock.c:2246)
>>   sk_alloc (net/core/sock.c:2308)
>>   inet_create (net/ipv4/af_inet.c:333)
>> 
>>  Freed by task 0:
>>   slab_free_after_rcu_debug (mm/slub.c:6570)
>>   rcu_core (kernel/rcu/tree.c:2919)
>> 
>>  The buggy address is located 904 bytes inside of
>>   freed 2624-byte region [ffff888132359600, ffff88813235a040)
>> 
>> Only move the socket to the closed section when __reuseport_detach_sock()
>> reports that it was removed from the listening section.
>> 
>> Fixes: 333bb73f620e ("tcp: Keep TCP_CLOSE sockets in the reuseport group.")
>> Assisted-by: LLM
>> Signed-off-by: Norbert Szetei <norbert@xxxxxxxxxxxx>
>> ---
>> net/core/sock_reuseport.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
>> index 29948cb44b7d..031b641be317 100644
>> --- a/net/core/sock_reuseport.c
>> +++ b/net/core/sock_reuseport.c
>> @@ -479,8 +479,8 @@ void reuseport_stop_listen_sock(struct sock *sk)
>> */
>> bpf_sk_reuseport_detach(sk);
>> 
>> - __reuseport_detach_sock(sk, reuse);
>> - __reuseport_add_closed_sock(sk, reuse);
>> + if (__reuseport_detach_sock(sk, reuse))
>> + __reuseport_add_closed_sock(sk, reuse);
>> 
>> spin_unlock_bh(&reuseport_lock);
>> return;
>> -- 
>> 2.55.0
>>