Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [PATCH] selftests/landlock: NULL-terminate unix pathname addresses
From: Mickaël Salaün @ 2025-12-19 20:26 UTC (permalink / raw)
  To: Günther Noack; +Cc: Matthieu Buffet, linux-security-module
In-Reply-To: <aTAHfes90YxsXWt0@google.com>

Thanks for both patches! Applied to my -next branch.

On Wed, Dec 03, 2025 at 10:48:45AM +0100, Günther Noack wrote:
> On Tue, Dec 02, 2025 at 10:51:41PM +0100, Matthieu Buffet wrote:
> > The size of Unix pathname addresses is computed in selftests using
> > offsetof(struct sockaddr_un, sun_path) + strlen(xxx). It should have
> > been that +1, which makes addresses passed to the libc and kernel
> > non-NULL-terminated. unix_mkname_bsd() fixes that in Linux so there is
> > no harm, but just using sizeof(the address struct) should improve
> > readability.
> > 
> > Signed-off-by: Matthieu Buffet <matthieu@buffet.re>
> > ---
> > [...]
> 
> Thank you very much, this looks good!
> 
> Reviewed-by: Günther Noack <gnoack@google.com>
> 
> —Günther
> 

^ permalink raw reply

* Re: [PATCH v5 12/36] bit_spinlock: Include missing <asm/processor.h>
From: Bart Van Assche @ 2025-12-19 20:38 UTC (permalink / raw)
  To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
  Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <20251219154418.3592607-13-elver@google.com>

On 12/19/25 7:40 AM, Marco Elver wrote:
> Including <linux/bit_spinlock.h> into an empty TU will result in the
> compiler complaining:
> 
> ./include/linux/bit_spinlock.h:34:4: error: call to undeclared function 'cpu_relax'; <...>
>     34 |                         cpu_relax();
>        |                         ^
> 1 error generated.
> 
> Include <asm/processor.h> to allow including bit_spinlock.h where
> <asm/processor.h> is not otherwise included.
> 
> Signed-off-by: Marco Elver <elver@google.com>
> ---
>   include/linux/bit_spinlock.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/bit_spinlock.h b/include/linux/bit_spinlock.h
> index c0989b5b0407..59e345f74b0e 100644
> --- a/include/linux/bit_spinlock.h
> +++ b/include/linux/bit_spinlock.h
> @@ -7,6 +7,8 @@
>   #include <linux/atomic.h>
>   #include <linux/bug.h>
>   
> +#include <asm/processor.h>  /* for cpu_relax() */
> +
>   /*
>    *  bit-based spin_lock()
>    *

The abbreviation "TU" is uncommon so it's probably a good idea to expand
it. Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply

* Re: [PATCH v5 13/36] bit_spinlock: Support Clang's context analysis
From: Bart Van Assche @ 2025-12-19 20:47 UTC (permalink / raw)
  To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
  Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <20251219154418.3592607-14-elver@google.com>

On 12/19/25 7:40 AM, Marco Elver wrote:
> +/*
> + * For static context analysis, we need a unique token for each possible bit
> + * that can be used as a bit_spinlock. The easiest way to do that is to create a
> + * fake context that we can cast to with the __bitlock(bitnum, addr) macro
> + * below, which will give us unique instances for each (bit, addr) pair that the
> + * static analysis can use.
> + */
> +context_lock_struct(__context_bitlock) { };
> +#define __bitlock(bitnum, addr) (struct __context_bitlock *)(bitnum + (addr))

Will this cause static analyzers to complain about out-of-bounds
accesses for (bitnum + (addr)), which is equivalent to &(addr)[bitnum]?

Thanks,

Bart.

^ permalink raw reply

* Re: [PATCH v5 16/36] kref: Add context-analysis annotations
From: Bart Van Assche @ 2025-12-19 20:49 UTC (permalink / raw)
  To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
  Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <20251219154418.3592607-17-elver@google.com>

On 12/19/25 7:40 AM, Marco Elver wrote:
> Mark functions that conditionally acquire the passed lock.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply

* Re: [PATCH v5 07/36] lockdep: Annotate lockdep assertions for context analysis
From: Bart Van Assche @ 2025-12-19 20:53 UTC (permalink / raw)
  To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
  Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <20251219154418.3592607-8-elver@google.com>

On 12/19/25 7:39 AM, Marco Elver wrote:
> diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> index dd634103b014..621566345406 100644
> --- a/include/linux/lockdep.h
> +++ b/include/linux/lockdep.h
> @@ -282,16 +282,16 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie);
>   	do { WARN_ON_ONCE(debug_locks && !(cond)); } while (0)
>   
>   #define lockdep_assert_held(l)		\
> -	lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
> +	do { lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD); __assume_ctx_lock(l); } while (0)
>   
>   #define lockdep_assert_not_held(l)	\
>   	lockdep_assert(lockdep_is_held(l) != LOCK_STATE_HELD)
>   
>   #define lockdep_assert_held_write(l)	\
> -	lockdep_assert(lockdep_is_held_type(l, 0))
> +	do { lockdep_assert(lockdep_is_held_type(l, 0)); __assume_ctx_lock(l); } while (0)
>   
>   #define lockdep_assert_held_read(l)	\
> -	lockdep_assert(lockdep_is_held_type(l, 1))
> +	do { lockdep_assert(lockdep_is_held_type(l, 1)); __assume_shared_ctx_lock(l); } while (0)
>   
>   #define lockdep_assert_held_once(l)		\
>   	lockdep_assert_once(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
> @@ -389,10 +389,10 @@ extern int lockdep_is_held(const void *);
>   #define lockdep_assert(c)			do { } while (0)
>   #define lockdep_assert_once(c)			do { } while (0)
>   
> -#define lockdep_assert_held(l)			do { (void)(l); } while (0)
> +#define lockdep_assert_held(l)			__assume_ctx_lock(l)
>   #define lockdep_assert_not_held(l)		do { (void)(l); } while (0)
> -#define lockdep_assert_held_write(l)		do { (void)(l); } while (0)
> -#define lockdep_assert_held_read(l)		do { (void)(l); } while (0)
> +#define lockdep_assert_held_write(l)		__assume_ctx_lock(l)
> +#define lockdep_assert_held_read(l)		__assume_shared_ctx_lock(l)
>   #define lockdep_assert_held_once(l)		do { (void)(l); } while (0)
>   #define lockdep_assert_none_held_once()	do { } while (0)

I think these macros should use __must_hold() instead of __assume...().
lockdep_assert_held() emits a runtime warning if 'l' is not held. Hence,
I think that code where lockdep_assert_held() is used should not compile
if it cannot be verified at compile time that 'l' is held.

Thanks,

Bart.



^ permalink raw reply

* Re: [PATCH v8 07/12] KEYS: trusted: Remove dead branch from tpm2_unseal_cmd
From: James Bottomley @ 2025-12-19 20:54 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
	Mimi Zohar, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM,
	open list
In-Reply-To: <20251216092147.2326606-8-jarkko@kernel.org>

On Tue, 2025-12-16 at 11:21 +0200, Jarkko Sakkinen wrote:
> TPM2_Unseal requires TPM2_ST_SESSIONS, and tpm2_unseal_cmd() always
> does set up either password or HMAC session.
> 
> Remove the branch in tpm2_unseal_cmd() conditionally setting
> TPM2_ST_NO_SESSIONS. It is faulty but luckily it is never exercised
> at run-time, and thus does not cause regressions.

Shouldn't that also be

Fixes: b7960b904861 ("tpm2-sessions: Open code tpm_buf_append_hmac_session()")

> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  security/keys/trusted-keys/trusted_tpm2.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c
> b/security/keys/trusted-keys/trusted_tpm2.c
> index d3a5c5f2b926..3666e3e48eab 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -451,10 +451,8 @@ static int tpm2_unseal_cmd(struct tpm_chip
> *chip,
>  			   struct trusted_key_options *options,
>  			   u32 blob_handle)
>  {
> -	struct tpm_header *head;
>  	struct tpm_buf buf;
>  	u16 data_len;
> -	int offset;
>  	u8 *data;
>  	int rc;
>  
> @@ -495,14 +493,8 @@ static int tpm2_unseal_cmd(struct tpm_chip
> *chip,
>  		tpm_buf_append_u16(&buf, options->blobauth_len);
>  		tpm_buf_append(&buf, options->blobauth, options-
> >blobauth_len);
>  
> -		if (tpm2_chip_auth(chip)) {
> +		if (tpm2_chip_auth(chip))

Since the statement above is that the if is always true, why do you
still have it here?

Regards,

James


^ permalink raw reply

* Re: [PATCH v5 17/36] locking/rwsem: Support Clang's context analysis
From: Bart Van Assche @ 2025-12-19 20:55 UTC (permalink / raw)
  To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
  Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <20251219154418.3592607-18-elver@google.com>

On 12/19/25 7:40 AM, Marco Elver wrote:
>   static inline void rwsem_assert_held_nolockdep(const struct rw_semaphore *sem)
> +	__assumes_ctx_lock(sem)
>   {
>   	WARN_ON(atomic_long_read(&sem->count) == RWSEM_UNLOCKED_VALUE);
>   }
>   
>   static inline void rwsem_assert_held_write_nolockdep(const struct rw_semaphore *sem)
> +	__assumes_ctx_lock(sem)
>   {
>   	WARN_ON(!(atomic_long_read(&sem->count) & RWSEM_WRITER_LOCKED));
>   }
> @@ -119,6 +121,7 @@ do {								\
>   	static struct lock_class_key __key;			\
>   								\
>   	__init_rwsem((sem), #sem, &__key);			\
> +	__assume_ctx_lock(sem);					\
>   } while (0)

Just like as for lockdep.h, I think that the above annotations should be 
changed into __must_hold().

Thanks,

Bart.

^ permalink raw reply

* Re: [PATCH v5 18/36] locking/local_lock: Include missing headers
From: Bart Van Assche @ 2025-12-19 20:56 UTC (permalink / raw)
  To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
  Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <20251219154418.3592607-19-elver@google.com>

On 12/19/25 7:40 AM, Marco Elver wrote:
> Including <linux/local_lock.h> into an empty TU will result in the
> compiler complaining:
> 
> ./include/linux/local_lock.h: In function ‘class_local_lock_irqsave_constructor’:
> ./include/linux/local_lock_internal.h:95:17: error: implicit declaration of function ‘local_irq_save’; <...>
>     95 |                 local_irq_save(flags);                          \
>        |                 ^~~~~~~~~~~~~~
> 
> As well as (some architectures only, such as 'sh'):
> 
> ./include/linux/local_lock_internal.h: In function ‘local_lock_acquire’:
> ./include/linux/local_lock_internal.h:33:20: error: ‘current’ undeclared (first use in this function)
>     33 |         l->owner = current;
> 
> Include missing headers to allow including local_lock.h where the
> required headers are not otherwise included.
> 
> Signed-off-by: Marco Elver <elver@google.com>
> ---
>   include/linux/local_lock_internal.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/local_lock_internal.h b/include/linux/local_lock_internal.h
> index 8f82b4eb542f..1a1ea1232add 100644
> --- a/include/linux/local_lock_internal.h
> +++ b/include/linux/local_lock_internal.h
> @@ -4,7 +4,9 @@
>   #endif
>   
>   #include <linux/percpu-defs.h>
> +#include <linux/irqflags.h>
>   #include <linux/lockdep.h>
> +#include <asm/current.h>
>   
>   #ifndef CONFIG_PREEMPT_RT
>   

The abbreviation "TU" is uncommon. Hence, please expand that
abbreviation. Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply

* Re: [PATCH v5 21/36] debugfs: Make debugfs_cancellation a context lock struct
From: Bart Van Assche @ 2025-12-19 21:01 UTC (permalink / raw)
  To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
  Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <20251219154418.3592607-22-elver@google.com>

On 12/19/25 7:40 AM, Marco Elver wrote:
> When compiling include/linux/debugfs.h with CONTEXT_ANALYSIS enabled, we
> can see this error:
> 
> ./include/linux/debugfs.h:239:17: error: use of undeclared identifier 'cancellation'
>    239 | void __acquires(cancellation)
> 
> Move the __acquires(..) attribute after the declaration, so that the
> compiler can see the cancellation function argument, as well as making
> struct debugfs_cancellation a real context lock to benefit from Clang's
> context analysis.
> 
> This change is a preparatory change to allow enabling context analysis
> in subsystems that include the above header.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply

* Re: [PATCH v5 08/36] locking/rwlock, spinlock: Support Clang's context analysis
From: Marco Elver @ 2025-12-19 21:02 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <17723ae6-9611-4731-905c-60dab9fb7102@acm.org>

On Fri, 19 Dec 2025 at 21:26, Bart Van Assche <bvanassche@acm.org> wrote:
> On 12/19/25 7:39 AM, Marco Elver wrote:
> > - extern void do_raw_read_lock(rwlock_t *lock) __acquires(lock);
> > + extern void do_raw_read_lock(rwlock_t *lock) __acquires_shared(lock);
>
> Given the "one change per patch" rule, shouldn't the annotation fixes
> for rwlock operations be moved into a separate patch?
>
> > -typedef struct {
> > +context_lock_struct(rwlock) {
> >       arch_rwlock_t raw_lock;
> >   #ifdef CONFIG_DEBUG_SPINLOCK
> >       unsigned int magic, owner_cpu;
> > @@ -31,7 +31,8 @@ typedef struct {
> >   #ifdef CONFIG_DEBUG_LOCK_ALLOC
> >       struct lockdep_map dep_map;
> >   #endif
> > -} rwlock_t;
> > +};
> > +typedef struct rwlock rwlock_t;
>
> This change introduces a new globally visible "struct rwlock". Although
> I haven't found any existing "struct rwlock" definitions, maybe it's a
> good idea to use a more unique name instead.

This doesn't actually introduce a new globally visible "struct
rwlock", it's already the case before.
An inlined struct definition in a typedef is available by its struct
name, so this is not introducing a new name
(https://godbolt.org/z/Y1jf66e1M).

> > diff --git a/include/linux/spinlock_api_up.h b/include/linux/spinlock_api_up.h
> > index 819aeba1c87e..018f5aabc1be 100644
> > --- a/include/linux/spinlock_api_up.h
> > +++ b/include/linux/spinlock_api_up.h
> > @@ -24,68 +24,77 @@
> >    * flags straight, to suppress compiler warnings of unused lock
> >    * variables, and to add the proper checker annotations:
> >    */
> > -#define ___LOCK(lock) \
> > -  do { __acquire(lock); (void)(lock); } while (0)
> > +#define ___LOCK_void(lock) \
> > +  do { (void)(lock); } while (0)
>
> Instead of introducing a new macro ___LOCK_void(), please expand this
> macro where it is used ((void)(lock)). I think this will make the code
> in this header file easier to read.

If I recall right, we needed this to generalize __LOCK(),
__LOCK_IRQ(), etc. which do preempt_disable(), local_irq_disable() in
the right way, but then need to make sure we call the right
acquire/release helper, which require different cases depending on the
lock kind. Obviously we could just expand all the macros below, but
the current pattern tried to not rewrite this altogether.

There's probably a way this can all be simplified for UP, but maybe a
separate patch. I'd leave it to the locking maintainers which way they
prefer to go.

>     > -#define __LOCK(lock) \
> > -  do { preempt_disable(); ___LOCK(lock); } while (0)
> > +#define ___LOCK_(lock) \
> > +  do { __acquire(lock); ___LOCK_void(lock); } while (0)
>
> Is the macro ___LOCK_() used anywhere? If not, can it be left out?

Yes, it's the default case if __VA_ARGS__ is empty.

> > -#define __LOCK_BH(lock) \
> > -  do { __local_bh_disable_ip(_THIS_IP_, SOFTIRQ_LOCK_OFFSET); ___LOCK(lock); } while (0)
> > +#define ___LOCK_shared(lock) \
> > +  do { __acquire_shared(lock); ___LOCK_void(lock); } while (0)
>
> The introduction of the new macros in this header file make the changes
> hard to follow. Please consider splitting the changes for this header
> file as follows:
> * A first patch that splits ___LOCK() into ___LOCK_exclusive() and
>    ___LOCK_shared().
> * A second patch with the thread-safety annotation changes
>    (__acquire() -> __acquire_shared()).

I've wrangled with this maze of interdependent macros and definitions
for days (though that was earlier in the year), believe me when I say
I tried to split it up. I think the commit message hints at this:

> Add support for Clang's context analysis for raw_spinlock_t,
> spinlock_t, and rwlock. This wholesale conversion is required because
> all three of them are interdependent.

It's like a carefully crafted house of cards: you take one away, the
whole thing breaks apart. If I recall correctly, the main problem was
that as soon as you make one of these a context lock type, and because
they are all interdependent, the compiler will just complain endlessly
about either wrong attributes or incorrectly acquired/released locks
until they are all precisely in the way you see them here.

> >   /* Non PREEMPT_RT kernels map spinlock to raw_spinlock */
> > -typedef struct spinlock {
> > +context_lock_struct(spinlock) {
> >       union {
> >               struct raw_spinlock rlock;
> >
> > @@ -26,7 +26,8 @@ typedef struct spinlock {
> >               };
> >   #endif
> >       };
> > -} spinlock_t;
> > +};
> > +typedef struct spinlock spinlock_t;
>
> Also here, a new global struct name is introduced (spinlock). Maybe the
> name of this new struct should be made more unique?

As above.

^ permalink raw reply

* Re: [PATCH v5 22/36] um: Fix incorrect __acquires/__releases annotations
From: Bart Van Assche @ 2025-12-19 21:05 UTC (permalink / raw)
  To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
  Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu, kernel test robot, Johannes Berg, Tiwei Bie
In-Reply-To: <20251219154418.3592607-23-elver@google.com>

On 12/19/25 7:40 AM, Marco Elver wrote:
> -void enter_turnstile(struct mm_id *mm_id) __acquires(turnstile)
> +struct mutex *__get_turnstile(struct mm_id *mm_id)
>   {
>   	struct mm_context *ctx = container_of(mm_id, struct mm_context, id);
>   
> -	mutex_lock(&ctx->turnstile);
> +	return &ctx->turnstile;
>   }

Many "container_of()" wrappers have "to" in their name. Please follow
that convention and rename this function into e.g. mm_id_to_turnstile().

Thanks,

Bart.

^ permalink raw reply

* Re: [PATCH v5 13/36] bit_spinlock: Support Clang's context analysis
From: Marco Elver @ 2025-12-19 21:09 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <3b070057-5fda-410e-a047-d9061d56a82f@acm.org>

On Fri, 19 Dec 2025 at 21:48, 'Bart Van Assche' via kasan-dev
<kasan-dev@googlegroups.com> wrote:
>
> On 12/19/25 7:40 AM, Marco Elver wrote:
> > +/*
> > + * For static context analysis, we need a unique token for each possible bit
> > + * that can be used as a bit_spinlock. The easiest way to do that is to create a
> > + * fake context that we can cast to with the __bitlock(bitnum, addr) macro
> > + * below, which will give us unique instances for each (bit, addr) pair that the
> > + * static analysis can use.
> > + */
> > +context_lock_struct(__context_bitlock) { };
> > +#define __bitlock(bitnum, addr) (struct __context_bitlock *)(bitnum + (addr))
>
> Will this cause static analyzers to complain about out-of-bounds
> accesses for (bitnum + (addr)), which is equivalent to &(addr)[bitnum]?

Only if they decide to interpret never-executed code (i think the
kernel has various dead code that's optimized out that might trigger
static analyzers if they analyzed it).
But this could probably be improved by using a different idiom, and
using an empty inline function that takes bitnum, addr as args, and
Clang simply takes the call to that function as the context lock
identity.

^ permalink raw reply

* Re: [RFC PATCH v1 3/3] landlock: Fix TCP handling of short AF_UNSPEC addresses
From: Mickaël Salaün @ 2025-12-19 21:07 UTC (permalink / raw)
  To: Matthieu Buffet
  Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
	Konstantin Meskhidze
In-Reply-To: <20251027190726.626244-4-matthieu@buffet.re>

On Mon, Oct 27, 2025 at 08:07:26PM +0100, Matthieu Buffet wrote:
> current_check_access_socket() treats AF_UNSPEC addresses as
> AF_INET ones, and only later adds special case handling to
> allow connect(AF_UNSPEC), and on IPv4 sockets
> bind(AF_UNSPEC+INADDR_ANY).
> This would be fine except AF_UNSPEC addresses can be as
> short as a bare AF_UNSPEC sa_family_t field, and nothing
> more. The AF_INET code path incorrectly enforces a length of
> sizeof(struct sockaddr_in) instead.

Good catch!

> 
> Move AF_UNSPEC edge case handling up inside the switch-case,
> before the address is (potentially incorrectly) treated as
> AF_INET.

And that's cleaner this way too.

> 
> Fixes: fff69fb03dde ("landlock: Support network rules with TCP bind and connect")
> Signed-off-by: Matthieu Buffet <matthieu@buffet.re>

I pushed this series to my -next branch, but moving the tests after the
kernel fix. This makes it possible to cleanly bisect commits.  Thanks!

> ---
>  security/landlock/net.c | 118 +++++++++++++++++++++++-----------------
>  1 file changed, 67 insertions(+), 51 deletions(-)
> 
> diff --git a/security/landlock/net.c b/security/landlock/net.c
> index 1f3915a90a80..e6367e30e5b0 100644
> --- a/security/landlock/net.c
> +++ b/security/landlock/net.c
> @@ -71,6 +71,61 @@ static int current_check_access_socket(struct socket *const sock,
>  
>  	switch (address->sa_family) {
>  	case AF_UNSPEC:
> +		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP) {
> +			/*
> +			 * Connecting to an address with AF_UNSPEC dissolves
> +			 * the TCP association, which have the same effect as
> +			 * closing the connection while retaining the socket
> +			 * object (i.e., the file descriptor).  As for dropping
> +			 * privileges, closing connections is always allowed.
> +			 *
> +			 * For a TCP access control system, this request is
> +			 * legitimate. Let the network stack handle potential
> +			 * inconsistencies and return -EINVAL if needed.
> +			 */
> +			return 0;
> +		} else if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
> +			/*
> +			 * Binding to an AF_UNSPEC address is treated
> +			 * differently by IPv4 and IPv6 sockets. The socket's
> +			 * family may change under our feet due to
> +			 * setsockopt(IPV6_ADDRFORM), but that's ok: we either
> +			 * reject entirely or require
> +			 * %LANDLOCK_ACCESS_NET_BIND_TCP for the given port, so
> +			 * it cannot be used to bypass the policy.
> +			 *
> +			 * IPv4 sockets map AF_UNSPEC to AF_INET for
> +			 * retrocompatibility for bind accesses, only if the
> +			 * address is INADDR_ANY (cf. __inet_bind). IPv6
> +			 * sockets always reject it.
> +			 *
> +			 * Checking the address is required to not wrongfully
> +			 * return -EACCES instead of -EAFNOSUPPORT or -EINVAL.
> +			 * We could return 0 and let the network stack handle
> +			 * these checks, but it is safer to return a proper
> +			 * error and test consistency thanks to kselftest.
> +			 */
> +			if (sock->sk->__sk_common.skc_family == AF_INET) {
> +				const struct sockaddr_in *const sockaddr =
> +					(struct sockaddr_in *)address;
> +
> +				if (addrlen < sizeof(struct sockaddr_in))
> +					return -EINVAL;
> +
> +				if (sockaddr->sin_addr.s_addr !=
> +				    htonl(INADDR_ANY))
> +					return -EAFNOSUPPORT;
> +			} else {
> +				if (addrlen < SIN6_LEN_RFC2133)
> +					return -EINVAL;
> +				else
> +					return -EAFNOSUPPORT;
> +			}
> +		} else {
> +			WARN_ON_ONCE(1);
> +		}
> +		/* Only for bind(AF_UNSPEC+INADDR_ANY) on IPv4 socket. */
> +		fallthrough;
>  	case AF_INET: {
>  		const struct sockaddr_in *addr4;
>  
> @@ -119,57 +174,18 @@ static int current_check_access_socket(struct socket *const sock,
>  		return 0;
>  	}
>  
> -	/* Specific AF_UNSPEC handling. */
> -	if (address->sa_family == AF_UNSPEC) {
> -		/*
> -		 * Connecting to an address with AF_UNSPEC dissolves the TCP
> -		 * association, which have the same effect as closing the
> -		 * connection while retaining the socket object (i.e., the file
> -		 * descriptor).  As for dropping privileges, closing
> -		 * connections is always allowed.
> -		 *
> -		 * For a TCP access control system, this request is legitimate.
> -		 * Let the network stack handle potential inconsistencies and
> -		 * return -EINVAL if needed.
> -		 */
> -		if (access_request == LANDLOCK_ACCESS_NET_CONNECT_TCP)
> -			return 0;
> -
> -		/*
> -		 * For compatibility reason, accept AF_UNSPEC for bind
> -		 * accesses (mapped to AF_INET) only if the address is
> -		 * INADDR_ANY (cf. __inet_bind).  Checking the address is
> -		 * required to not wrongfully return -EACCES instead of
> -		 * -EAFNOSUPPORT.
> -		 *
> -		 * We could return 0 and let the network stack handle these
> -		 * checks, but it is safer to return a proper error and test
> -		 * consistency thanks to kselftest.
> -		 */
> -		if (access_request == LANDLOCK_ACCESS_NET_BIND_TCP) {
> -			/* addrlen has already been checked for AF_UNSPEC. */
> -			const struct sockaddr_in *const sockaddr =
> -				(struct sockaddr_in *)address;
> -
> -			if (sock->sk->__sk_common.skc_family != AF_INET)
> -				return -EINVAL;
> -
> -			if (sockaddr->sin_addr.s_addr != htonl(INADDR_ANY))
> -				return -EAFNOSUPPORT;
> -		}
> -	} else {
> -		/*
> -		 * Checks sa_family consistency to not wrongfully return
> -		 * -EACCES instead of -EINVAL.  Valid sa_family changes are
> -		 * only (from AF_INET or AF_INET6) to AF_UNSPEC.
> -		 *
> -		 * We could return 0 and let the network stack handle this
> -		 * check, but it is safer to return a proper error and test
> -		 * consistency thanks to kselftest.
> -		 */
> -		if (address->sa_family != sock->sk->__sk_common.skc_family)
> -			return -EINVAL;
> -	}
> +	/*
> +	 * Checks sa_family consistency to not wrongfully return
> +	 * -EACCES instead of -EINVAL.  Valid sa_family changes are
> +	 * only (from AF_INET or AF_INET6) to AF_UNSPEC.
> +	 *
> +	 * We could return 0 and let the network stack handle this
> +	 * check, but it is safer to return a proper error and test
> +	 * consistency thanks to kselftest.
> +	 */
> +	if (address->sa_family != sock->sk->__sk_common.skc_family &&
> +	    address->sa_family != AF_UNSPEC)
> +		return -EINVAL;
>  
>  	id.key.data = (__force uintptr_t)port;
>  	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
> -- 
> 2.47.2
> 
> 

^ permalink raw reply

* Re: [PATCH v5 07/36] lockdep: Annotate lockdep assertions for context analysis
From: Marco Elver @ 2025-12-19 21:16 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <cdde6c60-7f6f-4715-a249-5aab39438b57@acm.org>

On Fri, 19 Dec 2025 at 21:54, 'Bart Van Assche' via kasan-dev
<kasan-dev@googlegroups.com> wrote:
>
> On 12/19/25 7:39 AM, Marco Elver wrote:
> > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> > index dd634103b014..621566345406 100644
> > --- a/include/linux/lockdep.h
> > +++ b/include/linux/lockdep.h
> > @@ -282,16 +282,16 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie);
> >       do { WARN_ON_ONCE(debug_locks && !(cond)); } while (0)
> >
> >   #define lockdep_assert_held(l)              \
> > -     lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
> > +     do { lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD); __assume_ctx_lock(l); } while (0)
> >
> >   #define lockdep_assert_not_held(l)  \
> >       lockdep_assert(lockdep_is_held(l) != LOCK_STATE_HELD)
> >
> >   #define lockdep_assert_held_write(l)        \
> > -     lockdep_assert(lockdep_is_held_type(l, 0))
> > +     do { lockdep_assert(lockdep_is_held_type(l, 0)); __assume_ctx_lock(l); } while (0)
> >
> >   #define lockdep_assert_held_read(l) \
> > -     lockdep_assert(lockdep_is_held_type(l, 1))
> > +     do { lockdep_assert(lockdep_is_held_type(l, 1)); __assume_shared_ctx_lock(l); } while (0)
> >
> >   #define lockdep_assert_held_once(l)         \
> >       lockdep_assert_once(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
> > @@ -389,10 +389,10 @@ extern int lockdep_is_held(const void *);
> >   #define lockdep_assert(c)                   do { } while (0)
> >   #define lockdep_assert_once(c)                      do { } while (0)
> >
> > -#define lockdep_assert_held(l)                       do { (void)(l); } while (0)
> > +#define lockdep_assert_held(l)                       __assume_ctx_lock(l)
> >   #define lockdep_assert_not_held(l)          do { (void)(l); } while (0)
> > -#define lockdep_assert_held_write(l)         do { (void)(l); } while (0)
> > -#define lockdep_assert_held_read(l)          do { (void)(l); } while (0)
> > +#define lockdep_assert_held_write(l)         __assume_ctx_lock(l)
> > +#define lockdep_assert_held_read(l)          __assume_shared_ctx_lock(l)
> >   #define lockdep_assert_held_once(l)         do { (void)(l); } while (0)
> >   #define lockdep_assert_none_held_once()     do { } while (0)
>
> I think these macros should use __must_hold() instead of __assume...().
> lockdep_assert_held() emits a runtime warning if 'l' is not held. Hence,
> I think that code where lockdep_assert_held() is used should not compile
> if it cannot be verified at compile time that 'l' is held.

That's not the purpose of this - if a function or variable should have
a lock held, we mark them explicitly with __must_hold() or
__guarded_by(), and we don't really need to use lockdep_assert,
because the compiler helped us out. In an ideal world, every function
or variable that requires a lock held is annotated, and we don't need
to ever worry about explicitly checking if a lock is held (but we'll
be far from that for a while).

The purpose is described in the commit message:

> Presence of these annotations causes the analysis to assume the context
> lock is held after calls to the annotated function, and avoid false
> positives with complex control-flow; [...]

It's basically an escape hatch to defer to dynamic analysis where the
limits of the static analysis are reached. This is also the original
purpose of the "assert"/"assume" attributes:
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#assert-capability-and-assert-shared-capability

Without this escape hatch, and deferral to dynamic analysis, we'd be
stuck in some cases.

^ permalink raw reply

* Re: [PATCH v5 07/36] lockdep: Annotate lockdep assertions for context analysis
From: Bart Van Assche @ 2025-12-19 21:28 UTC (permalink / raw)
  To: Marco Elver
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <CANpmjNPJXVtZgT96PP--eNAkHNOvw1MrYzWt5f2aA0LUeK8iGA@mail.gmail.com>

On 12/19/25 2:16 PM, Marco Elver wrote:
> It's basically an escape hatch to defer to dynamic analysis where the
> limits of the static analysis are reached.

That's not how lockdep_assert_held() is used in the kernel. This macro
is more often than not used to document assumptions that can be verified
at compile time.

This patch seems like a step in the wrong direction to me because it
*suppresses* compile time analysis compile-time analysis is useful. I
think that this patch either should be dropped or that the __assume()
annotations should be changed into __must_hold() annotations.

Bart.

^ permalink raw reply

* Re: [PATCH v5 08/36] locking/rwlock, spinlock: Support Clang's context analysis
From: Bart Van Assche @ 2025-12-19 21:34 UTC (permalink / raw)
  To: Marco Elver
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <CANpmjNO0B_BBse12kAobCRBK0D2pKkSu7pKa5LQAbdzBZa2xcw@mail.gmail.com>

On 12/19/25 2:02 PM, Marco Elver wrote:
> On Fri, 19 Dec 2025 at 21:26, Bart Van Assche <bvanassche@acm.org> wrote:
>> On 12/19/25 7:39 AM, Marco Elver wrote:
>>> - extern void do_raw_read_lock(rwlock_t *lock) __acquires(lock);
>>> + extern void do_raw_read_lock(rwlock_t *lock) __acquires_shared(lock);
>>
>> Given the "one change per patch" rule, shouldn't the annotation fixes
>> for rwlock operations be moved into a separate patch?
>>
>>> -typedef struct {
>>> +context_lock_struct(rwlock) {
>>>        arch_rwlock_t raw_lock;
>>>    #ifdef CONFIG_DEBUG_SPINLOCK
>>>        unsigned int magic, owner_cpu;
>>> @@ -31,7 +31,8 @@ typedef struct {
>>>    #ifdef CONFIG_DEBUG_LOCK_ALLOC
>>>        struct lockdep_map dep_map;
>>>    #endif
>>> -} rwlock_t;
>>> +};
>>> +typedef struct rwlock rwlock_t;
>>
>> This change introduces a new globally visible "struct rwlock". Although
>> I haven't found any existing "struct rwlock" definitions, maybe it's a
>> good idea to use a more unique name instead.
> 
> This doesn't actually introduce a new globally visible "struct
> rwlock", it's already the case before.
> An inlined struct definition in a typedef is available by its struct
> name, so this is not introducing a new name
> (https://godbolt.org/z/Y1jf66e1M).

Please take another look. The godbolt example follows the pattern
"typedef struct name { ... } name_t;". The "name" part is missing from
the rwlock_t definition. This is why I wrote that the above code
introduces a new global struct name.

Bart.

^ permalink raw reply

* Re: [PATCH v5 23/36] compiler-context-analysis: Remove Sparse support
From: Bart Van Assche @ 2025-12-19 21:38 UTC (permalink / raw)
  To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
  Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <20251219154418.3592607-24-elver@google.com>

On 12/19/25 8:40 AM, Marco Elver wrote:
> Remove Sparse support as discussed at [1].

Kernel patch descriptions should be self-contained. In other words, the
conclusion from [1] should be summarized in the patch description
instead of only referring to that discussion with a hyperlink.

Thanks,

Bart.

^ permalink raw reply

* Re: [PATCH v5 24/36] compiler-context-analysis: Remove __cond_lock() function-like helper
From: Bart Van Assche @ 2025-12-19 21:42 UTC (permalink / raw)
  To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
  Cc: David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <20251219154418.3592607-25-elver@google.com>

On 12/19/25 8:40 AM, Marco Elver wrote:
>   Documentation/dev-tools/context-analysis.rst  |  2 -
>   Documentation/mm/process_addrs.rst            |  6 +-
>   .../net/wireless/intel/iwlwifi/iwl-trans.c    |  4 +-
>   .../net/wireless/intel/iwlwifi/iwl-trans.h    |  6 +-
>   .../intel/iwlwifi/pcie/gen1_2/internal.h      |  5 +-
>   .../intel/iwlwifi/pcie/gen1_2/trans.c         |  4 +-
>   include/linux/compiler-context-analysis.h     | 31 ----------
>   include/linux/lockref.h                       |  4 +-
>   include/linux/mm.h                            | 33 ++--------
>   include/linux/rwlock.h                        | 11 +---
>   include/linux/rwlock_api_smp.h                | 14 ++++-
>   include/linux/rwlock_rt.h                     | 21 ++++---
>   include/linux/sched/signal.h                  | 14 +----
>   include/linux/spinlock.h                      | 45 +++++---------
>   include/linux/spinlock_api_smp.h              | 20 ++++++
>   include/linux/spinlock_api_up.h               | 61 ++++++++++++++++---
>   include/linux/spinlock_rt.h                   | 26 ++++----
>   kernel/signal.c                               |  4 +-
>   kernel/time/posix-timers.c                    | 13 +---
>   lib/dec_and_lock.c                            |  8 +--
>   lib/lockref.c                                 |  1 -
>   mm/memory.c                                   |  4 +-
>   mm/pgtable-generic.c                          | 19 +++---
>   tools/include/linux/compiler_types.h          |  2 -

This patch should be split into one patch per subsystem or driver.
E.g. one patch for the iwlwifi driver, another patch for the mm
subsystem, one patch for the rwlock primitive, one patch for the
spinlock primitive, etc.

The tools/include/linux/compiler_types.h change probably should be
left out because it is user space code instead of kernel code and
the rest of the series applies to kernel code only.

Thanks,

Bart.

^ permalink raw reply

* Re: [PATCH v5 07/36] lockdep: Annotate lockdep assertions for context analysis
From: Marco Elver @ 2025-12-19 21:47 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <ecb35204-ea13-488b-8d60-e21d4812902a@gmail.com>

On Fri, 19 Dec 2025 at 22:28, Bart Van Assche <bart.vanassche@gmail.com> wrote:
>
> On 12/19/25 2:16 PM, Marco Elver wrote:
> > It's basically an escape hatch to defer to dynamic analysis where the
> > limits of the static analysis are reached.
>
> That's not how lockdep_assert_held() is used in the kernel.

Because there had not been any static analysis like this, and dynamic
analysis is the only reasonable option.

> This macro
> is more often than not used to document assumptions that can be verified
> at compile time.

In that case the lockdep_assert can be dropped.

> This patch seems like a step in the wrong direction to me because it
> *suppresses* compile time analysis compile-time analysis is useful. I
> think that this patch either should be dropped or that the __assume()
> annotations should be changed into __must_hold() annotations.

If we drop this patch, e.g. the "sched: Enable context analysis for
core.c and fair.c" will no longer compile.

It's a trade-off: more false positives vs. more complete analysis. For
an analysis to be useful, these trade-offs make or break the analysis
depending on the system they are applied to.

In the kernel, our experience with developer tooling has been that any
efforts to reduce false positives will help a tool succeed at scale.
Later you can claw back some completeness, but focusing on
completeness first will kill the tool if false positives cannot
reasonably be dealt with.

From the user space world we know that "assert lock held" [1] as this
kind of escape hatch is valuable to deal with cases the static
analysis just can't deal with. Sure, here we can make our own rules,
but I'd argue we're in a worse position than most user space code, in
that kernel code is significantly more complex (which is the reason I
spent over half a year banging my head to make Clang's analysis
significantly more capable).

[1] https://github.com/abseil/abseil-cpp/blob/a8960c053bf4adadac097c1101d0028742d8042f/absl/synchronization/mutex.h#L210
(ASSERT_EXCLUSIVE_LOCK() == __assume_ctx_lock())

^ permalink raw reply

* Re: [PATCH v5 08/36] locking/rwlock, spinlock: Support Clang's context analysis
From: Marco Elver @ 2025-12-19 21:48 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <0088cc8c-b395-4659-854f-a6cc5df626ed@gmail.com>

On Fri, 19 Dec 2025 at 22:34, Bart Van Assche <bart.vanassche@gmail.com> wrote:
>
> On 12/19/25 2:02 PM, Marco Elver wrote:
> > On Fri, 19 Dec 2025 at 21:26, Bart Van Assche <bvanassche@acm.org> wrote:
> >> On 12/19/25 7:39 AM, Marco Elver wrote:
> >>> - extern void do_raw_read_lock(rwlock_t *lock) __acquires(lock);
> >>> + extern void do_raw_read_lock(rwlock_t *lock) __acquires_shared(lock);
> >>
> >> Given the "one change per patch" rule, shouldn't the annotation fixes
> >> for rwlock operations be moved into a separate patch?
> >>
> >>> -typedef struct {
> >>> +context_lock_struct(rwlock) {
> >>>        arch_rwlock_t raw_lock;
> >>>    #ifdef CONFIG_DEBUG_SPINLOCK
> >>>        unsigned int magic, owner_cpu;
> >>> @@ -31,7 +31,8 @@ typedef struct {
> >>>    #ifdef CONFIG_DEBUG_LOCK_ALLOC
> >>>        struct lockdep_map dep_map;
> >>>    #endif
> >>> -} rwlock_t;
> >>> +};
> >>> +typedef struct rwlock rwlock_t;
> >>
> >> This change introduces a new globally visible "struct rwlock". Although
> >> I haven't found any existing "struct rwlock" definitions, maybe it's a
> >> good idea to use a more unique name instead.
> >
> > This doesn't actually introduce a new globally visible "struct
> > rwlock", it's already the case before.
> > An inlined struct definition in a typedef is available by its struct
> > name, so this is not introducing a new name
> > (https://godbolt.org/z/Y1jf66e1M).
>
> Please take another look. The godbolt example follows the pattern
> "typedef struct name { ... } name_t;". The "name" part is missing from
> the rwlock_t definition. This is why I wrote that the above code
> introduces a new global struct name.

You're right. My point only applies to "typedef struct spinlock ..."

^ permalink raw reply

* Re: [PATCH v5 08/36] locking/rwlock, spinlock: Support Clang's context analysis
From: Bart Van Assche @ 2025-12-19 21:45 UTC (permalink / raw)
  To: Peter Zijlstra, Marco Elver
  Cc: Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
	Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <CANpmjNO0B_BBse12kAobCRBK0D2pKkSu7pKa5LQAbdzBZa2xcw@mail.gmail.com>

On 12/19/25 1:02 PM, Marco Elver wrote:
> On Fri, 19 Dec 2025 at 21:26, Bart Van Assche <bvanassche@acm.org> wrote:
>> On 12/19/25 7:39 AM, Marco Elver wrote:
>>> - extern void do_raw_read_lock(rwlock_t *lock) __acquires(lock);
>>> + extern void do_raw_read_lock(rwlock_t *lock) __acquires_shared(lock);
>>
>> Given the "one change per patch" rule, shouldn't the annotation fixes
>> for rwlock operations be moved into a separate patch?
>>
>>> -typedef struct {
>>> +context_lock_struct(rwlock) {
>>>        arch_rwlock_t raw_lock;
>>>    #ifdef CONFIG_DEBUG_SPINLOCK
>>>        unsigned int magic, owner_cpu;
>>> @@ -31,7 +31,8 @@ typedef struct {
>>>    #ifdef CONFIG_DEBUG_LOCK_ALLOC
>>>        struct lockdep_map dep_map;
>>>    #endif
>>> -} rwlock_t;
>>> +};
>>> +typedef struct rwlock rwlock_t;
>>
>> This change introduces a new globally visible "struct rwlock". Although
>> I haven't found any existing "struct rwlock" definitions, maybe it's a
>> good idea to use a more unique name instead.
> 
> This doesn't actually introduce a new globally visible "struct
> rwlock", it's already the case before.
> An inlined struct definition in a typedef is available by its struct
> name, so this is not introducing a new name
> (https://godbolt.org/z/Y1jf66e1M).

Please take another look. The godbolt example follows the pattern
"typedef struct name { ... } name_t;". The "name" part is missing from
the rwlock_t definition. This is why I wrote that the above code
introduces a new global struct name.

Bart.

^ permalink raw reply

* [PATCH] KEYS: replace -EEXIST with -EBUSY
From: Daniel Gomez @ 2025-12-20  3:50 UTC (permalink / raw)
  To: David Howells, Lukas Wunner, Ignat Korchagin, Herbert Xu,
	David S. Miller, Jarkko Sakkinen, Paul Moore, James Morris,
	Serge E. Hallyn
  Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	Aaron Tomlin, Lucas De Marchi, keyrings, linux-crypto,
	linux-modules, linux-kernel, linux-security-module, Daniel Gomez

From: Daniel Gomez <da.gomez@samsung.com>

The -EEXIST error code is reserved by the module loading infrastructure
to indicate that a module is already loaded. When a module's init
function returns -EEXIST, userspace tools like kmod interpret this as
"module already loaded" and treat the operation as successful, returning
0 to the user even though the module initialization actually failed.

This follows the precedent set by commit 54416fd76770 ("netfilter:
conntrack: helper: Replace -EEXIST by -EBUSY") which fixed the same
issue in nf_conntrack_helper_register().

Affected modules:
  * pkcs8_key_parser x509_key_parser asymmetric_keys dns_resolver
  * nvme_keyring pkcs7_test_key rxrpc turris_signing_key

Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
---
The error code -EEXIST is reserved by the kernel module loader to
indicate that a module with the same name is already loaded. When a
module's init function returns -EEXIST, kmod interprets this as "module
already loaded" and reports success instead of failure [1].

The kernel module loader will include a safety net that provides -EEXIST
to -EBUSY with a warning [2], and a documentation patch has been sent to
prevent future occurrences [3].

These affected code paths were identified using a static analysis tool
[4] that traces -EEXIST returns to module_init(). The tool was developed
with AI assistance and all findings were manually validated.

Link: https://lore.kernel.org/all/aKEVQhJpRdiZSliu@orbyte.nwl.cc/ [1]
Link: https://lore.kernel.org/all/20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com/ [2]
Link: https://lore.kernel.org/all/20251218-dev-module-init-eexists-modules-docs-v1-0-361569aa782a@samsung.com/ [3]
Link: https://gitlab.com/-/snippets/4913469 [4]
---
 crypto/asymmetric_keys/asymmetric_type.c | 2 +-
 security/keys/key.c                      | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 348966ea2175..2c6f3a725102 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -634,7 +634,7 @@ int register_asymmetric_key_parser(struct asymmetric_key_parser *parser)
 		if (strcmp(cursor->name, parser->name) == 0) {
 			pr_err("Asymmetric key parser '%s' already registered\n",
 			       parser->name);
-			ret = -EEXIST;
+			ret = -EBUSY;
 			goto out;
 		}
 	}
diff --git a/security/keys/key.c b/security/keys/key.c
index 3bbdde778631..ed597660f72e 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -1219,7 +1219,7 @@ EXPORT_SYMBOL(generic_key_instantiate);
  *
  * Register a new key type.
  *
- * Returns 0 on success or -EEXIST if a type of this name already exists.
+ * Returns 0 on success or -EBUSY if a type of this name already exists.
  */
 int register_key_type(struct key_type *ktype)
 {
@@ -1228,7 +1228,7 @@ int register_key_type(struct key_type *ktype)
 
 	memset(&ktype->lock_class, 0, sizeof(ktype->lock_class));
 
-	ret = -EEXIST;
+	ret = -EBUSY;
 	down_write(&key_types_sem);
 
 	/* disallow key types with the same name */

---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251218-dev-module-init-eexists-keyring-5b008d7efb40

Best regards,
--  
Daniel Gomez <da.gomez@samsung.com>


^ permalink raw reply related

* Re: [PATCH v5 24/36] compiler-context-analysis: Remove __cond_lock() function-like helper
From: Marco Elver @ 2025-12-20 12:51 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <9af0d949-45f5-45cd-b49d-d45d53f5d8f6@gmail.com>

On Fri, 19 Dec 2025 at 22:42, Bart Van Assche <bart.vanassche@gmail.com> wrote:
> On 12/19/25 8:40 AM, Marco Elver wrote:
> >   Documentation/dev-tools/context-analysis.rst  |  2 -
> >   Documentation/mm/process_addrs.rst            |  6 +-
> >   .../net/wireless/intel/iwlwifi/iwl-trans.c    |  4 +-
> >   .../net/wireless/intel/iwlwifi/iwl-trans.h    |  6 +-
> >   .../intel/iwlwifi/pcie/gen1_2/internal.h      |  5 +-
> >   .../intel/iwlwifi/pcie/gen1_2/trans.c         |  4 +-
> >   include/linux/compiler-context-analysis.h     | 31 ----------
> >   include/linux/lockref.h                       |  4 +-
> >   include/linux/mm.h                            | 33 ++--------
> >   include/linux/rwlock.h                        | 11 +---
> >   include/linux/rwlock_api_smp.h                | 14 ++++-
> >   include/linux/rwlock_rt.h                     | 21 ++++---
> >   include/linux/sched/signal.h                  | 14 +----
> >   include/linux/spinlock.h                      | 45 +++++---------
> >   include/linux/spinlock_api_smp.h              | 20 ++++++
> >   include/linux/spinlock_api_up.h               | 61 ++++++++++++++++---
> >   include/linux/spinlock_rt.h                   | 26 ++++----
> >   kernel/signal.c                               |  4 +-
> >   kernel/time/posix-timers.c                    | 13 +---
> >   lib/dec_and_lock.c                            |  8 +--
> >   lib/lockref.c                                 |  1 -
> >   mm/memory.c                                   |  4 +-
> >   mm/pgtable-generic.c                          | 19 +++---
> >   tools/include/linux/compiler_types.h          |  2 -
>
> This patch should be split into one patch per subsystem or driver.
> E.g. one patch for the iwlwifi driver, another patch for the mm
> subsystem, one patch for the rwlock primitive, one patch for the
> spinlock primitive, etc.
>
> The tools/include/linux/compiler_types.h change probably should be
> left out because it is user space code instead of kernel code and
> the rest of the series applies to kernel code only.

AFAIK, the user space version is just a copy of the kernel version to
support headers that are used by both. See
4bba4c4bb09ad4a2b70836725e08439c86d8f9e4. The sparse annotations were
copied in ab3c0ddb0d71dc214b61d11deb8770196ef46c05.

And there's no point in keeping it around given it's all gone:

% git grep __cond_lock
<nothing>

^ permalink raw reply

* Re: [PATCH v5 17/36] locking/rwsem: Support Clang's context analysis
From: Marco Elver @ 2025-12-20 12:52 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <81d2defc-8980-4022-a464-3d285aff199c@acm.org>

On Fri, 19 Dec 2025 at 21:55, 'Bart Van Assche' via kasan-dev
<kasan-dev@googlegroups.com> wrote:
>
> On 12/19/25 7:40 AM, Marco Elver wrote:
> >   static inline void rwsem_assert_held_nolockdep(const struct rw_semaphore *sem)
> > +     __assumes_ctx_lock(sem)
> >   {
> >       WARN_ON(atomic_long_read(&sem->count) == RWSEM_UNLOCKED_VALUE);
> >   }
> >
> >   static inline void rwsem_assert_held_write_nolockdep(const struct rw_semaphore *sem)
> > +     __assumes_ctx_lock(sem)
> >   {
> >       WARN_ON(!(atomic_long_read(&sem->count) & RWSEM_WRITER_LOCKED));
> >   }
> > @@ -119,6 +121,7 @@ do {                                                              \
> >       static struct lock_class_key __key;                     \
> >                                                               \
> >       __init_rwsem((sem), #sem, &__key);                      \
> > +     __assume_ctx_lock(sem);                                 \
> >   } while (0)
>
> Just like as for lockdep.h, I think that the above annotations should be
> changed into __must_hold().

My point is the same: we use it to delegate to dynamic analysis where
we reach the limits of static analysis, to avoid false positives [1].
Code should apply __must_hold() or __guarded_by() to called or
protected variables respectively, which is both cleaner and the
idiomatic way to use all this.

[1] https://lore.kernel.org/all/CANpmjNPp6Gkz3rdaD0V7EkPrm60sA5tPpw+m8Xg3u8MTXuc2mg@mail.gmail.com/

^ permalink raw reply

* Re: [PATCH v5 02/36] compiler-context-analysis: Add infrastructure for Context Analysis with Clang
From: Peter Zijlstra @ 2025-12-20 13:33 UTC (permalink / raw)
  To: Marco Elver
  Cc: Bart Van Assche, Boqun Feng, Ingo Molnar, Will Deacon,
	David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
	Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
	Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
	Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
	Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
	Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
	Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
	Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
	Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
	kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
	linux-mm, linux-security-module, linux-sparse, linux-wireless,
	llvm, rcu
In-Reply-To: <aUWjfxQ1fIZdxd-C@elver.google.com>

On Fri, Dec 19, 2025 at 08:11:59PM +0100, Marco Elver wrote:

> > Many kernel developers are used to look up the definition of a data
> > structure either by using ctags, etags or a similar tool or by using
> > grep and a pattern like "${struct_name} {\$". Breaking the tools kernel
> > developer use today to look up data structure definitions might cause
> > considerable frustration and hence shouldn't be done lightly.

Its a simple matter of adding a regex to scripts/tags.sh :-) Also clangd
language server sees right through it as is. So all 'modern' stuff using
that will have no problems.


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox