netdev - Re: [PATCH] net: do not release sk in sk_wait

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e6171479-28b4-4155-8578-37a14dabee50@redhat.com>
Date: Thu, 15 Aug 2024 12:03:37 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: sunyiqi <sunyiqixm@...il.com>, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: do not release sk in sk_wait_event



On 8/15/24 10:49, sunyiqi wrote:
> When investigating the kcm socket UAF which is also found by syzbot,
> I found that the root cause of this problem is actually in
> sk_wait_event.
> 
> In sk_wait_event, sk is released and relocked and called by
> sk_stream_wait_memory. Protocols like tcp, kcm, etc., called it in some
> ops function like *sendmsg which will lock the sk at the beginning.
> But sk_stream_wait_memory releases sk unexpectedly and destroy
> the thread safety. Finally it causes the kcm sk UAF.
> 
> If at the time when a thread(thread A) calls sk_stream_wait_memory
> and the other thread(thread B) is waiting for lock in lock_sock,
> thread B will successfully get the sk lock as thread A release sk lock
> in sk_wait_event.
> 
> The thread B may change the sk which is not thread A expecting.
> 
> As a result, it will lead kernel to the unexpected behavior. Just like
> the kcm sk UAF, which is actually cause by sk_wait_event in
> sk_stream_wait_memory.
> 
> Previous commit d9dc8b0f8b4e ("net: fix sleeping for sk_wait_event()")
> in 2016 seems do not solved this problem. Is it necessary to release
> sock in sk_wait_event? Or just delete it to make the protocol ops
> thread-secure.

As a I wrote previously, please describe the suspected race more 
clearly, with the exact calls sequence that lead to the UAF.

Releasing the socket lock is not enough to cause UAF.

Removing the release/lock pair in sk_wait_event() will break many 
protocols (e.g. TCP) as the stack will not be able to land packets in 
the receive queue while the socked lock is owned.

Cheers,

Paolo