Re: [PATCH v2 1/2] RDMA/rxe: Fix null-ptr-deref in kernel_sock_shutdown().

["yanjun.zhu" <yanjun.zhu@linux.dev>]

On 4/27/26 11:39 PM, Kuniyuki Iwashima wrote:
> On Mon, Apr 27, 2026 at 11:30 PM Zhu Yanjun <yanjun.zhu@linux.dev> wrote:
>>
>> 在 2026/4/27 22:22, Kuniyuki Iwashima 写道:
>>> On Mon, Apr 27, 2026 at 10:12 PM Zhu Yanjun <yanjun.zhu@linux.dev> wrote:
>>>>
>>>>
>>>>
>>>> 在 2026/4/27 19:15, Zhu Yanjun 写道:
>>>>>
>>>>> 在 2026/4/27 17:58, David Ahern 写道:
>>>>>> On 4/27/26 6:52 PM, Kuniyuki Iwashima wrote:
>>>>>>> To be clear, you meant implementing David' idea, right ?
>>>>>>> I'm asking because dellink won't need locking then.
>>>>>> dellink is not needed with my suggestion. It was added to manage
>>>>>> basically a refcount on the socket to close on last rxe delete in the
>>>>>
>>>>> This is my original implementation.
>>>>>
>>>>> @Kuniyuki Iwashima, can you reproduce this problem in your local host or
>>>>> other test environments?
>>>
>>> The syzbot does not have a repro, but I think it can be
>>> reproduced by calling newlink and dellink with multiple
>>> threads.
>>>
>>> newlink would trigger kmemleak splat while dellink trigger
>>> KASAN splat.
>>>
>>>
>>>>>
>>>>> If yes, can you make tests after applying the commit in the link:
>>>>> https://patchwork.kernel.org/project/linux-rdma/
>>>>> patch/20260424043522.22901-1-yanjun.zhu@linux.dev/
>>>>>
>>>>> Thanks a lot.
>>>>
>>>> Hi, David && Kuniyuki
>>>>
>>>> I read the call trace again.
>>>>
>>>> If net namespace has already released socket in A thread, then rdma link
>>>> del command is called in B thread to release socket.
>>>>
>>>> So A thread has released socket firstly, then B thread also release socket.
>>>>
>>>> The similar call trace would appear.
>>>>
>>>> The followiing is the explanation to the commit
>>>> https://patchwork.kernel.org/project/linux-rdma/patch/20260424043522.22901-1-yanjun.zhu@linux.dev/
>>>>
>>>> The double-free occurs as follows:
>>>>
>>>> CPU 0 (Net NameSpace cleanup)        CPU 1 (RDMA device removal)
>>>> ---------------------                ---------------------------
>>>> rxe_ns_exit()                        rxe_link_delete() (rdma link del )
>>>
>>> If rxe_link_delete() is in progress, it means the user thread is
>>> alive, holding the netns refcount, and rxe_ns_exit() cannot be
>>> called.
>>>
>>> So, dellink() never races with rxe_ns_exit(), and it races only
>>> with the concurrent dellink().
>>>
>>> And when that occurs, the number of threads is not limited to
>>> two, theoretically triple-free, quad-free, ... are possible.
>>
>> Thread 1: rdma link del          Thread 2: rdma link del
>>        (User A calls dellink)           (User B calls dellink)
>>                 |                                 |
>>         (1) Get Socket Pointer            (2) Get Socket Pointer
>>             sk = ns_sk->rxe_sk4               sk = ns_sk->rxe_sk4
>>                 |                                 |
>>         (3) Release Socket                (4) Release Socket
>>             udp_tunnel_sock_release(sk)       udp_tunnel_sock_release(sk)
>>                 |                                 |
>>           [ FIRST FREE ]                          |
>>                 |                          [ DOUBLE FREE! ]
>>                 v                                 v
>>           (Memory freed)                  (Kernel Panic / Crash)
>>
>> I think the above should explain your idea. If so, your solution makes
>> senses to add a per-netns mutex to synchronise.
>>
>> Let us use the first solution
>> https://lore.kernel.org/all/20260424013759.728288-1-kuniyu@google.com/
>>
>> BTW, 1) add mutex_destroy 2) take into account of rdma link add.
>>
>> I am not sure if it is OK or not. @David Ahern
> 
> No, newlink is still racy and the same kind of race leaks
> the udp tunnel.
> 
> If we defer allocation, there are two options:
> 
> 1. David's idea, allocate on first use, and no free
>    until netns destruction (newlink can add a fast path
>    like check the pointer and only take mutex when it's
>    NULL, and check again under mutex and allcoate a
>   tunnel if not yet allocated)
> 
> 2. Manage refcount properly.  (If we allocate a dedicated
>    refcount for each tunnel socket in rxe_ns_sock, we
>    can implement a similar fast path for newlink, and dellink
>    will be lockless thanks to atomic)

I suggest we stick with Option 2 (proper refcounting) but move away from 
a purely lockless dellink. By protecting the tunnel destruction with a 
mutex, we can effectively close the race window and ensure the UDP 
tunnel is cleaned up reliably without compromising the efficiency of the 
fast path in newlink.

Zhu Yanjun