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

yanjun.zhu
2026-04-28T16:56:44Z

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