linux-kernel - Re: [PATCH 3/3] driver/core: Fix build error when SRCU and lockdep disabled

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: <20190812130310.GA27552@google.com>
Date:   Mon, 12 Aug 2019 09:03:10 -0400
From:   Joel Fernandes <joel@...lfernandes.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, kbuild test robot <lkp@...el.com>,
        Jonathan Corbet <corbet@....net>,
        Josh Triplett <josh@...htriplett.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        linux-doc@...r.kernel.org,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        "Paul E. McKenney" <paulmck@...ux.ibm.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>, rcu@...r.kernel.org,
        Steven Rostedt <rostedt@...dmis.org>, Tejun Heo <tj@...nel.org>
Subject: Re: [PATCH 3/3] driver/core: Fix build error when SRCU and lockdep
 disabled

On Mon, Aug 12, 2019 at 07:02:56AM +0200, Greg Kroah-Hartman wrote:
> On Sun, Aug 11, 2019 at 06:11:11PM -0400, Joel Fernandes (Google) wrote:
> > Properly check if lockdep lock checking is disabled at config time. If
> > so, then lock_is_held() is undefined so don't do any checking.
> > 
> > This fix is similar to the pattern used in srcu_read_lock_held().
> > 
> > Link: https://lore.kernel.org/lkml/201908080026.WSAFx14k%25lkp@intel.com/
> > Fixes: c9e4d3a2fee8 ("acpi: Use built-in RCU list checking for acpi_ioremaps list")
> 
> What tree is this commit in?
> 
> > Reported-by: kbuild test robot <lkp@...el.com>
> > Signed-off-by: Joel Fernandes (Google) <joel@...lfernandes.org>
> > ---
> > This patch is based on the -rcu dev branch.
> 
> Ah...
> 
> >  drivers/base/core.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 32cf83d1c744..fe25cf690562 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -99,7 +99,11 @@ void device_links_read_unlock(int not_used)
> >  
> >  int device_links_read_lock_held(void)
> >  {
> > -	return lock_is_held(&device_links_lock);
> > +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> > +	return lock_is_held(&(device_links_lock.dep_map));
> > +#else
> > +	return 1;
> > +#endif
> 
> return 1?  So the lock is always held?

This is just the pattern of an assert that is disabled, so that
false-positives don't happen if lockdep is disabled.

So say someone writes a statement like:
WARN_ON_ONCE(!device_links_read_lock_held());

Since lockdep is disabled, we cannot check whether lock is held or not. Yet,
we don't want false positives by reporting that the lock is not held. In this
case, it is better to report that the lock is held to suppress
false-positives.  srcu_read_lock_held() also follows the same pattern.

thanks,

 - Joel