lkml.org 
[lkml]   [2023]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RFC PATCH] SUNRPC: Fix UAF in svc_tcp_listen_data_ready()
From
On 2023/5/8 12:00, Chuck Lever III wrote:
>
>
>> On May 7, 2023, at 6:32 PM, Ding Hui <dinghui@sangfor.com.cn> wrote:
>>
>> On 2023/5/7 23:26, Chuck Lever III wrote:
>>>> On May 7, 2023, at 5:11 AM, Ding Hui <dinghui@sangfor.com.cn> wrote:
>>>>
>>>> After the listener svc_sock freed, and before invoking svc_tcp_accept()
>>>> for the established child sock, there is a window that the newsock
>>>> retaining a freed listener svc_sock in sk_user_data which cloning from
>>>> parent. In the race windows if data is received on the newsock, we will
>>>> observe use-after-free report in svc_tcp_listen_data_ready().
>>> My thought is that not calling sk_odata() for the newsock
>>> could potentially result in missing a data_ready event,
>>> resulting in a hung client on that socket.
>>
>> I checked the vmcore, found that sk_odata points to sock_def_readable(),
>> and the sk_wq of newsock is NULL, which be assigned by sk_clone_lock()
>> unconditionally.
>>
>> Calling sk_odata() for the newsock maybe do not wake up any sleepers.
>>
>>> IMO the preferred approach is to ensure that svsk is always
>>> safe to dereference in tcp_listen_data_ready. I haven't yet
>>> thought carefully about how to do that.
>>
>> Agree, but I don't have a good way for now.
>
> Would a smartly-placed svc_xprt_get() hold the listener in place
> until accept processing completes?
>

It is difficult and complicated to me. I think it's a little bit out of
SUNRPC's control for the newsocks before accepted, e.g.: we don't know
how many they have.

Back to this RFC, I checked the code and thought it is safe by skipping
sk_odata() for the newsocks before accepted in **svc_tcp_listen_data_ready()**,
since these newsocks's sk_wq must be NULL, and will be assigned new one in
sock_alloc_inode() called by kernel_accept(), so we can say if the child sock
is not be accepted, there is nothing to be waked up.

>
>>>> Reproduce by two tasks:
>>>> ...

--
Thanks,
- Ding Hui

\
 
 \ /
  Last update: 2023-05-08 08:29    [W:0.047 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site