* 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 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 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 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 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 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 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] 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 08/36] locking/rwlock, spinlock: Support Clang's context analysis
From: Bart Van Assche @ 2025-12-19 20:26 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-9-elver@google.com>
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.
> 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.
> -#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?
> -#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()).
> /* 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?
Thanks,
Bart.
^ permalink raw reply
* [PATCH] ima: Fix stack-out-of-bounds in is_bprm_creds_for_exec()
From: Chris J Arges @ 2025-12-19 19:54 UTC (permalink / raw)
To: zohar, roberto.sassu
Cc: kernel-team, Chris J Arges, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn,
Mickaël Salaün, Kees Cook, linux-integrity,
linux-security-module, linux-kernel
KASAN reported a stack-out-of-bounds access in ima_appraise_measurement
from is_bprm_creds_for_exec:
BUG: KASAN: stack-out-of-bounds in ima_appraise_measurement+0x12dc/0x16a0
Read of size 1 at addr ffffc9000160f940 by task sudo/550
The buggy address belongs to stack of task sudo/550
and is located at offset 24 in frame:
ima_appraise_measurement+0x0/0x16a0
This frame has 2 objects:
[48, 56) 'file'
[80, 148) 'hash'
This is caused by using container_of on the *file pointer which by the time
this function is called is actually a stack variable.
In order to fix this pass in a bprm_is_check boolean which can be set
depending on how process_measurement is called. If the caller has a
linux_binprm pointer we can determine is_check and set it then. Otherwise
set it to false.
Fixes: 95b3cdafd7cb7 ("ima: instantiate the bprm_creds_for_exec() hook")
Signed-off-by: Chris J Arges <carges@cloudflare.com>
---
security/integrity/ima/ima.h | 2 +-
security/integrity/ima/ima_appraise.c | 15 ++-------------
security/integrity/ima/ima_main.c | 18 +++++++++---------
3 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e3d71d8d56e3..5c9f244ed1d6 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -441,7 +441,7 @@ int ima_check_blacklist(struct ima_iint_cache *iint,
int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
struct file *file, const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
- int xattr_len, const struct modsig *modsig);
+ int xattr_len, const struct modsig *modsig, bool bprm_is_check);
int ima_must_appraise(struct mnt_idmap *idmap, struct inode *inode,
int mask, enum ima_hooks func);
void ima_update_xattr(struct ima_iint_cache *iint, struct file *file);
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 5149ff4fd50d..ea2079417318 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -470,17 +470,6 @@ int ima_check_blacklist(struct ima_iint_cache *iint,
return rc;
}
-static bool is_bprm_creds_for_exec(enum ima_hooks func, struct file *file)
-{
- struct linux_binprm *bprm;
-
- if (func == BPRM_CHECK) {
- bprm = container_of(&file, struct linux_binprm, file);
- return bprm->is_check;
- }
- return false;
-}
-
/*
* ima_appraise_measurement - appraise file measurement
*
@@ -492,7 +481,7 @@ static bool is_bprm_creds_for_exec(enum ima_hooks func, struct file *file)
int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
struct file *file, const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
- int xattr_len, const struct modsig *modsig)
+ int xattr_len, const struct modsig *modsig, bool bprm_is_check)
{
static const char op[] = "appraise_data";
int audit_msgno = AUDIT_INTEGRITY_DATA;
@@ -514,7 +503,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
* of the script interpreter(userspace). Differentiate kernel and
* userspace enforced integrity audit messages.
*/
- if (is_bprm_creds_for_exec(func, file))
+ if (bprm_is_check)
audit_msgno = AUDIT_INTEGRITY_USERSPACE;
/* If reading the xattr failed and there's no modsig, error out. */
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5770cf691912..955dbaa42f4a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -236,7 +236,7 @@ static void ima_file_free(struct file *file)
static int process_measurement(struct file *file, const struct cred *cred,
struct lsm_prop *prop, char *buf, loff_t size,
int mask, enum ima_hooks func,
- enum kernel_read_file_id read_id)
+ enum kernel_read_file_id read_id, bool bprm_is_check)
{
struct inode *real_inode, *inode = file_inode(file);
struct ima_iint_cache *iint = NULL;
@@ -426,7 +426,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
inode_lock(inode);
rc = ima_appraise_measurement(func, iint, file,
pathname, xattr_value,
- xattr_len, modsig);
+ xattr_len, modsig, bprm_is_check);
inode_unlock(inode);
}
if (!rc)
@@ -493,14 +493,14 @@ static int ima_file_mmap(struct file *file, unsigned long reqprot,
if (reqprot & PROT_EXEC) {
ret = process_measurement(file, current_cred(), &prop, NULL,
- 0, MAY_EXEC, MMAP_CHECK_REQPROT, 0);
+ 0, MAY_EXEC, MMAP_CHECK_REQPROT, 0, false);
if (ret)
return ret;
}
if (prot & PROT_EXEC)
return process_measurement(file, current_cred(), &prop, NULL,
- 0, MAY_EXEC, MMAP_CHECK, 0);
+ 0, MAY_EXEC, MMAP_CHECK, 0, false);
return 0;
}
@@ -584,7 +584,7 @@ static int ima_bprm_check(struct linux_binprm *bprm)
security_current_getlsmprop_subj(&prop);
return process_measurement(bprm->file, current_cred(),
- &prop, NULL, 0, MAY_EXEC, BPRM_CHECK, 0);
+ &prop, NULL, 0, MAY_EXEC, BPRM_CHECK, 0, bprm->is_check);
}
/**
@@ -614,7 +614,7 @@ static int ima_creds_check(struct linux_binprm *bprm, const struct file *file)
security_current_getlsmprop_subj(&prop);
return process_measurement((struct file *)file, bprm->cred, &prop, NULL,
- 0, MAY_EXEC, CREDS_CHECK, 0);
+ 0, MAY_EXEC, CREDS_CHECK, 0, bprm->is_check);
}
/**
@@ -662,7 +662,7 @@ static int ima_file_check(struct file *file, int mask)
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, NULL, 0,
mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
- MAY_APPEND), FILE_CHECK, 0);
+ MAY_APPEND), FILE_CHECK, 0, false);
}
static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
@@ -881,7 +881,7 @@ static int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
func = read_idmap[read_id] ?: FILE_CHECK;
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, NULL, 0,
- MAY_READ, func, 0);
+ MAY_READ, func, 0, false);
}
const int read_idmap[READING_MAX_ID] = {
@@ -925,7 +925,7 @@ static int ima_post_read_file(struct file *file, char *buf, loff_t size,
func = read_idmap[read_id] ?: FILE_CHECK;
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, buf, size,
- MAY_READ, func, read_id);
+ MAY_READ, func, read_id, false);
}
/**
--
2.43.0
^ permalink raw reply related
* [PATCH v1 5/5] landlock: Fix formatting
From: Mickaël Salaün @ 2025-12-19 19:38 UTC (permalink / raw)
Cc: Mickaël Salaün, linux-security-module,
Christian Brauner, Günther Noack, Mateusz Guzik
In-Reply-To: <20251219193855.825889-1-mic@digikod.net>
Format with clang-format -i security/landlock/*.[ch]
Cc: Christian Brauner <brauner@kernel.org>
Cc: Günther Noack <gnoack3000@gmail.com>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Fixes: b4dbfd8653b3 ("Coccinelle-based conversion to use ->i_state accessors")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
security/landlock/fs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index fe794875ad46..e3c3a8a9ac27 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1314,7 +1314,8 @@ static void hook_sb_delete(struct super_block *const sb)
* second call to iput() for the same Landlock object. Also
* checks I_NEW because such inode cannot be tied to an object.
*/
- if (inode_state_read(inode) & (I_FREEING | I_WILL_FREE | I_NEW)) {
+ if (inode_state_read(inode) &
+ (I_FREEING | I_WILL_FREE | I_NEW)) {
spin_unlock(&inode->i_lock);
continue;
}
--
2.52.0
^ permalink raw reply related
* [PATCH v1 4/5] landlock: Fix spelling
From: Mickaël Salaün @ 2025-12-19 19:38 UTC (permalink / raw)
Cc: Mickaël Salaün, linux-security-module,
Günther Noack
In-Reply-To: <20251219193855.825889-1-mic@digikod.net>
Cc: Günther Noack <gnoack3000@gmail.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
security/landlock/domain.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index 7fb70b25f85a..621f054c9a2b 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -97,7 +97,7 @@ struct landlock_hierarchy {
*/
atomic64_t num_denials;
/**
- * @id: Landlock domain ID, sets once at domain creation time.
+ * @id: Landlock domain ID, set once at domain creation time.
*/
u64 id;
/**
--
2.52.0
^ permalink raw reply related
* [PATCH v1 3/5] landlock: Clean up hook_ptrace_access_check()
From: Mickaël Salaün @ 2025-12-19 19:38 UTC (permalink / raw)
Cc: Mickaël Salaün, linux-security-module,
Günther Noack
In-Reply-To: <20251219193855.825889-1-mic@digikod.net>
Make variable's scope minimal in hook_ptrace_access_check().
Cc: Günther Noack <gnoack3000@gmail.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
security/landlock/task.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 2385017418ca..bf4ed15a7f01 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -86,7 +86,6 @@ static int hook_ptrace_access_check(struct task_struct *const child,
const unsigned int mode)
{
const struct landlock_cred_security *parent_subject;
- const struct landlock_ruleset *child_dom;
int err;
/* Quick return for non-landlocked tasks. */
@@ -96,7 +95,8 @@ static int hook_ptrace_access_check(struct task_struct *const child,
scoped_guard(rcu)
{
- child_dom = landlock_get_task_domain(child);
+ const struct landlock_ruleset *const child_dom =
+ landlock_get_task_domain(child);
err = domain_ptrace(parent_subject->domain, child_dom);
}
--
2.52.0
^ permalink raw reply related
* [PATCH v1 2/5] landlock: Improve erratum documentation
From: Mickaël Salaün @ 2025-12-19 19:38 UTC (permalink / raw)
Cc: Mickaël Salaün, linux-security-module,
Günther Noack
In-Reply-To: <20251219193855.825889-1-mic@digikod.net>
Improve description about scoped signal handling.
Reported-by: Günther Noack <gnoack3000@gmail.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
security/landlock/errata/abi-6.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/landlock/errata/abi-6.h b/security/landlock/errata/abi-6.h
index df7bc0e1fdf4..5113a829f87e 100644
--- a/security/landlock/errata/abi-6.h
+++ b/security/landlock/errata/abi-6.h
@@ -9,7 +9,7 @@
* This fix addresses an issue where signal scoping was overly restrictive,
* preventing sandboxed threads from signaling other threads within the same
* process if they belonged to different domains. Because threads are not
- * security boundaries, user space might assume that any thread within the same
+ * security boundaries, user space might assume that all threads within the same
* process can send signals between themselves (see :manpage:`nptl(7)` and
* :manpage:`libpsx(3)`). Consistent with :manpage:`ptrace(2)` behavior, direct
* interaction between threads of the same process should always be allowed.
--
2.52.0
^ permalink raw reply related
* [PATCH v1 1/5] landlock: Remove useless include
From: Mickaël Salaün @ 2025-12-19 19:38 UTC (permalink / raw)
Cc: Mickaël Salaün, linux-security-module,
Günther Noack
Remove useless audit.h include.
Cc: Günther Noack <gnoack@google.com>
Fixes: 33e65b0d3add ("landlock: Add AUDIT_LANDLOCK_ACCESS and log ptrace denials")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
security/landlock/ruleset.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index dfcdc19ea268..0a5b0c76b3f7 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -23,7 +23,6 @@
#include <linux/workqueue.h>
#include "access.h"
-#include "audit.h"
#include "domain.h"
#include "limits.h"
#include "object.h"
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v5 06/36] cleanup: Basic compatibility with context analysis
From: Bart Van Assche @ 2025-12-19 19:16 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-7-elver@google.com>
On 12/19/25 7:39 AM, Marco Elver wrote:
> +#define DECLARE_LOCK_GUARD_1_ATTRS(_name, _lock, _unlock) \
> +static inline class_##_name##_t class_##_name##_constructor(lock_##_name##_t *_T) _lock;\
> +static __always_inline void __class_##_name##_cleanup_ctx(class_##_name##_t **_T) \
> + __no_context_analysis _unlock { }
Elsewhere in the cleanup.h header arguments with the names "_lock" and
"_unlock" hold executable code that perform "lock" and "unlock"
operations respectively, e.g. mutex_lock() and mutex_unlock(). The
DECLARE_LOCK_GUARD_1_ATTRS() "_lock" and "_unlock" arguments however are
function annotations. Please prevent confusion and use other names for
the _lock and _unlock arguments, e.g. _acquire_attr and _release_attr.
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH v5 02/36] compiler-context-analysis: Add infrastructure for Context Analysis with Clang
From: Marco Elver @ 2025-12-19 19:11 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: <2f0c27eb-eca5-4a7f-8035-71c6b0c84e30@acm.org>
On Fri, Dec 19, 2025 at 11:04AM -0800, 'Bart Van Assche' via kasan-dev wrote:
> On 12/19/25 10:59 AM, Marco Elver wrote:
> > On Fri, 19 Dec 2025 at 19:39, 'Bart Van Assche' via kasan-dev
> > <kasan-dev@googlegroups.com> wrote:
> > > I'm concerned that the context_lock_struct() macro will make code harder
> > > to read. Anyone who encounters the context_lock_struct() macro will have
> > > to look up its definition to learn what it does. I propose to split this
> > > macro into two macros:
> > > * One macro that expands into "__ctx_lock_type(name)".
> > > * A second macro that expands into the rest of the above macro.
> > >
> > > In other words, instead of having to write
> > > context_lock_struct(struct_name, { ... }); developers will have to write
> > >
> > > struct context_lock_type struct_name {
> > > ...;
> > > };
> > > context_struct_helper_functions(struct_name);
> >
> > This doesn't necessarily help with not having to look up its
> > definition to learn what it does.
> >
> > If this is the common pattern, it will blindly be repeated, and this
> > adds 1 more line and makes this a bit more verbose. Maybe the helper
> > functions aren't always needed, but I also think that context lock
> > types should remain relatively few. For all synchronization
> > primitives that were enabled in this series, the helpers are required.
> >
> > The current usage is simply:
> >
> > context_lock_struct(name) {
> > ... struct goes here ...
> > }; // note no awkward ) brace
> >
> > I don't know which way the current kernel style is leaning towards,
> > but if we take <linux/cleanup.h> as an example, a simple programming
> > model / API is actually preferred.
> 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.
Fair point. In fact, it's as simple as e.g. (just tested with mutex) as
this:
diff --git a/include/linux/mutex_types.h b/include/linux/mutex_types.h
index 80975935ec48..63ab9e65bb48 100644
--- a/include/linux/mutex_types.h
+++ b/include/linux/mutex_types.h
@@ -38,7 +38,8 @@
* - detects multi-task circular deadlocks and prints out all affected
* locks and tasks (and only those tasks)
*/
-context_lock_struct(mutex) {
+context_lock_struct(mutex);
+struct mutex {
atomic_long_t owner;
raw_spinlock_t wait_lock;
#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
@@ -59,7 +60,8 @@ context_lock_struct(mutex) {
*/
#include <linux/rtmutex.h>
-context_lock_struct(mutex) {
+context_lock_struct(mutex);
+struct mutex {
struct rt_mutex_base rtmutex;
#ifdef CONFIG_DEBUG_LOCK_ALLOC
struct lockdep_map dep_map;
So the existing macro does support both use-cases as-is. I suppose we
could force the above use pattern.
The reason it works, is because it forward-declares the struct anyway to
define the helper functions.
^ permalink raw reply related
* Re: [PATCH v5 02/36] compiler-context-analysis: Add infrastructure for Context Analysis with Clang
From: Bart Van Assche @ 2025-12-19 19:04 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: <CANpmjNM=4baTiSWGOiSWLfQV2YqMt6qkdV__uj+QtD4zAY8Weg@mail.gmail.com>
On 12/19/25 10:59 AM, Marco Elver wrote:
> On Fri, 19 Dec 2025 at 19:39, 'Bart Van Assche' via kasan-dev
> <kasan-dev@googlegroups.com> wrote:
>> I'm concerned that the context_lock_struct() macro will make code harder
>> to read. Anyone who encounters the context_lock_struct() macro will have
>> to look up its definition to learn what it does. I propose to split this
>> macro into two macros:
>> * One macro that expands into "__ctx_lock_type(name)".
>> * A second macro that expands into the rest of the above macro.
>>
>> In other words, instead of having to write
>> context_lock_struct(struct_name, { ... }); developers will have to write
>>
>> struct context_lock_type struct_name {
>> ...;
>> };
>> context_struct_helper_functions(struct_name);
>
> This doesn't necessarily help with not having to look up its
> definition to learn what it does.
>
> If this is the common pattern, it will blindly be repeated, and this
> adds 1 more line and makes this a bit more verbose. Maybe the helper
> functions aren't always needed, but I also think that context lock
> types should remain relatively few. For all synchronization
> primitives that were enabled in this series, the helpers are required.
>
> The current usage is simply:
>
> context_lock_struct(name) {
> ... struct goes here ...
> }; // note no awkward ) brace
>
> I don't know which way the current kernel style is leaning towards,
> but if we take <linux/cleanup.h> as an example, a simple programming
> model / API is actually preferred.
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.
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH v4 06/35] cleanup: Basic compatibility with context analysis
From: Bart Van Assche @ 2025-12-19 18:59 UTC (permalink / raw)
To: Marco Elver, Peter Zijlstra
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: <CANpmjNNm-kbTw46Wh1BJudynHOeLn-Oxew8VuAnCppvV_WtyBw@mail.gmail.com>
On 12/15/25 7:53 AM, Marco Elver wrote:
> For cleanup.h, the problem is that to instantiate we use
> "guard(class)(args..)". If it had been designed as "guard(class,
> args...)", i.e. just use __VA_ARGS__ explicitly instead of the
> implicit 'args...', it might have been possible to add a second
> cleanup variable to do the same (with some additional magic to extract
> the first arg if one exists). Unfortunately, the use of the current
> guard()() idiom has become so pervasive that this is a bigger
> refactor. I'm going to leave cleanup.h as-is for now, if we think we
> want to give this a go in the current state.
Peter, has it already been considered to make the guard() and
scoped_guard() macros more consistent? If there would be agreement that
guard(class)(args..) should be changed into guard(class, args..), I can
help with realizing this conversion.
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH v5 02/36] compiler-context-analysis: Add infrastructure for Context Analysis with Clang
From: Marco Elver @ 2025-12-19 18:59 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: <97e832b7-04a9-49cb-973a-bf9870c21c2f@acm.org>
On Fri, 19 Dec 2025 at 19:39, 'Bart Van Assche' via kasan-dev
<kasan-dev@googlegroups.com> wrote:
> On 12/19/25 7:39 AM, Marco Elver wrote:
> > +#if defined(WARN_CONTEXT_ANALYSIS)
> > +
> > +/*
> > + * These attributes define new context lock (Clang: capability) types.
> > + * Internal only.
> > + */
>
> How can macros be "internal only" that are defined in a header file that
> will be included by almost all kernel code? Please consider changing
> "internal only" into something that is more clear, e.g. "should only be
> used in the macro definitions in this header file".
Sure, comment could be improved.
Let's say they aren't for general use by normal code that just enables
the analysis for checking; for that we define the shorter (retaining
previous names already in use) ones below. But some of these
attributes can and are used by implementing support for some of the
synchronization primitives.
> > +/*
> > + * The below are used to annotate code being checked. Internal only.
> > + */
>
> Same comment here about "internal only".
Sure, can be clarified.
> > +/**
> > + * context_lock_struct() - declare or define a context lock struct
> > + * @name: struct name
> > + *
> > + * Helper to declare or define a struct type that is also a context lock.
> > + *
> > + * .. code-block:: c
> > + *
> > + * context_lock_struct(my_handle) {
> > + * int foo;
> > + * long bar;
> > + * };
> > + *
> > + * struct some_state {
> > + * ...
> > + * };
> > + * // ... declared elsewhere ...
> > + * context_lock_struct(some_state);
> > + *
> > + * Note: The implementation defines several helper functions that can acquire
> > + * and release the context lock.
> > + */
> > +# define context_lock_struct(name, ...) \
> > + struct __ctx_lock_type(name) __VA_ARGS__ name; \
> > + static __always_inline void __acquire_ctx_lock(const struct name *var) \
> > + __attribute__((overloadable)) __no_context_analysis __acquires_ctx_lock(var) { } \
> > + static __always_inline void __acquire_shared_ctx_lock(const struct name *var) \
> > + __attribute__((overloadable)) __no_context_analysis __acquires_shared_ctx_lock(var) { } \
> > + static __always_inline bool __try_acquire_ctx_lock(const struct name *var, bool ret) \
> > + __attribute__((overloadable)) __no_context_analysis __try_acquires_ctx_lock(1, var) \
> > + { return ret; } \
> > + static __always_inline bool __try_acquire_shared_ctx_lock(const struct name *var, bool ret) \
> > + __attribute__((overloadable)) __no_context_analysis __try_acquires_shared_ctx_lock(1, var) \
> > + { return ret; } \
> > + static __always_inline void __release_ctx_lock(const struct name *var) \
> > + __attribute__((overloadable)) __no_context_analysis __releases_ctx_lock(var) { } \
> > + static __always_inline void __release_shared_ctx_lock(const struct name *var) \
> > + __attribute__((overloadable)) __no_context_analysis __releases_shared_ctx_lock(var) { } \
> > + static __always_inline void __assume_ctx_lock(const struct name *var) \
> > + __attribute__((overloadable)) __assumes_ctx_lock(var) { } \
> > + static __always_inline void __assume_shared_ctx_lock(const struct name *var) \
> > + __attribute__((overloadable)) __assumes_shared_ctx_lock(var) { } \
> > + struct name
>
> I'm concerned that the context_lock_struct() macro will make code harder
> to read. Anyone who encounters the context_lock_struct() macro will have
> to look up its definition to learn what it does. I propose to split this
> macro into two macros:
> * One macro that expands into "__ctx_lock_type(name)".
> * A second macro that expands into the rest of the above macro.
>
> In other words, instead of having to write
> context_lock_struct(struct_name, { ... }); developers will have to write
>
> struct context_lock_type struct_name {
> ...;
> };
> context_struct_helper_functions(struct_name);
This doesn't necessarily help with not having to look up its
definition to learn what it does.
If this is the common pattern, it will blindly be repeated, and this
adds 1 more line and makes this a bit more verbose. Maybe the helper
functions aren't always needed, but I also think that context lock
types should remain relatively few. For all synchronization
primitives that were enabled in this series, the helpers are required.
The current usage is simply:
context_lock_struct(name) {
... struct goes here ...
}; // note no awkward ) brace
I don't know which way the current kernel style is leaning towards,
but if we take <linux/cleanup.h> as an example, a simple programming
model / API is actually preferred.
> My opinion is that the alternative that I'm proposing is easier to read.
> Additionally, it doesn't break existing tools that support jumping from
> the name of a struct to its definition, e.g. ctags and etags.
>
> > +config WARN_CONTEXT_ANALYSIS_ALL
> > + bool "Enable context analysis for all source files"
> > + depends on WARN_CONTEXT_ANALYSIS
> > + depends on EXPERT && !COMPILE_TEST
> > + help
> > + Enable tree-wide context analysis. This is likely to produce a
> > + large number of false positives - enable at your own risk.
> > +
> > + If unsure, say N.
>
> Why !COMPILE_TEST?
That's the idiomatic way to prevent this being enabled in allyesconfig
builds, and other compile-only random configs enabling this and then
stumbling over 1000s of warnings.
Thanks,
-- Marco
^ permalink raw reply
* Re: [PATCH v5 04/36] Documentation: Add documentation for Compiler-Based Context Analysis
From: Bart Van Assche @ 2025-12-19 18:51 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-5-elver@google.com>
On 12/19/25 7:39 AM, Marco Elver wrote:
> +Context Analysis is a language extension, which enables statically checking
^
My grammar checker tells me that there shouldn't be a comma here.
However, I'm not sure whether the software that I'm using got this
right.
> +that required contexts are active (or inactive) by acquiring and releasing
> +user-definable "context locks". An obvious application is lock-safety checking
Please improve clarity of this text by adding a definition for "context
lock", e.g. the following: "Each context lock has a name. A state is
associated with each context lock. Supported states are locked,
unlocked, shared locked and exclusive locked. Functions can be annotated
to declare what lock state is expected upon entry and what the lock
state will be upon return. Members of data structures can be annotated
to indicate what context locks should be held upon access."
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH v5 02/36] compiler-context-analysis: Add infrastructure for Context Analysis with Clang
From: Bart Van Assche @ 2025-12-19 18: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-3-elver@google.com>
On 12/19/25 7:39 AM, Marco Elver wrote:
> +#if defined(WARN_CONTEXT_ANALYSIS)
> +
> +/*
> + * These attributes define new context lock (Clang: capability) types.
> + * Internal only.
> + */
How can macros be "internal only" that are defined in a header file that
will be included by almost all kernel code? Please consider changing
"internal only" into something that is more clear, e.g. "should only be
used in the macro definitions in this header file".
> +/*
> + * The below are used to annotate code being checked. Internal only.
> + */
Same comment here about "internal only".
> +/**
> + * context_lock_struct() - declare or define a context lock struct
> + * @name: struct name
> + *
> + * Helper to declare or define a struct type that is also a context lock.
> + *
> + * .. code-block:: c
> + *
> + * context_lock_struct(my_handle) {
> + * int foo;
> + * long bar;
> + * };
> + *
> + * struct some_state {
> + * ...
> + * };
> + * // ... declared elsewhere ...
> + * context_lock_struct(some_state);
> + *
> + * Note: The implementation defines several helper functions that can acquire
> + * and release the context lock.
> + */
> +# define context_lock_struct(name, ...) \
> + struct __ctx_lock_type(name) __VA_ARGS__ name; \
> + static __always_inline void __acquire_ctx_lock(const struct name *var) \
> + __attribute__((overloadable)) __no_context_analysis __acquires_ctx_lock(var) { } \
> + static __always_inline void __acquire_shared_ctx_lock(const struct name *var) \
> + __attribute__((overloadable)) __no_context_analysis __acquires_shared_ctx_lock(var) { } \
> + static __always_inline bool __try_acquire_ctx_lock(const struct name *var, bool ret) \
> + __attribute__((overloadable)) __no_context_analysis __try_acquires_ctx_lock(1, var) \
> + { return ret; } \
> + static __always_inline bool __try_acquire_shared_ctx_lock(const struct name *var, bool ret) \
> + __attribute__((overloadable)) __no_context_analysis __try_acquires_shared_ctx_lock(1, var) \
> + { return ret; } \
> + static __always_inline void __release_ctx_lock(const struct name *var) \
> + __attribute__((overloadable)) __no_context_analysis __releases_ctx_lock(var) { } \
> + static __always_inline void __release_shared_ctx_lock(const struct name *var) \
> + __attribute__((overloadable)) __no_context_analysis __releases_shared_ctx_lock(var) { } \
> + static __always_inline void __assume_ctx_lock(const struct name *var) \
> + __attribute__((overloadable)) __assumes_ctx_lock(var) { } \
> + static __always_inline void __assume_shared_ctx_lock(const struct name *var) \
> + __attribute__((overloadable)) __assumes_shared_ctx_lock(var) { } \
> + struct name
I'm concerned that the context_lock_struct() macro will make code harder
to read. Anyone who encounters the context_lock_struct() macro will have
to look up its definition to learn what it does. I propose to split this
macro into two macros:
* One macro that expands into "__ctx_lock_type(name)".
* A second macro that expands into the rest of the above macro.
In other words, instead of having to write
context_lock_struct(struct_name, { ... }); developers will have to write
struct context_lock_type struct_name {
...;
};
context_struct_helper_functions(struct_name);
My opinion is that the alternative that I'm proposing is easier to read.
Additionally, it doesn't break existing tools that support jumping from
the name of a struct to its definition, e.g. ctags and etags.
> +config WARN_CONTEXT_ANALYSIS_ALL
> + bool "Enable context analysis for all source files"
> + depends on WARN_CONTEXT_ANALYSIS
> + depends on EXPERT && !COMPILE_TEST
> + help
> + Enable tree-wide context analysis. This is likely to produce a
> + large number of false positives - enable at your own risk.
> +
> + If unsure, say N.
Why !COMPILE_TEST?
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH] ipv6: BUG() in pskb_expand_head() as part of calipso_skbuff_setattr()
From: Paul Moore @ 2025-12-19 18:11 UTC (permalink / raw)
To: Will Rosenberg
Cc: security, David S. Miller, David Ahern, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Huw Davies, netdev,
linux-security-module, linux-kernel
In-Reply-To: <20251219173637.797418-1-whrosenb@asu.edu>
On Fri, Dec 19, 2025 at 12:37 PM Will Rosenberg <whrosenb@asu.edu> wrote:
>
> There exists a kernel oops caused by a BUG_ON(nhead < 0) at
> net/core/skbuff.c:2232 in pskb_expand_head().
> This bug is triggered as part of the calipso_skbuff_setattr()
> routine when skb_cow() is passed headroom > INT_MAX
> (i.e. (int)(skb_headroom(skb) + len_delta) < 0).
>
> The root cause of the bug is due to an implicit integer cast in
> __skb_cow(). The check (headroom > skb_headroom(skb)) is meant to ensure
> that delta = headroom - skb_headroom(skb) is never negative, otherwise
> we will trigger a BUG_ON in pskb_expand_head(). However, if
> headroom > INT_MAX and delta <= -NET_SKB_PAD, the check passes, delta
> becomes negative, and pskb_expand_head() is passed a negative value for
> nhead.
>
> Fix the trigger condition in calipso_skbuff_setattr(). Avoid passing
> "negative" headroom sizes to skb_cow() within calipso_skbuff_setattr()
> by only using skb_cow() to grow headroom.
>
> PoC:
> Using `netlabelctl` tool:
>
> netlabelctl map del default
> netlabelctl calipso add pass doi:7
> netlabelctl map add default address:0::1/128 protocol:calipso,7
>
> Then run the following PoC:
>
> int fd = socket(AF_INET6, SOCK_DGRAM, IPPROTO_UDP);
>
> // setup msghdr
> int cmsg_size = 2;
> int cmsg_len = 0x60;
> struct msghdr msg;
> struct sockaddr_in6 dest_addr;
> struct cmsghdr * cmsg = (struct cmsghdr *) calloc(1,
> sizeof(struct cmsghdr) + cmsg_len);
> msg.msg_name = &dest_addr;
> msg.msg_namelen = sizeof(dest_addr);
> msg.msg_iov = NULL;
> msg.msg_iovlen = 0;
> msg.msg_control = cmsg;
> msg.msg_controllen = cmsg_len;
> msg.msg_flags = 0;
>
> // setup sockaddr
> dest_addr.sin6_family = AF_INET6;
> dest_addr.sin6_port = htons(31337);
> dest_addr.sin6_flowinfo = htonl(31337);
> dest_addr.sin6_addr = in6addr_loopback;
> dest_addr.sin6_scope_id = 31337;
>
> // setup cmsghdr
> cmsg->cmsg_len = cmsg_len;
> cmsg->cmsg_level = IPPROTO_IPV6;
> cmsg->cmsg_type = IPV6_HOPOPTS;
> char * hop_hdr = (char *)cmsg + sizeof(struct cmsghdr);
> hop_hdr[1] = 0x9; //set hop size - (0x9 + 1) * 8 = 80
>
> sendmsg(fd, &msg, 0);
>
> Fixes: 2917f57b6bc1 ("calipso: Allow the lsm to label the skbuff directly.")
> Signed-off-by: Will Rosenberg <whrosenb@asu.edu>
> ---
>
> Notes:
> -Changing __skb_cow() would likely require an audit of all its use
> cases due to its long legacy in the kernel. After private discussions,
> it was decided that this patch should be applied to calipso to remedy
> the immediate symptoms and allow for easy backporting. However, net
> devs should consider remedying the root cause through __skb_cow()
> and skb_cow().
>
> -Paul, please let me know if I should add any form of credit for
> the patch code, such as "Suggested-By."
>
> net/ipv6/calipso.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Folks can add my Suggested-by if they like, but I'm not bothered
either way; getting it fixed upstream is the important part. Thanks
for your work on this Will!
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
> index df1986973430..21f6ed126253 100644
> --- a/net/ipv6/calipso.c
> +++ b/net/ipv6/calipso.c
> @@ -1342,7 +1342,8 @@ static int calipso_skbuff_setattr(struct sk_buff *skb,
> /* At this point new_end aligns to 4n, so (new_end & 4) pads to 8n */
> pad = ((new_end & 4) + (end & 7)) & 7;
> len_delta = new_end - (int)end + pad;
> - ret_val = skb_cow(skb, skb_headroom(skb) + len_delta);
> + ret_val = skb_cow(skb,
> + skb_headroom(skb) + (len_delta > 0 ? len_delta : 0));
> if (ret_val < 0)
> return ret_val;
>
>
> base-commit: ea1013c1539270e372fc99854bc6e4d94eaeff66
> --
> 2.34.1
--
paul-moore.com
^ permalink raw reply
* [PATCH] ipv6: BUG() in pskb_expand_head() as part of calipso_skbuff_setattr()
From: Will Rosenberg @ 2025-12-19 17:36 UTC (permalink / raw)
Cc: security, Will Rosenberg, Paul Moore, David S. Miller,
David Ahern, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Huw Davies, netdev, linux-security-module,
linux-kernel
There exists a kernel oops caused by a BUG_ON(nhead < 0) at
net/core/skbuff.c:2232 in pskb_expand_head().
This bug is triggered as part of the calipso_skbuff_setattr()
routine when skb_cow() is passed headroom > INT_MAX
(i.e. (int)(skb_headroom(skb) + len_delta) < 0).
The root cause of the bug is due to an implicit integer cast in
__skb_cow(). The check (headroom > skb_headroom(skb)) is meant to ensure
that delta = headroom - skb_headroom(skb) is never negative, otherwise
we will trigger a BUG_ON in pskb_expand_head(). However, if
headroom > INT_MAX and delta <= -NET_SKB_PAD, the check passes, delta
becomes negative, and pskb_expand_head() is passed a negative value for
nhead.
Fix the trigger condition in calipso_skbuff_setattr(). Avoid passing
"negative" headroom sizes to skb_cow() within calipso_skbuff_setattr()
by only using skb_cow() to grow headroom.
PoC:
Using `netlabelctl` tool:
netlabelctl map del default
netlabelctl calipso add pass doi:7
netlabelctl map add default address:0::1/128 protocol:calipso,7
Then run the following PoC:
int fd = socket(AF_INET6, SOCK_DGRAM, IPPROTO_UDP);
// setup msghdr
int cmsg_size = 2;
int cmsg_len = 0x60;
struct msghdr msg;
struct sockaddr_in6 dest_addr;
struct cmsghdr * cmsg = (struct cmsghdr *) calloc(1,
sizeof(struct cmsghdr) + cmsg_len);
msg.msg_name = &dest_addr;
msg.msg_namelen = sizeof(dest_addr);
msg.msg_iov = NULL;
msg.msg_iovlen = 0;
msg.msg_control = cmsg;
msg.msg_controllen = cmsg_len;
msg.msg_flags = 0;
// setup sockaddr
dest_addr.sin6_family = AF_INET6;
dest_addr.sin6_port = htons(31337);
dest_addr.sin6_flowinfo = htonl(31337);
dest_addr.sin6_addr = in6addr_loopback;
dest_addr.sin6_scope_id = 31337;
// setup cmsghdr
cmsg->cmsg_len = cmsg_len;
cmsg->cmsg_level = IPPROTO_IPV6;
cmsg->cmsg_type = IPV6_HOPOPTS;
char * hop_hdr = (char *)cmsg + sizeof(struct cmsghdr);
hop_hdr[1] = 0x9; //set hop size - (0x9 + 1) * 8 = 80
sendmsg(fd, &msg, 0);
Fixes: 2917f57b6bc1 ("calipso: Allow the lsm to label the skbuff directly.")
Signed-off-by: Will Rosenberg <whrosenb@asu.edu>
---
Notes:
-Changing __skb_cow() would likely require an audit of all its use
cases due to its long legacy in the kernel. After private discussions,
it was decided that this patch should be applied to calipso to remedy
the immediate symptoms and allow for easy backporting. However, net
devs should consider remedying the root cause through __skb_cow()
and skb_cow().
-Paul, please let me know if I should add any form of credit for
the patch code, such as "Suggested-By."
net/ipv6/calipso.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
index df1986973430..21f6ed126253 100644
--- a/net/ipv6/calipso.c
+++ b/net/ipv6/calipso.c
@@ -1342,7 +1342,8 @@ static int calipso_skbuff_setattr(struct sk_buff *skb,
/* At this point new_end aligns to 4n, so (new_end & 4) pads to 8n */
pad = ((new_end & 4) + (end & 7)) & 7;
len_delta = new_end - (int)end + pad;
- ret_val = skb_cow(skb, skb_headroom(skb) + len_delta);
+ ret_val = skb_cow(skb,
+ skb_headroom(skb) + (len_delta > 0 ? len_delta : 0));
if (ret_val < 0)
return ret_val;
base-commit: ea1013c1539270e372fc99854bc6e4d94eaeff66
--
2.34.1
^ permalink raw reply related
* [PATCH v5 36/36] sched: Enable context analysis for core.c and fair.c
From: Marco Elver @ 2025-12-19 15:40 UTC (permalink / raw)
To: 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, Bart Van Assche,
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, Ingo Molnar
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
This demonstrates a larger conversion to use Clang's context
analysis. The benefit is additional static checking of locking rules,
along with better documentation.
Notably, kernel/sched contains sufficiently complex synchronization
patterns, and application to core.c & fair.c demonstrates that the
latest Clang version has become powerful enough to start applying this
to more complex subsystems (with some modest annotations and changes).
Signed-off-by: Marco Elver <elver@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
---
v5:
* Rename "context guard" -> "context lock".
* Use new cleanup.h helpers to properly support scoped lock guards.
v4:
* Rename capability -> context analysis.
v3:
* New patch.
---
include/linux/sched.h | 6 +-
include/linux/sched/signal.h | 4 +-
include/linux/sched/task.h | 6 +-
include/linux/sched/wake_q.h | 3 +
kernel/sched/Makefile | 3 +
kernel/sched/core.c | 89 +++++++++++-----
kernel/sched/fair.c | 7 +-
kernel/sched/sched.h | 126 ++++++++++++++++-------
scripts/context-analysis-suppression.txt | 1 +
9 files changed, 177 insertions(+), 68 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d395f2810fac..c4022647282e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2094,9 +2094,9 @@ static inline int _cond_resched(void)
_cond_resched(); \
})
-extern int __cond_resched_lock(spinlock_t *lock);
-extern int __cond_resched_rwlock_read(rwlock_t *lock);
-extern int __cond_resched_rwlock_write(rwlock_t *lock);
+extern int __cond_resched_lock(spinlock_t *lock) __must_hold(lock);
+extern int __cond_resched_rwlock_read(rwlock_t *lock) __must_hold_shared(lock);
+extern int __cond_resched_rwlock_write(rwlock_t *lock) __must_hold(lock);
#define MIGHT_RESCHED_RCU_SHIFT 8
#define MIGHT_RESCHED_PREEMPT_MASK ((1U << MIGHT_RESCHED_RCU_SHIFT) - 1)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index a63f65aa5bdd..a22248aebcf9 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -738,10 +738,12 @@ static inline int thread_group_empty(struct task_struct *p)
(thread_group_leader(p) && !thread_group_empty(p))
extern struct sighand_struct *lock_task_sighand(struct task_struct *task,
- unsigned long *flags);
+ unsigned long *flags)
+ __acquires(&task->sighand->siglock);
static inline void unlock_task_sighand(struct task_struct *task,
unsigned long *flags)
+ __releases(&task->sighand->siglock)
{
spin_unlock_irqrestore(&task->sighand->siglock, *flags);
}
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 525aa2a632b2..41ed884cffc9 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -214,15 +214,19 @@ static inline struct vm_struct *task_stack_vm_area(const struct task_struct *t)
* write_lock_irq(&tasklist_lock), neither inside nor outside.
*/
static inline void task_lock(struct task_struct *p)
+ __acquires(&p->alloc_lock)
{
spin_lock(&p->alloc_lock);
}
static inline void task_unlock(struct task_struct *p)
+ __releases(&p->alloc_lock)
{
spin_unlock(&p->alloc_lock);
}
-DEFINE_GUARD(task_lock, struct task_struct *, task_lock(_T), task_unlock(_T))
+DEFINE_LOCK_GUARD_1(task_lock, struct task_struct, task_lock(_T->lock), task_unlock(_T->lock))
+DECLARE_LOCK_GUARD_1_ATTRS(task_lock, __acquires(&_T->alloc_lock), __releases(&(*(struct task_struct **)_T)->alloc_lock))
+#define class_task_lock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(task_lock, _T)
#endif /* _LINUX_SCHED_TASK_H */
diff --git a/include/linux/sched/wake_q.h b/include/linux/sched/wake_q.h
index 0f28b4623ad4..765bbc3d54be 100644
--- a/include/linux/sched/wake_q.h
+++ b/include/linux/sched/wake_q.h
@@ -66,6 +66,7 @@ extern void wake_up_q(struct wake_q_head *head);
/* Spin unlock helpers to unlock and call wake_up_q with preempt disabled */
static inline
void raw_spin_unlock_wake(raw_spinlock_t *lock, struct wake_q_head *wake_q)
+ __releases(lock)
{
guard(preempt)();
raw_spin_unlock(lock);
@@ -77,6 +78,7 @@ void raw_spin_unlock_wake(raw_spinlock_t *lock, struct wake_q_head *wake_q)
static inline
void raw_spin_unlock_irq_wake(raw_spinlock_t *lock, struct wake_q_head *wake_q)
+ __releases(lock)
{
guard(preempt)();
raw_spin_unlock_irq(lock);
@@ -89,6 +91,7 @@ void raw_spin_unlock_irq_wake(raw_spinlock_t *lock, struct wake_q_head *wake_q)
static inline
void raw_spin_unlock_irqrestore_wake(raw_spinlock_t *lock, unsigned long flags,
struct wake_q_head *wake_q)
+ __releases(lock)
{
guard(preempt)();
raw_spin_unlock_irqrestore(lock, flags);
diff --git a/kernel/sched/Makefile b/kernel/sched/Makefile
index 8ae86371ddcd..b1f1a367034f 100644
--- a/kernel/sched/Makefile
+++ b/kernel/sched/Makefile
@@ -1,5 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS_core.o := y
+CONTEXT_ANALYSIS_fair.o := y
+
# The compilers are complaining about unused variables inside an if(0) scope
# block. This is daft, shut them up.
ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 41ba0be16911..ae543ee91272 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -396,6 +396,8 @@ static atomic_t sched_core_count;
static struct cpumask sched_core_mask;
static void sched_core_lock(int cpu, unsigned long *flags)
+ __context_unsafe(/* acquires multiple */)
+ __acquires(&runqueues.__lock) /* overapproximation */
{
const struct cpumask *smt_mask = cpu_smt_mask(cpu);
int t, i = 0;
@@ -406,6 +408,8 @@ static void sched_core_lock(int cpu, unsigned long *flags)
}
static void sched_core_unlock(int cpu, unsigned long *flags)
+ __context_unsafe(/* releases multiple */)
+ __releases(&runqueues.__lock) /* overapproximation */
{
const struct cpumask *smt_mask = cpu_smt_mask(cpu);
int t;
@@ -630,6 +634,7 @@ EXPORT_SYMBOL(__trace_set_current_state);
*/
void raw_spin_rq_lock_nested(struct rq *rq, int subclass)
+ __context_unsafe()
{
raw_spinlock_t *lock;
@@ -655,6 +660,7 @@ void raw_spin_rq_lock_nested(struct rq *rq, int subclass)
}
bool raw_spin_rq_trylock(struct rq *rq)
+ __context_unsafe()
{
raw_spinlock_t *lock;
bool ret;
@@ -696,15 +702,16 @@ void double_rq_lock(struct rq *rq1, struct rq *rq2)
raw_spin_rq_lock(rq1);
if (__rq_lockp(rq1) != __rq_lockp(rq2))
raw_spin_rq_lock_nested(rq2, SINGLE_DEPTH_NESTING);
+ else
+ __acquire_ctx_lock(__rq_lockp(rq2)); /* fake acquire */
double_rq_clock_clear_update(rq1, rq2);
}
/*
- * __task_rq_lock - lock the rq @p resides on.
+ * ___task_rq_lock - lock the rq @p resides on.
*/
-struct rq *__task_rq_lock(struct task_struct *p, struct rq_flags *rf)
- __acquires(rq->lock)
+struct rq *___task_rq_lock(struct task_struct *p, struct rq_flags *rf)
{
struct rq *rq;
@@ -727,9 +734,7 @@ struct rq *__task_rq_lock(struct task_struct *p, struct rq_flags *rf)
/*
* task_rq_lock - lock p->pi_lock and lock the rq @p resides on.
*/
-struct rq *task_rq_lock(struct task_struct *p, struct rq_flags *rf)
- __acquires(p->pi_lock)
- __acquires(rq->lock)
+struct rq *_task_rq_lock(struct task_struct *p, struct rq_flags *rf)
{
struct rq *rq;
@@ -2431,6 +2436,7 @@ static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
*/
static struct rq *move_queued_task(struct rq *rq, struct rq_flags *rf,
struct task_struct *p, int new_cpu)
+ __must_hold(__rq_lockp(rq))
{
lockdep_assert_rq_held(rq);
@@ -2477,6 +2483,7 @@ struct set_affinity_pending {
*/
static struct rq *__migrate_task(struct rq *rq, struct rq_flags *rf,
struct task_struct *p, int dest_cpu)
+ __must_hold(__rq_lockp(rq))
{
/* Affinity changed (again). */
if (!is_cpu_allowed(p, dest_cpu))
@@ -2513,6 +2520,12 @@ static int migration_cpu_stop(void *data)
*/
flush_smp_call_function_queue();
+ /*
+ * We may change the underlying rq, but the locks held will
+ * appropriately be "transferred" when switching.
+ */
+ context_unsafe_alias(rq);
+
raw_spin_lock(&p->pi_lock);
rq_lock(rq, &rf);
@@ -2624,6 +2637,8 @@ int push_cpu_stop(void *arg)
if (!lowest_rq)
goto out_unlock;
+ lockdep_assert_rq_held(lowest_rq);
+
// XXX validate p is still the highest prio task
if (task_rq(p) == rq) {
move_queued_task_locked(rq, lowest_rq, p);
@@ -2834,8 +2849,7 @@ void release_user_cpus_ptr(struct task_struct *p)
*/
static int affine_move_task(struct rq *rq, struct task_struct *p, struct rq_flags *rf,
int dest_cpu, unsigned int flags)
- __releases(rq->lock)
- __releases(p->pi_lock)
+ __releases(__rq_lockp(rq), &p->pi_lock)
{
struct set_affinity_pending my_pending = { }, *pending = NULL;
bool stop_pending, complete = false;
@@ -2990,8 +3004,7 @@ static int __set_cpus_allowed_ptr_locked(struct task_struct *p,
struct affinity_context *ctx,
struct rq *rq,
struct rq_flags *rf)
- __releases(rq->lock)
- __releases(p->pi_lock)
+ __releases(__rq_lockp(rq), &p->pi_lock)
{
const struct cpumask *cpu_allowed_mask = task_cpu_possible_mask(p);
const struct cpumask *cpu_valid_mask = cpu_active_mask;
@@ -4273,29 +4286,30 @@ static bool __task_needs_rq_lock(struct task_struct *p)
*/
int task_call_func(struct task_struct *p, task_call_f func, void *arg)
{
- struct rq *rq = NULL;
struct rq_flags rf;
int ret;
raw_spin_lock_irqsave(&p->pi_lock, rf.flags);
- if (__task_needs_rq_lock(p))
- rq = __task_rq_lock(p, &rf);
+ if (__task_needs_rq_lock(p)) {
+ struct rq *rq = __task_rq_lock(p, &rf);
- /*
- * At this point the task is pinned; either:
- * - blocked and we're holding off wakeups (pi->lock)
- * - woken, and we're holding off enqueue (rq->lock)
- * - queued, and we're holding off schedule (rq->lock)
- * - running, and we're holding off de-schedule (rq->lock)
- *
- * The called function (@func) can use: task_curr(), p->on_rq and
- * p->__state to differentiate between these states.
- */
- ret = func(p, arg);
+ /*
+ * At this point the task is pinned; either:
+ * - blocked and we're holding off wakeups (pi->lock)
+ * - woken, and we're holding off enqueue (rq->lock)
+ * - queued, and we're holding off schedule (rq->lock)
+ * - running, and we're holding off de-schedule (rq->lock)
+ *
+ * The called function (@func) can use: task_curr(), p->on_rq and
+ * p->__state to differentiate between these states.
+ */
+ ret = func(p, arg);
- if (rq)
__task_rq_unlock(rq, p, &rf);
+ } else {
+ ret = func(p, arg);
+ }
raw_spin_unlock_irqrestore(&p->pi_lock, rf.flags);
return ret;
@@ -4968,6 +4982,8 @@ void balance_callbacks(struct rq *rq, struct balance_callback *head)
static inline void
prepare_lock_switch(struct rq *rq, struct task_struct *next, struct rq_flags *rf)
+ __releases(__rq_lockp(rq))
+ __acquires(__rq_lockp(this_rq()))
{
/*
* Since the runqueue lock will be released by the next
@@ -4981,9 +4997,15 @@ prepare_lock_switch(struct rq *rq, struct task_struct *next, struct rq_flags *rf
/* this is a valid case when another task releases the spinlock */
rq_lockp(rq)->owner = next;
#endif
+ /*
+ * Model the rq reference switcheroo.
+ */
+ __release(__rq_lockp(rq));
+ __acquire(__rq_lockp(this_rq()));
}
static inline void finish_lock_switch(struct rq *rq)
+ __releases(__rq_lockp(rq))
{
/*
* If we are tracking spinlock dependencies then we have to
@@ -5039,6 +5061,7 @@ static inline void kmap_local_sched_in(void)
static inline void
prepare_task_switch(struct rq *rq, struct task_struct *prev,
struct task_struct *next)
+ __must_hold(__rq_lockp(rq))
{
kcov_prepare_switch(prev);
sched_info_switch(rq, prev, next);
@@ -5069,7 +5092,7 @@ prepare_task_switch(struct rq *rq, struct task_struct *prev,
* because prev may have moved to another CPU.
*/
static struct rq *finish_task_switch(struct task_struct *prev)
- __releases(rq->lock)
+ __releases(__rq_lockp(this_rq()))
{
struct rq *rq = this_rq();
struct mm_struct *mm = rq->prev_mm;
@@ -5165,7 +5188,7 @@ static struct rq *finish_task_switch(struct task_struct *prev)
* @prev: the thread we just switched away from.
*/
asmlinkage __visible void schedule_tail(struct task_struct *prev)
- __releases(rq->lock)
+ __releases(__rq_lockp(this_rq()))
{
/*
* New tasks start with FORK_PREEMPT_COUNT, see there and
@@ -5197,6 +5220,7 @@ asmlinkage __visible void schedule_tail(struct task_struct *prev)
static __always_inline struct rq *
context_switch(struct rq *rq, struct task_struct *prev,
struct task_struct *next, struct rq_flags *rf)
+ __releases(__rq_lockp(rq))
{
prepare_task_switch(rq, prev, next);
@@ -5865,6 +5889,7 @@ static void prev_balance(struct rq *rq, struct task_struct *prev,
*/
static inline struct task_struct *
__pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
+ __must_hold(__rq_lockp(rq))
{
const struct sched_class *class;
struct task_struct *p;
@@ -5965,6 +5990,7 @@ static void queue_core_balance(struct rq *rq);
static struct task_struct *
pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
+ __must_hold(__rq_lockp(rq))
{
struct task_struct *next, *p, *max;
const struct cpumask *smt_mask;
@@ -6273,6 +6299,7 @@ static bool steal_cookie_task(int cpu, struct sched_domain *sd)
}
static void sched_core_balance(struct rq *rq)
+ __must_hold(__rq_lockp(rq))
{
struct sched_domain *sd;
int cpu = cpu_of(rq);
@@ -6418,6 +6445,7 @@ static inline void sched_core_cpu_dying(unsigned int cpu) {}
static struct task_struct *
pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
+ __must_hold(__rq_lockp(rq))
{
return __pick_next_task(rq, prev, rf);
}
@@ -8043,6 +8071,12 @@ static int __balance_push_cpu_stop(void *arg)
int cpu;
scoped_guard (raw_spinlock_irq, &p->pi_lock) {
+ /*
+ * We may change the underlying rq, but the locks held will
+ * appropriately be "transferred" when switching.
+ */
+ context_unsafe_alias(rq);
+
cpu = select_fallback_rq(rq->cpu, p);
rq_lock(rq, &rf);
@@ -8066,6 +8100,7 @@ static DEFINE_PER_CPU(struct cpu_stop_work, push_work);
* effective when the hotplug motion is down.
*/
static void balance_push(struct rq *rq)
+ __must_hold(__rq_lockp(rq))
{
struct task_struct *push_task = rq->curr;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index da46c3164537..d0c929ecdb6a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2860,6 +2860,7 @@ static int preferred_group_nid(struct task_struct *p, int nid)
}
static void task_numa_placement(struct task_struct *p)
+ __context_unsafe(/* conditional locking */)
{
int seq, nid, max_nid = NUMA_NO_NODE;
unsigned long max_faults = 0;
@@ -4781,7 +4782,8 @@ static inline unsigned long cfs_rq_load_avg(struct cfs_rq *cfs_rq)
return cfs_rq->avg.load_avg;
}
-static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf);
+static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
+ __must_hold(__rq_lockp(this_rq));
static inline unsigned long task_util(struct task_struct *p)
{
@@ -6188,6 +6190,7 @@ static bool distribute_cfs_runtime(struct cfs_bandwidth *cfs_b)
* used to track this state.
*/
static int do_sched_cfs_period_timer(struct cfs_bandwidth *cfs_b, int overrun, unsigned long flags)
+ __must_hold(&cfs_b->lock)
{
int throttled;
@@ -8919,6 +8922,7 @@ static void set_next_task_fair(struct rq *rq, struct task_struct *p, bool first)
struct task_struct *
pick_next_task_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
+ __must_hold(__rq_lockp(rq))
{
struct sched_entity *se;
struct task_struct *p;
@@ -12858,6 +12862,7 @@ static inline void nohz_newidle_balance(struct rq *this_rq) { }
* > 0 - success, new (fair) tasks present
*/
static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
+ __must_hold(__rq_lockp(this_rq))
{
unsigned long next_balance = jiffies + HZ;
int this_cpu = this_rq->cpu;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index d30cca6870f5..25d2ff265227 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1358,8 +1358,13 @@ static inline u32 sched_rng(void)
return prandom_u32_state(this_cpu_ptr(&sched_rnd_state));
}
+static __always_inline struct rq *__this_rq(void)
+{
+ return this_cpu_ptr(&runqueues);
+}
+
#define cpu_rq(cpu) (&per_cpu(runqueues, (cpu)))
-#define this_rq() this_cpu_ptr(&runqueues)
+#define this_rq() __this_rq()
#define task_rq(p) cpu_rq(task_cpu(p))
#define cpu_curr(cpu) (cpu_rq(cpu)->curr)
#define raw_rq() raw_cpu_ptr(&runqueues)
@@ -1404,6 +1409,7 @@ static inline raw_spinlock_t *rq_lockp(struct rq *rq)
}
static inline raw_spinlock_t *__rq_lockp(struct rq *rq)
+ __returns_ctx_lock(rq_lockp(rq)) /* alias them */
{
if (rq->core_enabled)
return &rq->core->__lock;
@@ -1503,6 +1509,7 @@ static inline raw_spinlock_t *rq_lockp(struct rq *rq)
}
static inline raw_spinlock_t *__rq_lockp(struct rq *rq)
+ __returns_ctx_lock(rq_lockp(rq)) /* alias them */
{
return &rq->__lock;
}
@@ -1545,32 +1552,42 @@ static inline bool rt_group_sched_enabled(void)
#endif /* !CONFIG_RT_GROUP_SCHED */
static inline void lockdep_assert_rq_held(struct rq *rq)
+ __assumes_ctx_lock(__rq_lockp(rq))
{
lockdep_assert_held(__rq_lockp(rq));
}
-extern void raw_spin_rq_lock_nested(struct rq *rq, int subclass);
-extern bool raw_spin_rq_trylock(struct rq *rq);
-extern void raw_spin_rq_unlock(struct rq *rq);
+extern void raw_spin_rq_lock_nested(struct rq *rq, int subclass)
+ __acquires(__rq_lockp(rq));
+
+extern bool raw_spin_rq_trylock(struct rq *rq)
+ __cond_acquires(true, __rq_lockp(rq));
+
+extern void raw_spin_rq_unlock(struct rq *rq)
+ __releases(__rq_lockp(rq));
static inline void raw_spin_rq_lock(struct rq *rq)
+ __acquires(__rq_lockp(rq))
{
raw_spin_rq_lock_nested(rq, 0);
}
static inline void raw_spin_rq_lock_irq(struct rq *rq)
+ __acquires(__rq_lockp(rq))
{
local_irq_disable();
raw_spin_rq_lock(rq);
}
static inline void raw_spin_rq_unlock_irq(struct rq *rq)
+ __releases(__rq_lockp(rq))
{
raw_spin_rq_unlock(rq);
local_irq_enable();
}
static inline unsigned long _raw_spin_rq_lock_irqsave(struct rq *rq)
+ __acquires(__rq_lockp(rq))
{
unsigned long flags;
@@ -1581,6 +1598,7 @@ static inline unsigned long _raw_spin_rq_lock_irqsave(struct rq *rq)
}
static inline void raw_spin_rq_unlock_irqrestore(struct rq *rq, unsigned long flags)
+ __releases(__rq_lockp(rq))
{
raw_spin_rq_unlock(rq);
local_irq_restore(flags);
@@ -1829,18 +1847,16 @@ static inline void rq_repin_lock(struct rq *rq, struct rq_flags *rf)
rq->clock_update_flags |= rf->clock_update_flags;
}
-extern
-struct rq *__task_rq_lock(struct task_struct *p, struct rq_flags *rf)
- __acquires(rq->lock);
+#define __task_rq_lock(...) __acquire_ret(___task_rq_lock(__VA_ARGS__), __rq_lockp(__ret))
+extern struct rq *___task_rq_lock(struct task_struct *p, struct rq_flags *rf) __acquires_ret;
-extern
-struct rq *task_rq_lock(struct task_struct *p, struct rq_flags *rf)
- __acquires(p->pi_lock)
- __acquires(rq->lock);
+#define task_rq_lock(...) __acquire_ret(_task_rq_lock(__VA_ARGS__), __rq_lockp(__ret))
+extern struct rq *_task_rq_lock(struct task_struct *p, struct rq_flags *rf)
+ __acquires(&p->pi_lock) __acquires_ret;
static inline void
__task_rq_unlock(struct rq *rq, struct task_struct *p, struct rq_flags *rf)
- __releases(rq->lock)
+ __releases(__rq_lockp(rq))
{
rq_unpin_lock(rq, rf);
raw_spin_rq_unlock(rq);
@@ -1848,8 +1864,7 @@ __task_rq_unlock(struct rq *rq, struct task_struct *p, struct rq_flags *rf)
static inline void
task_rq_unlock(struct rq *rq, struct task_struct *p, struct rq_flags *rf)
- __releases(rq->lock)
- __releases(p->pi_lock)
+ __releases(__rq_lockp(rq), &p->pi_lock)
{
__task_rq_unlock(rq, p, rf);
raw_spin_unlock_irqrestore(&p->pi_lock, rf->flags);
@@ -1859,6 +1874,8 @@ DEFINE_LOCK_GUARD_1(task_rq_lock, struct task_struct,
_T->rq = task_rq_lock(_T->lock, &_T->rf),
task_rq_unlock(_T->rq, _T->lock, &_T->rf),
struct rq *rq; struct rq_flags rf)
+DECLARE_LOCK_GUARD_1_ATTRS(task_rq_lock, __acquires(_T->pi_lock), __releases((*(struct task_struct **)_T)->pi_lock))
+#define class_task_rq_lock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(task_rq_lock, _T)
DEFINE_LOCK_GUARD_1(__task_rq_lock, struct task_struct,
_T->rq = __task_rq_lock(_T->lock, &_T->rf),
@@ -1866,42 +1883,42 @@ DEFINE_LOCK_GUARD_1(__task_rq_lock, struct task_struct,
struct rq *rq; struct rq_flags rf)
static inline void rq_lock_irqsave(struct rq *rq, struct rq_flags *rf)
- __acquires(rq->lock)
+ __acquires(__rq_lockp(rq))
{
raw_spin_rq_lock_irqsave(rq, rf->flags);
rq_pin_lock(rq, rf);
}
static inline void rq_lock_irq(struct rq *rq, struct rq_flags *rf)
- __acquires(rq->lock)
+ __acquires(__rq_lockp(rq))
{
raw_spin_rq_lock_irq(rq);
rq_pin_lock(rq, rf);
}
static inline void rq_lock(struct rq *rq, struct rq_flags *rf)
- __acquires(rq->lock)
+ __acquires(__rq_lockp(rq))
{
raw_spin_rq_lock(rq);
rq_pin_lock(rq, rf);
}
static inline void rq_unlock_irqrestore(struct rq *rq, struct rq_flags *rf)
- __releases(rq->lock)
+ __releases(__rq_lockp(rq))
{
rq_unpin_lock(rq, rf);
raw_spin_rq_unlock_irqrestore(rq, rf->flags);
}
static inline void rq_unlock_irq(struct rq *rq, struct rq_flags *rf)
- __releases(rq->lock)
+ __releases(__rq_lockp(rq))
{
rq_unpin_lock(rq, rf);
raw_spin_rq_unlock_irq(rq);
}
static inline void rq_unlock(struct rq *rq, struct rq_flags *rf)
- __releases(rq->lock)
+ __releases(__rq_lockp(rq))
{
rq_unpin_lock(rq, rf);
raw_spin_rq_unlock(rq);
@@ -1912,18 +1929,27 @@ DEFINE_LOCK_GUARD_1(rq_lock, struct rq,
rq_unlock(_T->lock, &_T->rf),
struct rq_flags rf)
+DECLARE_LOCK_GUARD_1_ATTRS(rq_lock, __acquires(__rq_lockp(_T)), __releases(__rq_lockp(*(struct rq **)_T)));
+#define class_rq_lock_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rq_lock, _T)
+
DEFINE_LOCK_GUARD_1(rq_lock_irq, struct rq,
rq_lock_irq(_T->lock, &_T->rf),
rq_unlock_irq(_T->lock, &_T->rf),
struct rq_flags rf)
+DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irq, __acquires(__rq_lockp(_T)), __releases(__rq_lockp(*(struct rq **)_T)));
+#define class_rq_lock_irq_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rq_lock_irq, _T)
+
DEFINE_LOCK_GUARD_1(rq_lock_irqsave, struct rq,
rq_lock_irqsave(_T->lock, &_T->rf),
rq_unlock_irqrestore(_T->lock, &_T->rf),
struct rq_flags rf)
-static inline struct rq *this_rq_lock_irq(struct rq_flags *rf)
- __acquires(rq->lock)
+DECLARE_LOCK_GUARD_1_ATTRS(rq_lock_irqsave, __acquires(__rq_lockp(_T)), __releases(__rq_lockp(*(struct rq **)_T)));
+#define class_rq_lock_irqsave_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(rq_lock_irqsave, _T)
+
+#define this_rq_lock_irq(...) __acquire_ret(_this_rq_lock_irq(__VA_ARGS__), __rq_lockp(__ret))
+static inline struct rq *_this_rq_lock_irq(struct rq_flags *rf) __acquires_ret
{
struct rq *rq;
@@ -3050,8 +3076,20 @@ static inline void double_rq_clock_clear_update(struct rq *rq1, struct rq *rq2)
#define DEFINE_LOCK_GUARD_2(name, type, _lock, _unlock, ...) \
__DEFINE_UNLOCK_GUARD(name, type, _unlock, type *lock2; __VA_ARGS__) \
static inline class_##name##_t class_##name##_constructor(type *lock, type *lock2) \
+ __no_context_analysis \
{ class_##name##_t _t = { .lock = lock, .lock2 = lock2 }, *_T = &_t; \
_lock; return _t; }
+#define DECLARE_LOCK_GUARD_2_ATTRS(_name, _lock, _unlock1, _unlock2) \
+static inline class_##_name##_t class_##_name##_constructor(lock_##_name##_t *_T1, \
+ lock_##_name##_t *_T2) _lock; \
+static __always_inline void __class_##_name##_cleanup_ctx1(class_##_name##_t **_T1) \
+ __no_context_analysis _unlock1 { } \
+static __always_inline void __class_##_name##_cleanup_ctx2(class_##_name##_t **_T2) \
+ __no_context_analysis _unlock2 { }
+#define WITH_LOCK_GUARD_2_ATTRS(_name, _T1, _T2) \
+ class_##_name##_constructor(_T1, _T2), \
+ *__UNIQUE_ID(unlock1) __cleanup(__class_##_name##_cleanup_ctx1) = (void *)(_T1),\
+ *__UNIQUE_ID(unlock2) __cleanup(__class_##_name##_cleanup_ctx2) = (void *)(_T2)
static inline bool rq_order_less(struct rq *rq1, struct rq *rq2)
{
@@ -3079,7 +3117,8 @@ static inline bool rq_order_less(struct rq *rq1, struct rq *rq2)
return rq1->cpu < rq2->cpu;
}
-extern void double_rq_lock(struct rq *rq1, struct rq *rq2);
+extern void double_rq_lock(struct rq *rq1, struct rq *rq2)
+ __acquires(__rq_lockp(rq1), __rq_lockp(rq2));
#ifdef CONFIG_PREEMPTION
@@ -3092,9 +3131,8 @@ extern void double_rq_lock(struct rq *rq1, struct rq *rq2);
* also adds more overhead and therefore may reduce throughput.
*/
static inline int _double_lock_balance(struct rq *this_rq, struct rq *busiest)
- __releases(this_rq->lock)
- __acquires(busiest->lock)
- __acquires(this_rq->lock)
+ __must_hold(__rq_lockp(this_rq))
+ __acquires(__rq_lockp(busiest))
{
raw_spin_rq_unlock(this_rq);
double_rq_lock(this_rq, busiest);
@@ -3111,12 +3149,16 @@ static inline int _double_lock_balance(struct rq *this_rq, struct rq *busiest)
* regardless of entry order into the function.
*/
static inline int _double_lock_balance(struct rq *this_rq, struct rq *busiest)
- __releases(this_rq->lock)
- __acquires(busiest->lock)
- __acquires(this_rq->lock)
+ __must_hold(__rq_lockp(this_rq))
+ __acquires(__rq_lockp(busiest))
{
- if (__rq_lockp(this_rq) == __rq_lockp(busiest) ||
- likely(raw_spin_rq_trylock(busiest))) {
+ if (__rq_lockp(this_rq) == __rq_lockp(busiest)) {
+ __acquire(__rq_lockp(busiest)); /* already held */
+ double_rq_clock_clear_update(this_rq, busiest);
+ return 0;
+ }
+
+ if (likely(raw_spin_rq_trylock(busiest))) {
double_rq_clock_clear_update(this_rq, busiest);
return 0;
}
@@ -3139,6 +3181,8 @@ static inline int _double_lock_balance(struct rq *this_rq, struct rq *busiest)
* double_lock_balance - lock the busiest runqueue, this_rq is locked already.
*/
static inline int double_lock_balance(struct rq *this_rq, struct rq *busiest)
+ __must_hold(__rq_lockp(this_rq))
+ __acquires(__rq_lockp(busiest))
{
lockdep_assert_irqs_disabled();
@@ -3146,14 +3190,17 @@ static inline int double_lock_balance(struct rq *this_rq, struct rq *busiest)
}
static inline void double_unlock_balance(struct rq *this_rq, struct rq *busiest)
- __releases(busiest->lock)
+ __releases(__rq_lockp(busiest))
{
if (__rq_lockp(this_rq) != __rq_lockp(busiest))
raw_spin_rq_unlock(busiest);
+ else
+ __release(__rq_lockp(busiest)); /* fake release */
lock_set_subclass(&__rq_lockp(this_rq)->dep_map, 0, _RET_IP_);
}
static inline void double_lock(spinlock_t *l1, spinlock_t *l2)
+ __acquires(l1, l2)
{
if (l1 > l2)
swap(l1, l2);
@@ -3163,6 +3210,7 @@ static inline void double_lock(spinlock_t *l1, spinlock_t *l2)
}
static inline void double_lock_irq(spinlock_t *l1, spinlock_t *l2)
+ __acquires(l1, l2)
{
if (l1 > l2)
swap(l1, l2);
@@ -3172,6 +3220,7 @@ static inline void double_lock_irq(spinlock_t *l1, spinlock_t *l2)
}
static inline void double_raw_lock(raw_spinlock_t *l1, raw_spinlock_t *l2)
+ __acquires(l1, l2)
{
if (l1 > l2)
swap(l1, l2);
@@ -3181,6 +3230,7 @@ static inline void double_raw_lock(raw_spinlock_t *l1, raw_spinlock_t *l2)
}
static inline void double_raw_unlock(raw_spinlock_t *l1, raw_spinlock_t *l2)
+ __releases(l1, l2)
{
raw_spin_unlock(l1);
raw_spin_unlock(l2);
@@ -3190,6 +3240,13 @@ DEFINE_LOCK_GUARD_2(double_raw_spinlock, raw_spinlock_t,
double_raw_lock(_T->lock, _T->lock2),
double_raw_unlock(_T->lock, _T->lock2))
+DECLARE_LOCK_GUARD_2_ATTRS(double_raw_spinlock,
+ __acquires(_T1, _T2),
+ __releases(*(raw_spinlock_t **)_T1),
+ __releases(*(raw_spinlock_t **)_T2));
+#define class_double_raw_spinlock_constructor(_T1, _T2) \
+ WITH_LOCK_GUARD_2_ATTRS(double_raw_spinlock, _T1, _T2)
+
/*
* double_rq_unlock - safely unlock two runqueues
*
@@ -3197,13 +3254,12 @@ DEFINE_LOCK_GUARD_2(double_raw_spinlock, raw_spinlock_t,
* you need to do so manually after calling.
*/
static inline void double_rq_unlock(struct rq *rq1, struct rq *rq2)
- __releases(rq1->lock)
- __releases(rq2->lock)
+ __releases(__rq_lockp(rq1), __rq_lockp(rq2))
{
if (__rq_lockp(rq1) != __rq_lockp(rq2))
raw_spin_rq_unlock(rq2);
else
- __release(rq2->lock);
+ __release(__rq_lockp(rq2)); /* fake release */
raw_spin_rq_unlock(rq1);
}
diff --git a/scripts/context-analysis-suppression.txt b/scripts/context-analysis-suppression.txt
index df25c3d07a5b..fd8951d06706 100644
--- a/scripts/context-analysis-suppression.txt
+++ b/scripts/context-analysis-suppression.txt
@@ -26,6 +26,7 @@ src:*include/linux/refcount.h=emit
src:*include/linux/rhashtable.h=emit
src:*include/linux/rwlock*.h=emit
src:*include/linux/rwsem.h=emit
+src:*include/linux/sched*=emit
src:*include/linux/seqlock*.h=emit
src:*include/linux/spinlock*.h=emit
src:*include/linux/srcu*.h=emit
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox