linux-kernel - Re: [PATCH] lockdep: Let lock_is_held_type() detect recursive read as read

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]

Date:   Thu, 2 Sep 2021 00:43:45 +0800
From:   Boqun Feng <boqun.feng@...il.com>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
        Waiman Long <longman@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] lockdep: Let lock_is_held_type() detect recursive read
 as read

On Wed, Sep 01, 2021 at 06:22:55PM +0200, Sebastian Andrzej Siewior wrote:
> lock_is_held_type(, 1) detects acquired read locks. It only recognized
> locks acquired with lock_acquire_shared(). Read locks acquired with
> lock_acquire_shared_recursive() are not recognized because a `2' is
> stored as the read value.
> 
> Rework the check to additionally recognise lock's read value one and two
> as a read held lock.
> 
> Fixes: e918188611f07 ("locking: More accurate annotations for read_lock()")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> ---
> 
> On a related note: What exactly means read_lock_is_recursive() in terms
> of recursive locking? The second items mentions QRW locks. Does this
> mean that a pending WRITER blocks further READER from acquiring the
> lock?
> 

If a reader is recursive, then a pending writer doesn't block the
recursive reader, otherwise, a pending write blocks the reader. IOW, a
pending writer blocks non-recursive readers but not recursive readers.

In most case for a lock, all the readers are either recursive or
non-recursive, but queued rwlock is a little different, readers in the
interrupt are recursive while readers in normal context are not. So
lockdep needs to handle them differently.

However, one special case is that in the selftest cases for lockdep, we
want to make all read_lock() recursive (for the purpose of testing),
that's why read_lock_is_recursive() has a force_read_lock_recursive bit.

Does the above make things answer your question?

Regards,
Boqun

>  kernel/locking/lockdep.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index f15df3fd7c5a6..39f98454a8827 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -5366,7 +5366,9 @@ int __lock_is_held(const struct lockdep_map *lock, int read)
>  		struct held_lock *hlock = curr->held_locks + i;
>  
>  		if (match_held_lock(hlock, lock)) {
> -			if (read == -1 || hlock->read == read)
> +			if (read == -1 ||
> +			    (read == 0 && hlock->read == 0) ||
> +			    (read == 1 && hlock->read > 0))
>  				return LOCK_STATE_HELD;
>  
>  			return LOCK_STATE_NOT_HELD;
> -- 
> 2.33.0
>