linux-kernel - Re: [PATCH v3 00/35] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aNEX46WJh2IWhVUc@elver.google.com>
Date: Mon, 22 Sep 2025 11:33:23 +0200
From: Marco Elver <elver@...gle.com>
To: Christoph Hellwig <hch@....de>
Cc: Nathan Chancellor <nathan@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Boqun Feng <boqun.feng@...il.com>, Ingo Molnar <mingo@...nel.org>,
	Will Deacon <will@...nel.org>,
	"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>, Bart Van Assche <bvanassche@....org>,
	Bill Wendling <morbo@...gle.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>,
	Ian Rogers <irogers@...gle.com>, Jann Horn <jannh@...gle.com>,
	Joel Fernandes <joelagnelf@...dia.com>,
	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>,
	Lukas Bulwahn <lukas.bulwahn@...il.com>,
	Mark Rutland <mark.rutland@....com>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Miguel Ojeda <ojeda@...nel.org>,
	Neeraj Upadhyay <neeraj.upadhyay@...nel.org>,
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	Thomas Gleixner <tglx@...utronix.de>, Thomas Graf <tgraf@...g.ch>,
	Uladzislau Rezki <urezki@...il.com>,
	Waiman Long <longman@...hat.com>, kasan-dev@...glegroups.com,
	linux-crypto@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org, linux-security-module@...r.kernel.org,
	linux-sparse@...r.kernel.org, llvm@...ts.linux.dev,
	rcu@...r.kernel.org
Subject: Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis

On Fri, Sep 19, 2025 at 04:09PM +0200, Christoph Hellwig wrote:
> On Fri, Sep 19, 2025 at 04:08:03PM +0200, Christoph Hellwig wrote:
> > I started to play around with that.  For the nvme code adding the
> > annotations was very simply, and I also started adding trivial
> > __guarded_by which instantly found issues.
> > 
> > For XFS it was a lot more work and I still see tons of compiler
> > warnings, which I'm not entirely sure how to address.  Right now I
> > see three major classes:
> 
> And in case anyone cares, here are my patches for that:
> 
> https://git.infradead.org/?p=users/hch/misc.git;a=shortlog;h=refs/heads/cap-analysis
> 
> git://git.infradead.org/users/hch/misc.git cap-analysis

I gave this a try, and with the below patch and the Clang fix [1],
fs/xfs compiles cleanly. I think the fundamental limitation are the
conditional locking wrappers. I suspect it's possible to do better than
disabling the analysis here, by overapproximating the lock set taken
(like you did elsewhere), so that at least the callers are checked, but
when I tried it showed lots of callers need annotating as well, so I
gave up at that point. Still, it might be better than no checking at
all.

[1] https://github.com/llvm/llvm-project/pull/159921

Thanks,
 -- Marco


diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 9c39251961a3..f371a08e5d44 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -140,6 +140,7 @@ void
 xfs_ilock(
 	xfs_inode_t		*ip,
 	uint			lock_flags)
+	__capability_unsafe(/* conditional locking */)
 {
 	trace_xfs_ilock(ip, lock_flags, _RET_IP_);
 
@@ -183,6 +184,7 @@ int
 xfs_ilock_nowait(
 	xfs_inode_t		*ip,
 	uint			lock_flags)
+	__capability_unsafe(/* conditional locking */)
 {
 	trace_xfs_ilock_nowait(ip, lock_flags, _RET_IP_);
 
@@ -243,6 +245,7 @@ void
 xfs_iunlock(
 	xfs_inode_t		*ip,
 	uint			lock_flags)
+	__capability_unsafe(/* conditional locking */)
 {
 	xfs_lock_flags_assert(lock_flags);
 
@@ -272,6 +275,7 @@ void
 xfs_ilock_demote(
 	xfs_inode_t		*ip,
 	uint			lock_flags)
+	__capability_unsafe(/* conditional locking */)
 {
 	ASSERT(lock_flags & (XFS_IOLOCK_EXCL|XFS_MMAPLOCK_EXCL|XFS_ILOCK_EXCL));
 	ASSERT((lock_flags &
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index d9ac9521c203..9c4ec3aa8bf9 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -472,6 +472,7 @@ xfs_log_reserve(
 static void
 xlog_state_shutdown_callbacks(
 	struct xlog		*log)
+	__must_hold(&log->l_icloglock)
 {
 	struct xlog_in_core	*iclog;
 	LIST_HEAD(cb_list);