linux-kernel - Re: [PATCH v2 00/34] Compiler-Based Capability- and Locking-Analysis

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: <20250305161652.GA18280@noisy.programming.kicks-ass.net>
Date: Wed, 5 Mar 2025 17:16:52 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Bart Van Assche <bvanassche@....org>
Cc: Marco Elver <elver@...gle.com>, "David S. Miller" <davem@...emloft.net>,
	Luc Van Oostenryck <luc.vanoostenryck@...il.com>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Alexander Potapenko <glider@...gle.com>,
	Arnd Bergmann <arnd@...db.de>, Bill Wendling <morbo@...gle.com>,
	Boqun Feng <boqun.feng@...il.com>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Frederic Weisbecker <frederic@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Ingo Molnar <mingo@...nel.org>, Jann Horn <jannh@...gle.com>,
	Jiri Slaby <jirislaby@...nel.org>,
	Joel Fernandes <joel@...lfernandes.org>,
	Jonathan Corbet <corbet@....net>,
	Josh Triplett <josh@...htriplett.org>,
	Justin Stitt <justinstitt@...gle.com>, Kees Cook <kees@...nel.org>,
	Kentaro Takeda <takedakn@...data.co.jp>,
	Mark Rutland <mark.rutland@....com>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Miguel Ojeda <ojeda@...nel.org>,
	Nathan Chancellor <nathan@...nel.org>,
	Neeraj Upadhyay <neeraj.upadhyay@...nel.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	Thomas Gleixner <tglx@...utronix.de>,
	Uladzislau Rezki <urezki@...il.com>,
	Waiman Long <longman@...hat.com>, Will Deacon <will@...nel.org>,
	kasan-dev@...glegroups.com, linux-kernel@...r.kernel.org,
	llvm@...ts.linux.dev, rcu@...r.kernel.org,
	linux-crypto@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH v2 00/34] Compiler-Based Capability- and Locking-Analysis

On Wed, Mar 05, 2025 at 07:27:32AM -0800, Bart Van Assche wrote:
> On 3/5/25 3:20 AM, Peter Zijlstra wrote:
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index 248416ecd01c..d27607d9c2dc 100644
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -945,6 +945,7 @@ static inline unsigned int blk_boundary_sectors_left(sector_t offset,
> >    */
> >   static inline struct queue_limits
> >   queue_limits_start_update(struct request_queue *q)
> > +	__acquires(q->limits_lock)
> >   {
> >   	mutex_lock(&q->limits_lock);
> >   	return q->limits;
> > @@ -965,6 +966,7 @@ int blk_validate_limits(struct queue_limits *lim);
> >    * starting update.
> >    */
> >   static inline void queue_limits_cancel_update(struct request_queue *q)
> > +	__releases(q->limits_lock)
> >   {
> >   	mutex_unlock(&q->limits_lock);
> >   }
> 
> The above is incomplete. Here is what I came up with myself:

Oh, I'm sure. I simply fixed whatever was topmost in the compile output
when trying to build kernel/sched/. After fixing these two, it stopped
complaining about blkdev.

I think it complains about these because they're inline, even though
they're otherwise unused.

> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index 80a5b3268986..283fb85d96c8 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -1026,21 +1026,25 @@ static inline bool dev_pm_test_driver_flags(struct device *dev, u32 flags)
> >   }
> >   static inline void device_lock(struct device *dev)
> > +	__acquires(dev->mutex)
> >   {
> >   	mutex_lock(&dev->mutex);
> >   }
> >   static inline int device_lock_interruptible(struct device *dev)
> > +	__cond_acquires(0, dev->mutex)
> >   {
> >   	return mutex_lock_interruptible(&dev->mutex);
> >   }
> >   static inline int device_trylock(struct device *dev)
> > +	__cond_acquires(true, dev->mutex)
> >   {
> >   	return mutex_trylock(&dev->mutex);
> >   }
> >   static inline void device_unlock(struct device *dev)
> > +	__releases(dev->mutex)
> >   {
> >   	mutex_unlock(&dev->mutex);
> >   }
> 
> I propose to annotate these functions with __no_capability_analysis as a
> first step. Review of all callers of these functions in the entire
> kernel tree learned me that annotating these functions results in a
> significant number of false positives and not to the discovery of any
> bugs. The false positives are triggered by conditional locking. An
> example of code that triggers false positive thread-safety warnings:

Yeah, I've ran into this as well. The thing is entirely stupid when it
sees a branch. This is really unfortunate. But I disagree, I would
annotate those functions that have conditional locking with
__no_capability_analysis, or possibly:

#define __confused_by_conditionals __no_capability_analysis

I'm also not quite sure how to annotate things like pte_lockptr().


Anyway, this thing has some promise, however it is *really*, as in
*really* *REALLY* simple. Anything remotely interesting, where you
actually want the help, it falls over.

But you gotta start somewhere I suppose. I think the thing that is
important here is how receptive the clang folks are to working on this
-- because it definitely needs work.