netdev - Re: [PATCH net] af_unix: Fix a data-race in unix_dgram_peer_wake

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 07 Jun 2022 12:35:13 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Kuniyuki Iwashima <kuniyu@...zon.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Rainer Weikusat <rweikusat@...ileactivedefense.com>
Cc:     Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net] af_unix: Fix a data-race in
 unix_dgram_peer_wake_me().

On Sun, 2022-06-05 at 16:23 -0700, Kuniyuki Iwashima wrote:
> unix_dgram_poll() calls unix_dgram_peer_wake_me() without `other`'s
> lock held and check if its receive queue is full.  Here we need to
> use unix_recvq_full_lockless() instead of unix_recvq_full(), otherwise
> KCSAN will report a data-race.
> 
> Fixes: 7d267278a9ec ("unix: avoid use-after-free in ep_remove_wait_queue")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> ---
> As Eric noted in commit 04f08eb44b501, I think rest of unix_recvq_full()
> can be turned into the lockless version.  After this merge window, I can
> send a follow-up patch if there is no objection.

It looks like replacing the remaining instances of unix_recvq_full()
with unix_recvq_full_lockless() should be safe, but I'm wondering if
doing that while retaining the current state lock scope it's worthy?!? 

It may trick later readers of the relevant code to think that such code
may be reached without a lock. Or are you suggesting to additionally
shrink the state lock scope? that latter part looks much more tricky,
IMHO.

Cheers,

Paolo