linux-kernel - Re: [BUG]locking/rwsem: only clean RWSEM_FLAG

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] [day] [month] [year] [list]

Message-ID: <a630a9aa-8c66-31c9-21a0-3d30bde2c9df@redhat.com>
Date:   Sat, 6 Nov 2021 23:28:53 -0400
From:   Waiman Long <longman@...hat.com>
To:     马振华 <mazhenhua@...omi.com>,
        peterz <peterz@...radead.org>, mingo <mingo@...hat.com>,
        will <will@...nel.org>, "boqun.feng" <boqun.feng@...il.com>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [BUG]locking/rwsem: only clean RWSEM_FLAG_HANDOFF when already
 set


On 11/6/21 23:25, Waiman Long wrote:
>
>> so sem->count will be negative after writer is killed
>> i think if flag RWSEM_FLAG_HANDOFF is not set, we shouldn't clean it
>
> Thanks for reporting this possible race condition.
>
> However, I am still trying to figure how it is possible to set the 
> wstate to WRITER_HANDOFF without actually setting the handoff bit as 
> well. The statement sequence should be as follows:
>
> wstate = WRITER_HANDOFF;
> raw_spin_lock_irq(&sem->wait_lock);
> if (rwsem_try_write_lock(sem, wstate))
> raw_spin_unlock_irq(&sem->wait_lock);
>   :
> if (signal_pending_state(state, current))
>     goto out_nolock
>
> The rwsem_try_write_lock() function will make sure that we either 
> acquire the lock and clear handoff or set the handoff bit. This should 
> be done before we actually check for signal. I do think that it is 
> probably safer to use atomic_long_andnot to clear the handoff bit 
> instead of using atomic_long_add().
BTW, do you have a reproducer that can reproduce the race condition?

Thanks,
Longman