* 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
* [PATCH v5 35/36] crypto: Enable context analysis
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
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
Enable context analysis for crypto subsystem.
This demonstrates a larger conversion to use Clang's context
analysis. The benefit is additional static checking of locking rules,
along with better documentation.
Note the use of the __acquire_ret macro how to define an API where a
function returns a pointer to an object (struct scomp_scratch) with a
lock held. Additionally, the analysis only resolves aliases where the
analysis unambiguously sees that a variable was not reassigned after
initialization, requiring minor code changes.
Signed-off-by: Marco Elver <elver@google.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-crypto@vger.kernel.org
---
v4:
* Rename capability -> context analysis.
v3:
* Rebase - make use of __acquire_ret macro for new functions.
* Initialize variables once where we want the analysis to recognize aliases.
v2:
* New patch.
---
crypto/Makefile | 2 ++
crypto/acompress.c | 6 +++---
crypto/algapi.c | 2 ++
crypto/api.c | 1 +
crypto/crypto_engine.c | 2 +-
crypto/drbg.c | 5 +++++
crypto/internal.h | 2 +-
crypto/proc.c | 3 +++
crypto/scompress.c | 24 ++++++++++++------------
include/crypto/internal/acompress.h | 7 ++++---
include/crypto/internal/engine.h | 2 +-
11 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/crypto/Makefile b/crypto/Makefile
index 16a35649dd91..db264feab7e7 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -3,6 +3,8 @@
# Cryptographic API
#
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_CRYPTO) += crypto.o
crypto-y := api.o cipher.o
diff --git a/crypto/acompress.c b/crypto/acompress.c
index be28cbfd22e3..25df368df098 100644
--- a/crypto/acompress.c
+++ b/crypto/acompress.c
@@ -449,8 +449,8 @@ int crypto_acomp_alloc_streams(struct crypto_acomp_streams *s)
}
EXPORT_SYMBOL_GPL(crypto_acomp_alloc_streams);
-struct crypto_acomp_stream *crypto_acomp_lock_stream_bh(
- struct crypto_acomp_streams *s) __acquires(stream)
+struct crypto_acomp_stream *_crypto_acomp_lock_stream_bh(
+ struct crypto_acomp_streams *s)
{
struct crypto_acomp_stream __percpu *streams = s->streams;
int cpu = raw_smp_processor_id();
@@ -469,7 +469,7 @@ struct crypto_acomp_stream *crypto_acomp_lock_stream_bh(
spin_lock(&ps->lock);
return ps;
}
-EXPORT_SYMBOL_GPL(crypto_acomp_lock_stream_bh);
+EXPORT_SYMBOL_GPL(_crypto_acomp_lock_stream_bh);
void acomp_walk_done_src(struct acomp_walk *walk, int used)
{
diff --git a/crypto/algapi.c b/crypto/algapi.c
index e604d0d8b7b4..abc9333327d4 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -244,6 +244,7 @@ EXPORT_SYMBOL_GPL(crypto_remove_spawns);
static void crypto_alg_finish_registration(struct crypto_alg *alg,
struct list_head *algs_to_put)
+ __must_hold(&crypto_alg_sem)
{
struct crypto_alg *q;
@@ -299,6 +300,7 @@ static struct crypto_larval *crypto_alloc_test_larval(struct crypto_alg *alg)
static struct crypto_larval *
__crypto_register_alg(struct crypto_alg *alg, struct list_head *algs_to_put)
+ __must_hold(&crypto_alg_sem)
{
struct crypto_alg *q;
struct crypto_larval *larval;
diff --git a/crypto/api.c b/crypto/api.c
index 5724d62e9d07..05629644a688 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -57,6 +57,7 @@ EXPORT_SYMBOL_GPL(crypto_mod_put);
static struct crypto_alg *__crypto_alg_lookup(const char *name, u32 type,
u32 mask)
+ __must_hold_shared(&crypto_alg_sem)
{
struct crypto_alg *q, *alg = NULL;
int best = -2;
diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
index 18e1689efe12..1653a4bf5b31 100644
--- a/crypto/crypto_engine.c
+++ b/crypto/crypto_engine.c
@@ -453,8 +453,8 @@ struct crypto_engine *crypto_engine_alloc_init_and_set(struct device *dev,
snprintf(engine->name, sizeof(engine->name),
"%s-engine", dev_name(dev));
- crypto_init_queue(&engine->queue, qlen);
spin_lock_init(&engine->queue_lock);
+ crypto_init_queue(&engine->queue, qlen);
engine->kworker = kthread_run_worker(0, "%s", engine->name);
if (IS_ERR(engine->kworker)) {
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 1d433dae9955..0a6f6c05a78f 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -232,6 +232,7 @@ static inline unsigned short drbg_sec_strength(drbg_flag_t flags)
*/
static int drbg_fips_continuous_test(struct drbg_state *drbg,
const unsigned char *entropy)
+ __must_hold(&drbg->drbg_mutex)
{
unsigned short entropylen = drbg_sec_strength(drbg->core->flags);
int ret = 0;
@@ -848,6 +849,7 @@ static inline int __drbg_seed(struct drbg_state *drbg, struct list_head *seed,
static inline int drbg_get_random_bytes(struct drbg_state *drbg,
unsigned char *entropy,
unsigned int entropylen)
+ __must_hold(&drbg->drbg_mutex)
{
int ret;
@@ -862,6 +864,7 @@ static inline int drbg_get_random_bytes(struct drbg_state *drbg,
}
static int drbg_seed_from_random(struct drbg_state *drbg)
+ __must_hold(&drbg->drbg_mutex)
{
struct drbg_string data;
LIST_HEAD(seedlist);
@@ -919,6 +922,7 @@ static bool drbg_nopr_reseed_interval_elapsed(struct drbg_state *drbg)
*/
static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
bool reseed)
+ __must_hold(&drbg->drbg_mutex)
{
int ret;
unsigned char entropy[((32 + 16) * 2)];
@@ -1153,6 +1157,7 @@ static inline int drbg_alloc_state(struct drbg_state *drbg)
static int drbg_generate(struct drbg_state *drbg,
unsigned char *buf, unsigned int buflen,
struct drbg_string *addtl)
+ __must_hold(&drbg->drbg_mutex)
{
int len = 0;
LIST_HEAD(addtllist);
diff --git a/crypto/internal.h b/crypto/internal.h
index b9afd68767c1..8fbe0226d48e 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -61,8 +61,8 @@ enum {
/* Maximum number of (rtattr) parameters for each template. */
#define CRYPTO_MAX_ATTRS 32
-extern struct list_head crypto_alg_list;
extern struct rw_semaphore crypto_alg_sem;
+extern struct list_head crypto_alg_list __guarded_by(&crypto_alg_sem);
extern struct blocking_notifier_head crypto_chain;
int alg_test(const char *driver, const char *alg, u32 type, u32 mask);
diff --git a/crypto/proc.c b/crypto/proc.c
index 82f15b967e85..5fb9fe86d023 100644
--- a/crypto/proc.c
+++ b/crypto/proc.c
@@ -19,17 +19,20 @@
#include "internal.h"
static void *c_start(struct seq_file *m, loff_t *pos)
+ __acquires_shared(&crypto_alg_sem)
{
down_read(&crypto_alg_sem);
return seq_list_start(&crypto_alg_list, *pos);
}
static void *c_next(struct seq_file *m, void *p, loff_t *pos)
+ __must_hold_shared(&crypto_alg_sem)
{
return seq_list_next(p, &crypto_alg_list, pos);
}
static void c_stop(struct seq_file *m, void *p)
+ __releases_shared(&crypto_alg_sem)
{
up_read(&crypto_alg_sem);
}
diff --git a/crypto/scompress.c b/crypto/scompress.c
index 1a7ed8ae65b0..7aee1d50e148 100644
--- a/crypto/scompress.c
+++ b/crypto/scompress.c
@@ -28,8 +28,8 @@
struct scomp_scratch {
spinlock_t lock;
union {
- void *src;
- unsigned long saddr;
+ void *src __guarded_by(&lock);
+ unsigned long saddr __guarded_by(&lock);
};
};
@@ -38,8 +38,8 @@ static DEFINE_PER_CPU(struct scomp_scratch, scomp_scratch) = {
};
static const struct crypto_type crypto_scomp_type;
-static int scomp_scratch_users;
static DEFINE_MUTEX(scomp_lock);
+static int scomp_scratch_users __guarded_by(&scomp_lock);
static cpumask_t scomp_scratch_want;
static void scomp_scratch_workfn(struct work_struct *work);
@@ -67,6 +67,7 @@ static void crypto_scomp_show(struct seq_file *m, struct crypto_alg *alg)
}
static void crypto_scomp_free_scratches(void)
+ __context_unsafe(/* frees @scratch */)
{
struct scomp_scratch *scratch;
int i;
@@ -101,7 +102,7 @@ static void scomp_scratch_workfn(struct work_struct *work)
struct scomp_scratch *scratch;
scratch = per_cpu_ptr(&scomp_scratch, cpu);
- if (scratch->src)
+ if (context_unsafe(scratch->src))
continue;
if (scomp_alloc_scratch(scratch, cpu))
break;
@@ -111,6 +112,7 @@ static void scomp_scratch_workfn(struct work_struct *work)
}
static int crypto_scomp_alloc_scratches(void)
+ __context_unsafe(/* allocates @scratch */)
{
unsigned int i = cpumask_first(cpu_possible_mask);
struct scomp_scratch *scratch;
@@ -139,7 +141,8 @@ static int crypto_scomp_init_tfm(struct crypto_tfm *tfm)
return ret;
}
-static struct scomp_scratch *scomp_lock_scratch(void) __acquires(scratch)
+#define scomp_lock_scratch(...) __acquire_ret(_scomp_lock_scratch(__VA_ARGS__), &__ret->lock)
+static struct scomp_scratch *_scomp_lock_scratch(void) __acquires_ret
{
int cpu = raw_smp_processor_id();
struct scomp_scratch *scratch;
@@ -159,7 +162,7 @@ static struct scomp_scratch *scomp_lock_scratch(void) __acquires(scratch)
}
static inline void scomp_unlock_scratch(struct scomp_scratch *scratch)
- __releases(scratch)
+ __releases(&scratch->lock)
{
spin_unlock(&scratch->lock);
}
@@ -171,8 +174,6 @@ static int scomp_acomp_comp_decomp(struct acomp_req *req, int dir)
bool src_isvirt = acomp_request_src_isvirt(req);
bool dst_isvirt = acomp_request_dst_isvirt(req);
struct crypto_scomp *scomp = *tfm_ctx;
- struct crypto_acomp_stream *stream;
- struct scomp_scratch *scratch;
unsigned int slen = req->slen;
unsigned int dlen = req->dlen;
struct page *spage, *dpage;
@@ -232,13 +233,12 @@ static int scomp_acomp_comp_decomp(struct acomp_req *req, int dir)
} while (0);
}
- stream = crypto_acomp_lock_stream_bh(&crypto_scomp_alg(scomp)->streams);
+ struct crypto_acomp_stream *stream = crypto_acomp_lock_stream_bh(&crypto_scomp_alg(scomp)->streams);
if (!src_isvirt && !src) {
- const u8 *src;
+ struct scomp_scratch *scratch = scomp_lock_scratch();
+ const u8 *src = scratch->src;
- scratch = scomp_lock_scratch();
- src = scratch->src;
memcpy_from_sglist(scratch->src, req->src, 0, slen);
if (dir)
diff --git a/include/crypto/internal/acompress.h b/include/crypto/internal/acompress.h
index 2d97440028ff..9a3f28baa804 100644
--- a/include/crypto/internal/acompress.h
+++ b/include/crypto/internal/acompress.h
@@ -191,11 +191,12 @@ static inline bool crypto_acomp_req_virt(struct crypto_acomp *tfm)
void crypto_acomp_free_streams(struct crypto_acomp_streams *s);
int crypto_acomp_alloc_streams(struct crypto_acomp_streams *s);
-struct crypto_acomp_stream *crypto_acomp_lock_stream_bh(
- struct crypto_acomp_streams *s) __acquires(stream);
+#define crypto_acomp_lock_stream_bh(...) __acquire_ret(_crypto_acomp_lock_stream_bh(__VA_ARGS__), &__ret->lock);
+struct crypto_acomp_stream *_crypto_acomp_lock_stream_bh(
+ struct crypto_acomp_streams *s) __acquires_ret;
static inline void crypto_acomp_unlock_stream_bh(
- struct crypto_acomp_stream *stream) __releases(stream)
+ struct crypto_acomp_stream *stream) __releases(&stream->lock)
{
spin_unlock_bh(&stream->lock);
}
diff --git a/include/crypto/internal/engine.h b/include/crypto/internal/engine.h
index f19ef376833f..6a1d27880615 100644
--- a/include/crypto/internal/engine.h
+++ b/include/crypto/internal/engine.h
@@ -45,7 +45,7 @@ struct crypto_engine {
struct list_head list;
spinlock_t queue_lock;
- struct crypto_queue queue;
+ struct crypto_queue queue __guarded_by(&queue_lock);
struct device *dev;
struct kthread_worker *kworker;
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
* [PATCH v5 34/36] security/tomoyo: Enable context analysis
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
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
Enable context analysis for security/tomoyo.
This demonstrates a larger conversion to use Clang's context
analysis. The benefit is additional static checking of locking rules,
along with better documentation.
Tomoyo makes use of several synchronization primitives, yet its clear
design made it relatively straightforward to enable context analysis.
One notable finding was:
security/tomoyo/gc.c:664:20: error: reading variable 'write_buf' requires holding mutex '&tomoyo_io_buffer::io_sem'
664 | is_write = head->write_buf != NULL;
For which Tetsuo writes:
"Good catch. This should be data_race(), for tomoyo_write_control()
might concurrently update head->write_buf from non-NULL to non-NULL
with head->io_sem held."
Signed-off-by: Marco Elver <elver@google.com>
Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
v4:
* Rename capability -> context analysis.
v2:
* New patch.
---
security/tomoyo/Makefile | 2 +
security/tomoyo/common.c | 52 ++++++++++++++++++++++++--
security/tomoyo/common.h | 77 ++++++++++++++++++++-------------------
security/tomoyo/domain.c | 1 +
security/tomoyo/environ.c | 1 +
security/tomoyo/file.c | 5 +++
security/tomoyo/gc.c | 28 ++++++++++----
security/tomoyo/mount.c | 2 +
security/tomoyo/network.c | 3 ++
9 files changed, 122 insertions(+), 49 deletions(-)
diff --git a/security/tomoyo/Makefile b/security/tomoyo/Makefile
index 55c67b9846a9..e3c0f853aa3b 100644
--- a/security/tomoyo/Makefile
+++ b/security/tomoyo/Makefile
@@ -1,4 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
obj-y = audit.o common.o condition.o domain.o environ.o file.o gc.o group.o load_policy.o memory.o mount.o network.o realpath.o securityfs_if.o tomoyo.o util.o
targets += builtin-policy.h
diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
index 0f78898bce09..86ce56c32d37 100644
--- a/security/tomoyo/common.c
+++ b/security/tomoyo/common.c
@@ -268,6 +268,7 @@ static void tomoyo_io_printf(struct tomoyo_io_buffer *head, const char *fmt,
*/
static void tomoyo_io_printf(struct tomoyo_io_buffer *head, const char *fmt,
...)
+ __must_hold(&head->io_sem)
{
va_list args;
size_t len;
@@ -416,8 +417,9 @@ static void tomoyo_print_name_union_quoted(struct tomoyo_io_buffer *head,
*
* Returns nothing.
*/
-static void tomoyo_print_number_union_nospace
-(struct tomoyo_io_buffer *head, const struct tomoyo_number_union *ptr)
+static void
+tomoyo_print_number_union_nospace(struct tomoyo_io_buffer *head, const struct tomoyo_number_union *ptr)
+ __must_hold(&head->io_sem)
{
if (ptr->group) {
tomoyo_set_string(head, "@");
@@ -466,6 +468,7 @@ static void tomoyo_print_number_union_nospace
*/
static void tomoyo_print_number_union(struct tomoyo_io_buffer *head,
const struct tomoyo_number_union *ptr)
+ __must_hold(&head->io_sem)
{
tomoyo_set_space(head);
tomoyo_print_number_union_nospace(head, ptr);
@@ -664,6 +667,7 @@ static int tomoyo_set_mode(char *name, const char *value,
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_write_profile(struct tomoyo_io_buffer *head)
+ __must_hold(&head->io_sem)
{
char *data = head->write_buf;
unsigned int i;
@@ -719,6 +723,7 @@ static int tomoyo_write_profile(struct tomoyo_io_buffer *head)
* Caller prints functionality's name.
*/
static void tomoyo_print_config(struct tomoyo_io_buffer *head, const u8 config)
+ __must_hold(&head->io_sem)
{
tomoyo_io_printf(head, "={ mode=%s grant_log=%s reject_log=%s }\n",
tomoyo_mode[config & 3],
@@ -734,6 +739,7 @@ static void tomoyo_print_config(struct tomoyo_io_buffer *head, const u8 config)
* Returns nothing.
*/
static void tomoyo_read_profile(struct tomoyo_io_buffer *head)
+ __must_hold(&head->io_sem)
{
u8 index;
struct tomoyo_policy_namespace *ns =
@@ -852,6 +858,7 @@ static bool tomoyo_same_manager(const struct tomoyo_acl_head *a,
*/
static int tomoyo_update_manager_entry(const char *manager,
const bool is_delete)
+ __must_hold_shared(&tomoyo_ss)
{
struct tomoyo_manager e = { };
struct tomoyo_acl_param param = {
@@ -883,6 +890,8 @@ static int tomoyo_update_manager_entry(const char *manager,
* Caller holds tomoyo_read_lock().
*/
static int tomoyo_write_manager(struct tomoyo_io_buffer *head)
+ __must_hold_shared(&tomoyo_ss)
+ __must_hold(&head->io_sem)
{
char *data = head->write_buf;
@@ -901,6 +910,7 @@ static int tomoyo_write_manager(struct tomoyo_io_buffer *head)
* Caller holds tomoyo_read_lock().
*/
static void tomoyo_read_manager(struct tomoyo_io_buffer *head)
+ __must_hold_shared(&tomoyo_ss)
{
if (head->r.eof)
return;
@@ -927,6 +937,7 @@ static void tomoyo_read_manager(struct tomoyo_io_buffer *head)
* Caller holds tomoyo_read_lock().
*/
static bool tomoyo_manager(void)
+ __must_hold_shared(&tomoyo_ss)
{
struct tomoyo_manager *ptr;
const char *exe;
@@ -981,6 +992,8 @@ static struct tomoyo_domain_info *tomoyo_find_domain_by_qid
*/
static bool tomoyo_select_domain(struct tomoyo_io_buffer *head,
const char *data)
+ __must_hold_shared(&tomoyo_ss)
+ __must_hold(&head->io_sem)
{
unsigned int pid;
struct tomoyo_domain_info *domain = NULL;
@@ -1051,6 +1064,7 @@ static bool tomoyo_same_task_acl(const struct tomoyo_acl_info *a,
* Caller holds tomoyo_read_lock().
*/
static int tomoyo_write_task(struct tomoyo_acl_param *param)
+ __must_hold_shared(&tomoyo_ss)
{
int error = -EINVAL;
@@ -1079,6 +1093,7 @@ static int tomoyo_write_task(struct tomoyo_acl_param *param)
* Caller holds tomoyo_read_lock().
*/
static int tomoyo_delete_domain(char *domainname)
+ __must_hold_shared(&tomoyo_ss)
{
struct tomoyo_domain_info *domain;
struct tomoyo_path_info name;
@@ -1118,6 +1133,7 @@ static int tomoyo_delete_domain(char *domainname)
static int tomoyo_write_domain2(struct tomoyo_policy_namespace *ns,
struct list_head *list, char *data,
const bool is_delete)
+ __must_hold_shared(&tomoyo_ss)
{
struct tomoyo_acl_param param = {
.ns = ns,
@@ -1162,6 +1178,8 @@ const char * const tomoyo_dif[TOMOYO_MAX_DOMAIN_INFO_FLAGS] = {
* Caller holds tomoyo_read_lock().
*/
static int tomoyo_write_domain(struct tomoyo_io_buffer *head)
+ __must_hold_shared(&tomoyo_ss)
+ __must_hold(&head->io_sem)
{
char *data = head->write_buf;
struct tomoyo_policy_namespace *ns;
@@ -1223,6 +1241,7 @@ static int tomoyo_write_domain(struct tomoyo_io_buffer *head)
*/
static bool tomoyo_print_condition(struct tomoyo_io_buffer *head,
const struct tomoyo_condition *cond)
+ __must_hold(&head->io_sem)
{
switch (head->r.cond_step) {
case 0:
@@ -1364,6 +1383,7 @@ static bool tomoyo_print_condition(struct tomoyo_io_buffer *head,
*/
static void tomoyo_set_group(struct tomoyo_io_buffer *head,
const char *category)
+ __must_hold(&head->io_sem)
{
if (head->type == TOMOYO_EXCEPTIONPOLICY) {
tomoyo_print_namespace(head);
@@ -1383,6 +1403,7 @@ static void tomoyo_set_group(struct tomoyo_io_buffer *head,
*/
static bool tomoyo_print_entry(struct tomoyo_io_buffer *head,
struct tomoyo_acl_info *acl)
+ __must_hold(&head->io_sem)
{
const u8 acl_type = acl->type;
bool first = true;
@@ -1588,6 +1609,8 @@ static bool tomoyo_print_entry(struct tomoyo_io_buffer *head,
*/
static bool tomoyo_read_domain2(struct tomoyo_io_buffer *head,
struct list_head *list)
+ __must_hold_shared(&tomoyo_ss)
+ __must_hold(&head->io_sem)
{
list_for_each_cookie(head->r.acl, list) {
struct tomoyo_acl_info *ptr =
@@ -1608,6 +1631,8 @@ static bool tomoyo_read_domain2(struct tomoyo_io_buffer *head,
* Caller holds tomoyo_read_lock().
*/
static void tomoyo_read_domain(struct tomoyo_io_buffer *head)
+ __must_hold_shared(&tomoyo_ss)
+ __must_hold(&head->io_sem)
{
if (head->r.eof)
return;
@@ -1686,6 +1711,7 @@ static int tomoyo_write_pid(struct tomoyo_io_buffer *head)
* using read()/write() interface rather than sysctl() interface.
*/
static void tomoyo_read_pid(struct tomoyo_io_buffer *head)
+ __must_hold(&head->io_sem)
{
char *buf = head->write_buf;
bool global_pid = false;
@@ -1746,6 +1772,8 @@ static const char *tomoyo_group_name[TOMOYO_MAX_GROUP] = {
* Caller holds tomoyo_read_lock().
*/
static int tomoyo_write_exception(struct tomoyo_io_buffer *head)
+ __must_hold_shared(&tomoyo_ss)
+ __must_hold(&head->io_sem)
{
const bool is_delete = head->w.is_delete;
struct tomoyo_acl_param param = {
@@ -1787,6 +1815,8 @@ static int tomoyo_write_exception(struct tomoyo_io_buffer *head)
* Caller holds tomoyo_read_lock().
*/
static bool tomoyo_read_group(struct tomoyo_io_buffer *head, const int idx)
+ __must_hold_shared(&tomoyo_ss)
+ __must_hold(&head->io_sem)
{
struct tomoyo_policy_namespace *ns =
container_of(head->r.ns, typeof(*ns), namespace_list);
@@ -1846,6 +1876,7 @@ static bool tomoyo_read_group(struct tomoyo_io_buffer *head, const int idx)
* Caller holds tomoyo_read_lock().
*/
static bool tomoyo_read_policy(struct tomoyo_io_buffer *head, const int idx)
+ __must_hold_shared(&tomoyo_ss)
{
struct tomoyo_policy_namespace *ns =
container_of(head->r.ns, typeof(*ns), namespace_list);
@@ -1906,6 +1937,8 @@ static bool tomoyo_read_policy(struct tomoyo_io_buffer *head, const int idx)
* Caller holds tomoyo_read_lock().
*/
static void tomoyo_read_exception(struct tomoyo_io_buffer *head)
+ __must_hold_shared(&tomoyo_ss)
+ __must_hold(&head->io_sem)
{
struct tomoyo_policy_namespace *ns =
container_of(head->r.ns, typeof(*ns), namespace_list);
@@ -2097,6 +2130,7 @@ static void tomoyo_patternize_path(char *buffer, const int len, char *entry)
* Returns nothing.
*/
static void tomoyo_add_entry(struct tomoyo_domain_info *domain, char *header)
+ __must_hold_shared(&tomoyo_ss)
{
char *buffer;
char *realpath = NULL;
@@ -2301,6 +2335,7 @@ static __poll_t tomoyo_poll_query(struct file *file, poll_table *wait)
* @head: Pointer to "struct tomoyo_io_buffer".
*/
static void tomoyo_read_query(struct tomoyo_io_buffer *head)
+ __must_hold(&head->io_sem)
{
struct list_head *tmp;
unsigned int pos = 0;
@@ -2362,6 +2397,7 @@ static void tomoyo_read_query(struct tomoyo_io_buffer *head)
* Returns 0 on success, -EINVAL otherwise.
*/
static int tomoyo_write_answer(struct tomoyo_io_buffer *head)
+ __must_hold(&head->io_sem)
{
char *data = head->write_buf;
struct list_head *tmp;
@@ -2401,6 +2437,7 @@ static int tomoyo_write_answer(struct tomoyo_io_buffer *head)
* Returns version information.
*/
static void tomoyo_read_version(struct tomoyo_io_buffer *head)
+ __must_hold(&head->io_sem)
{
if (!head->r.eof) {
tomoyo_io_printf(head, "2.6.0");
@@ -2449,6 +2486,7 @@ void tomoyo_update_stat(const u8 index)
* Returns nothing.
*/
static void tomoyo_read_stat(struct tomoyo_io_buffer *head)
+ __must_hold(&head->io_sem)
{
u8 i;
unsigned int total = 0;
@@ -2493,6 +2531,7 @@ static void tomoyo_read_stat(struct tomoyo_io_buffer *head)
* Returns 0.
*/
static int tomoyo_write_stat(struct tomoyo_io_buffer *head)
+ __must_hold(&head->io_sem)
{
char *data = head->write_buf;
u8 i;
@@ -2717,6 +2756,8 @@ ssize_t tomoyo_read_control(struct tomoyo_io_buffer *head, char __user *buffer,
* Caller holds tomoyo_read_lock().
*/
static int tomoyo_parse_policy(struct tomoyo_io_buffer *head, char *line)
+ __must_hold_shared(&tomoyo_ss)
+ __must_hold(&head->io_sem)
{
/* Delete request? */
head->w.is_delete = !strncmp(line, "delete ", 7);
@@ -2969,8 +3010,11 @@ void __init tomoyo_load_builtin_policy(void)
break;
*end = '\0';
tomoyo_normalize_line(start);
- head.write_buf = start;
- tomoyo_parse_policy(&head, start);
+ /* head is stack-local and not shared. */
+ context_unsafe(
+ head.write_buf = start;
+ tomoyo_parse_policy(&head, start);
+ );
start = end + 1;
}
}
diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
index 3b2a97d10a5d..4f1704c911ef 100644
--- a/security/tomoyo/common.h
+++ b/security/tomoyo/common.h
@@ -827,13 +827,13 @@ struct tomoyo_io_buffer {
bool is_delete;
} w;
/* Buffer for reading. */
- char *read_buf;
+ char *read_buf __guarded_by(&io_sem);
/* Size of read buffer. */
- size_t readbuf_size;
+ size_t readbuf_size __guarded_by(&io_sem);
/* Buffer for writing. */
- char *write_buf;
+ char *write_buf __guarded_by(&io_sem);
/* Size of write buffer. */
- size_t writebuf_size;
+ size_t writebuf_size __guarded_by(&io_sem);
/* Type of this interface. */
enum tomoyo_securityfs_interface_index type;
/* Users counter protected by tomoyo_io_buffer_list_lock. */
@@ -922,6 +922,35 @@ struct tomoyo_task {
struct tomoyo_domain_info *old_domain_info;
};
+/********** External variable definitions. **********/
+
+extern bool tomoyo_policy_loaded;
+extern int tomoyo_enabled;
+extern const char * const tomoyo_condition_keyword
+[TOMOYO_MAX_CONDITION_KEYWORD];
+extern const char * const tomoyo_dif[TOMOYO_MAX_DOMAIN_INFO_FLAGS];
+extern const char * const tomoyo_mac_keywords[TOMOYO_MAX_MAC_INDEX
+ + TOMOYO_MAX_MAC_CATEGORY_INDEX];
+extern const char * const tomoyo_mode[TOMOYO_CONFIG_MAX_MODE];
+extern const char * const tomoyo_path_keyword[TOMOYO_MAX_PATH_OPERATION];
+extern const char * const tomoyo_proto_keyword[TOMOYO_SOCK_MAX];
+extern const char * const tomoyo_socket_keyword[TOMOYO_MAX_NETWORK_OPERATION];
+extern const u8 tomoyo_index2category[TOMOYO_MAX_MAC_INDEX];
+extern const u8 tomoyo_pn2mac[TOMOYO_MAX_PATH_NUMBER_OPERATION];
+extern const u8 tomoyo_pnnn2mac[TOMOYO_MAX_MKDEV_OPERATION];
+extern const u8 tomoyo_pp2mac[TOMOYO_MAX_PATH2_OPERATION];
+extern struct list_head tomoyo_condition_list;
+extern struct list_head tomoyo_domain_list;
+extern struct list_head tomoyo_name_list[TOMOYO_MAX_HASH];
+extern struct list_head tomoyo_namespace_list;
+extern struct mutex tomoyo_policy_lock;
+extern struct srcu_struct tomoyo_ss;
+extern struct tomoyo_domain_info tomoyo_kernel_domain;
+extern struct tomoyo_policy_namespace tomoyo_kernel_namespace;
+extern unsigned int tomoyo_memory_quota[TOMOYO_MAX_MEMORY_STAT];
+extern unsigned int tomoyo_memory_used[TOMOYO_MAX_MEMORY_STAT];
+extern struct lsm_blob_sizes tomoyo_blob_sizes;
+
/********** Function prototypes. **********/
int tomoyo_interface_init(void);
@@ -971,10 +1000,10 @@ const struct tomoyo_path_info *tomoyo_path_matches_group
int tomoyo_check_open_permission(struct tomoyo_domain_info *domain,
const struct path *path, const int flag);
void tomoyo_close_control(struct tomoyo_io_buffer *head);
-int tomoyo_env_perm(struct tomoyo_request_info *r, const char *env);
+int tomoyo_env_perm(struct tomoyo_request_info *r, const char *env) __must_hold_shared(&tomoyo_ss);
int tomoyo_execute_permission(struct tomoyo_request_info *r,
- const struct tomoyo_path_info *filename);
-int tomoyo_find_next_domain(struct linux_binprm *bprm);
+ const struct tomoyo_path_info *filename) __must_hold_shared(&tomoyo_ss);
+int tomoyo_find_next_domain(struct linux_binprm *bprm) __must_hold_shared(&tomoyo_ss);
int tomoyo_get_mode(const struct tomoyo_policy_namespace *ns, const u8 profile,
const u8 index);
int tomoyo_init_request_info(struct tomoyo_request_info *r,
@@ -1002,6 +1031,7 @@ int tomoyo_socket_listen_permission(struct socket *sock);
int tomoyo_socket_sendmsg_permission(struct socket *sock, struct msghdr *msg,
int size);
int tomoyo_supervisor(struct tomoyo_request_info *r, const char *fmt, ...)
+ __must_hold_shared(&tomoyo_ss)
__printf(2, 3);
int tomoyo_update_domain(struct tomoyo_acl_info *new_entry, const int size,
struct tomoyo_acl_param *param,
@@ -1061,7 +1091,7 @@ void tomoyo_print_ulong(char *buffer, const int buffer_len,
const unsigned long value, const u8 type);
void tomoyo_put_name_union(struct tomoyo_name_union *ptr);
void tomoyo_put_number_union(struct tomoyo_number_union *ptr);
-void tomoyo_read_log(struct tomoyo_io_buffer *head);
+void tomoyo_read_log(struct tomoyo_io_buffer *head) __must_hold(&head->io_sem);
void tomoyo_update_stat(const u8 index);
void tomoyo_warn_oom(const char *function);
void tomoyo_write_log(struct tomoyo_request_info *r, const char *fmt, ...)
@@ -1069,35 +1099,6 @@ void tomoyo_write_log(struct tomoyo_request_info *r, const char *fmt, ...)
void tomoyo_write_log2(struct tomoyo_request_info *r, int len, const char *fmt,
va_list args) __printf(3, 0);
-/********** External variable definitions. **********/
-
-extern bool tomoyo_policy_loaded;
-extern int tomoyo_enabled;
-extern const char * const tomoyo_condition_keyword
-[TOMOYO_MAX_CONDITION_KEYWORD];
-extern const char * const tomoyo_dif[TOMOYO_MAX_DOMAIN_INFO_FLAGS];
-extern const char * const tomoyo_mac_keywords[TOMOYO_MAX_MAC_INDEX
- + TOMOYO_MAX_MAC_CATEGORY_INDEX];
-extern const char * const tomoyo_mode[TOMOYO_CONFIG_MAX_MODE];
-extern const char * const tomoyo_path_keyword[TOMOYO_MAX_PATH_OPERATION];
-extern const char * const tomoyo_proto_keyword[TOMOYO_SOCK_MAX];
-extern const char * const tomoyo_socket_keyword[TOMOYO_MAX_NETWORK_OPERATION];
-extern const u8 tomoyo_index2category[TOMOYO_MAX_MAC_INDEX];
-extern const u8 tomoyo_pn2mac[TOMOYO_MAX_PATH_NUMBER_OPERATION];
-extern const u8 tomoyo_pnnn2mac[TOMOYO_MAX_MKDEV_OPERATION];
-extern const u8 tomoyo_pp2mac[TOMOYO_MAX_PATH2_OPERATION];
-extern struct list_head tomoyo_condition_list;
-extern struct list_head tomoyo_domain_list;
-extern struct list_head tomoyo_name_list[TOMOYO_MAX_HASH];
-extern struct list_head tomoyo_namespace_list;
-extern struct mutex tomoyo_policy_lock;
-extern struct srcu_struct tomoyo_ss;
-extern struct tomoyo_domain_info tomoyo_kernel_domain;
-extern struct tomoyo_policy_namespace tomoyo_kernel_namespace;
-extern unsigned int tomoyo_memory_quota[TOMOYO_MAX_MEMORY_STAT];
-extern unsigned int tomoyo_memory_used[TOMOYO_MAX_MEMORY_STAT];
-extern struct lsm_blob_sizes tomoyo_blob_sizes;
-
/********** Inlined functions. **********/
/**
@@ -1106,6 +1107,7 @@ extern struct lsm_blob_sizes tomoyo_blob_sizes;
* Returns index number for tomoyo_read_unlock().
*/
static inline int tomoyo_read_lock(void)
+ __acquires_shared(&tomoyo_ss)
{
return srcu_read_lock(&tomoyo_ss);
}
@@ -1118,6 +1120,7 @@ static inline int tomoyo_read_lock(void)
* Returns nothing.
*/
static inline void tomoyo_read_unlock(int idx)
+ __releases_shared(&tomoyo_ss)
{
srcu_read_unlock(&tomoyo_ss, idx);
}
diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c
index 90cf0e2969df..0612eac7f2f2 100644
--- a/security/tomoyo/domain.c
+++ b/security/tomoyo/domain.c
@@ -611,6 +611,7 @@ struct tomoyo_domain_info *tomoyo_assign_domain(const char *domainname,
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_environ(struct tomoyo_execve *ee)
+ __must_hold_shared(&tomoyo_ss)
{
struct tomoyo_request_info *r = &ee->r;
struct linux_binprm *bprm = ee->bprm;
diff --git a/security/tomoyo/environ.c b/security/tomoyo/environ.c
index 7f0a471f19b2..bcb05910facc 100644
--- a/security/tomoyo/environ.c
+++ b/security/tomoyo/environ.c
@@ -32,6 +32,7 @@ static bool tomoyo_check_env_acl(struct tomoyo_request_info *r,
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_audit_env_log(struct tomoyo_request_info *r)
+ __must_hold_shared(&tomoyo_ss)
{
return tomoyo_supervisor(r, "misc env %s\n",
r->param.environ.name->name);
diff --git a/security/tomoyo/file.c b/security/tomoyo/file.c
index 8f3b90b6e03d..e9b67dbb38e7 100644
--- a/security/tomoyo/file.c
+++ b/security/tomoyo/file.c
@@ -164,6 +164,7 @@ static bool tomoyo_get_realpath(struct tomoyo_path_info *buf, const struct path
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_audit_path_log(struct tomoyo_request_info *r)
+ __must_hold_shared(&tomoyo_ss)
{
return tomoyo_supervisor(r, "file %s %s\n", tomoyo_path_keyword
[r->param.path.operation],
@@ -178,6 +179,7 @@ static int tomoyo_audit_path_log(struct tomoyo_request_info *r)
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_audit_path2_log(struct tomoyo_request_info *r)
+ __must_hold_shared(&tomoyo_ss)
{
return tomoyo_supervisor(r, "file %s %s %s\n", tomoyo_mac_keywords
[tomoyo_pp2mac[r->param.path2.operation]],
@@ -193,6 +195,7 @@ static int tomoyo_audit_path2_log(struct tomoyo_request_info *r)
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_audit_mkdev_log(struct tomoyo_request_info *r)
+ __must_hold_shared(&tomoyo_ss)
{
return tomoyo_supervisor(r, "file %s %s 0%o %u %u\n",
tomoyo_mac_keywords
@@ -210,6 +213,7 @@ static int tomoyo_audit_mkdev_log(struct tomoyo_request_info *r)
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_audit_path_number_log(struct tomoyo_request_info *r)
+ __must_hold_shared(&tomoyo_ss)
{
const u8 type = r->param.path_number.operation;
u8 radix;
@@ -572,6 +576,7 @@ static int tomoyo_update_path2_acl(const u8 perm,
*/
static int tomoyo_path_permission(struct tomoyo_request_info *r, u8 operation,
const struct tomoyo_path_info *filename)
+ __must_hold_shared(&tomoyo_ss)
{
int error;
diff --git a/security/tomoyo/gc.c b/security/tomoyo/gc.c
index 026e29ea3796..8e2008863af8 100644
--- a/security/tomoyo/gc.c
+++ b/security/tomoyo/gc.c
@@ -23,11 +23,10 @@ static inline void tomoyo_memory_free(void *ptr)
tomoyo_memory_used[TOMOYO_MEMORY_POLICY] -= ksize(ptr);
kfree(ptr);
}
-
-/* The list for "struct tomoyo_io_buffer". */
-static LIST_HEAD(tomoyo_io_buffer_list);
/* Lock for protecting tomoyo_io_buffer_list. */
static DEFINE_SPINLOCK(tomoyo_io_buffer_list_lock);
+/* The list for "struct tomoyo_io_buffer". */
+static __guarded_by(&tomoyo_io_buffer_list_lock) LIST_HEAD(tomoyo_io_buffer_list);
/**
* tomoyo_struct_used_by_io_buffer - Check whether the list element is used by /sys/kernel/security/tomoyo/ users or not.
@@ -385,6 +384,7 @@ static inline void tomoyo_del_number_group(struct list_head *element)
*/
static void tomoyo_try_to_gc(const enum tomoyo_policy_id type,
struct list_head *element)
+ __must_hold(&tomoyo_policy_lock)
{
/*
* __list_del_entry() guarantees that the list element became no longer
@@ -484,6 +484,7 @@ static void tomoyo_try_to_gc(const enum tomoyo_policy_id type,
*/
static void tomoyo_collect_member(const enum tomoyo_policy_id id,
struct list_head *member_list)
+ __must_hold(&tomoyo_policy_lock)
{
struct tomoyo_acl_head *member;
struct tomoyo_acl_head *tmp;
@@ -504,6 +505,7 @@ static void tomoyo_collect_member(const enum tomoyo_policy_id id,
* Returns nothing.
*/
static void tomoyo_collect_acl(struct list_head *list)
+ __must_hold(&tomoyo_policy_lock)
{
struct tomoyo_acl_info *acl;
struct tomoyo_acl_info *tmp;
@@ -627,8 +629,11 @@ static int tomoyo_gc_thread(void *unused)
if (head->users)
continue;
list_del(&head->list);
- kfree(head->read_buf);
- kfree(head->write_buf);
+ /* Safe destruction because no users are left. */
+ context_unsafe(
+ kfree(head->read_buf);
+ kfree(head->write_buf);
+ );
kfree(head);
}
spin_unlock(&tomoyo_io_buffer_list_lock);
@@ -656,11 +661,18 @@ void tomoyo_notify_gc(struct tomoyo_io_buffer *head, const bool is_register)
head->users = 1;
list_add(&head->list, &tomoyo_io_buffer_list);
} else {
- is_write = head->write_buf != NULL;
+ /*
+ * tomoyo_write_control() can concurrently update write_buf from
+ * a non-NULL to new non-NULL pointer with io_sem held.
+ */
+ is_write = data_race(head->write_buf != NULL);
if (!--head->users) {
list_del(&head->list);
- kfree(head->read_buf);
- kfree(head->write_buf);
+ /* Safe destruction because no users are left. */
+ context_unsafe(
+ kfree(head->read_buf);
+ kfree(head->write_buf);
+ );
kfree(head);
}
}
diff --git a/security/tomoyo/mount.c b/security/tomoyo/mount.c
index 2755971f50df..322dfd188ada 100644
--- a/security/tomoyo/mount.c
+++ b/security/tomoyo/mount.c
@@ -28,6 +28,7 @@ static const char * const tomoyo_mounts[TOMOYO_MAX_SPECIAL_MOUNT] = {
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_audit_mount_log(struct tomoyo_request_info *r)
+ __must_hold_shared(&tomoyo_ss)
{
return tomoyo_supervisor(r, "file mount %s %s %s 0x%lX\n",
r->param.mount.dev->name,
@@ -78,6 +79,7 @@ static int tomoyo_mount_acl(struct tomoyo_request_info *r,
const char *dev_name,
const struct path *dir, const char *type,
unsigned long flags)
+ __must_hold_shared(&tomoyo_ss)
{
struct tomoyo_obj_info obj = { };
struct path path;
diff --git a/security/tomoyo/network.c b/security/tomoyo/network.c
index 8dc61335f65e..cfc2a019de1e 100644
--- a/security/tomoyo/network.c
+++ b/security/tomoyo/network.c
@@ -363,6 +363,7 @@ int tomoyo_write_unix_network(struct tomoyo_acl_param *param)
static int tomoyo_audit_net_log(struct tomoyo_request_info *r,
const char *family, const u8 protocol,
const u8 operation, const char *address)
+ __must_hold_shared(&tomoyo_ss)
{
return tomoyo_supervisor(r, "network %s %s %s %s\n", family,
tomoyo_proto_keyword[protocol],
@@ -377,6 +378,7 @@ static int tomoyo_audit_net_log(struct tomoyo_request_info *r,
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_audit_inet_log(struct tomoyo_request_info *r)
+ __must_hold_shared(&tomoyo_ss)
{
char buf[128];
int len;
@@ -402,6 +404,7 @@ static int tomoyo_audit_inet_log(struct tomoyo_request_info *r)
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_audit_unix_log(struct tomoyo_request_info *r)
+ __must_hold_shared(&tomoyo_ss)
{
return tomoyo_audit_net_log(r, "unix", r->param.unix_network.protocol,
r->param.unix_network.operation,
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
* [PATCH v5 33/36] printk: Move locking annotation to printk.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
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
With Sparse support gone, Clang is a bit more strict and warns:
./include/linux/console.h:492:50: error: use of undeclared identifier 'console_mutex'
492 | extern void console_list_unlock(void) __releases(console_mutex);
Since it does not make sense to make console_mutex itself global, move
the annotation to printk.c. Context analysis remains disabled for
printk.c.
This is needed to enable context analysis for modules that include
<linux/console.h>.
Signed-off-by: Marco Elver <elver@google.com>
---
v2:
* New patch.
---
include/linux/console.h | 4 ++--
kernel/printk/printk.c | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/linux/console.h b/include/linux/console.h
index fc9f5c5c1b04..f882833bedf0 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -492,8 +492,8 @@ static inline bool console_srcu_read_lock_is_held(void)
extern int console_srcu_read_lock(void);
extern void console_srcu_read_unlock(int cookie);
-extern void console_list_lock(void) __acquires(console_mutex);
-extern void console_list_unlock(void) __releases(console_mutex);
+extern void console_list_lock(void);
+extern void console_list_unlock(void);
extern struct hlist_head console_list;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 1d765ad242b8..37d16ef27f13 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -245,6 +245,7 @@ int devkmsg_sysctl_set_loglvl(const struct ctl_table *table, int write,
* For console list or console->flags updates
*/
void console_list_lock(void)
+ __acquires(&console_mutex)
{
/*
* In unregister_console() and console_force_preferred_locked(),
@@ -269,6 +270,7 @@ EXPORT_SYMBOL(console_list_lock);
* Counterpart to console_list_lock()
*/
void console_list_unlock(void)
+ __releases(&console_mutex)
{
mutex_unlock(&console_mutex);
}
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
* [PATCH v5 32/36] rhashtable: Enable context analysis
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
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
Enable context analysis for rhashtable, which was used as an initial
test as it contains a combination of RCU, mutex, and bit_spinlock usage.
Users of rhashtable now also benefit from annotations on the API, which
will now warn if the RCU read lock is not held where required.
Signed-off-by: Marco Elver <elver@google.com>
Cc: Thomas Graf <tgraf@suug.ch>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
---
v5:
* Fix annotations for recently modified/added functions.
v4:
* Rename capability -> context analysis.
v2:
* Remove disable/enable_context_analysis() around headers.
---
include/linux/rhashtable.h | 16 +++++++++++++---
lib/Makefile | 2 ++
lib/rhashtable.c | 5 +++--
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 08e664b21f5a..133ccb39137a 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -245,16 +245,17 @@ void *rhashtable_insert_slow(struct rhashtable *ht, const void *key,
void rhashtable_walk_enter(struct rhashtable *ht,
struct rhashtable_iter *iter);
void rhashtable_walk_exit(struct rhashtable_iter *iter);
-int rhashtable_walk_start_check(struct rhashtable_iter *iter) __acquires(RCU);
+int rhashtable_walk_start_check(struct rhashtable_iter *iter) __acquires_shared(RCU);
static inline void rhashtable_walk_start(struct rhashtable_iter *iter)
+ __acquires_shared(RCU)
{
(void)rhashtable_walk_start_check(iter);
}
void *rhashtable_walk_next(struct rhashtable_iter *iter);
void *rhashtable_walk_peek(struct rhashtable_iter *iter);
-void rhashtable_walk_stop(struct rhashtable_iter *iter) __releases(RCU);
+void rhashtable_walk_stop(struct rhashtable_iter *iter) __releases_shared(RCU);
void rhashtable_free_and_destroy(struct rhashtable *ht,
void (*free_fn)(void *ptr, void *arg),
@@ -325,6 +326,7 @@ static inline struct rhash_lock_head __rcu **rht_bucket_insert(
static inline unsigned long rht_lock(struct bucket_table *tbl,
struct rhash_lock_head __rcu **bkt)
+ __acquires(__bitlock(0, bkt))
{
unsigned long flags;
@@ -337,6 +339,7 @@ static inline unsigned long rht_lock(struct bucket_table *tbl,
static inline unsigned long rht_lock_nested(struct bucket_table *tbl,
struct rhash_lock_head __rcu **bucket,
unsigned int subclass)
+ __acquires(__bitlock(0, bucket))
{
unsigned long flags;
@@ -349,6 +352,7 @@ static inline unsigned long rht_lock_nested(struct bucket_table *tbl,
static inline void rht_unlock(struct bucket_table *tbl,
struct rhash_lock_head __rcu **bkt,
unsigned long flags)
+ __releases(__bitlock(0, bkt))
{
lock_map_release(&tbl->dep_map);
bit_spin_unlock(0, (unsigned long *)bkt);
@@ -424,13 +428,14 @@ static inline void rht_assign_unlock(struct bucket_table *tbl,
struct rhash_lock_head __rcu **bkt,
struct rhash_head *obj,
unsigned long flags)
+ __releases(__bitlock(0, bkt))
{
if (rht_is_a_nulls(obj))
obj = NULL;
lock_map_release(&tbl->dep_map);
rcu_assign_pointer(*bkt, (void *)obj);
preempt_enable();
- __release(bitlock);
+ __release(__bitlock(0, bkt));
local_irq_restore(flags);
}
@@ -612,6 +617,7 @@ static __always_inline struct rhash_head *__rhashtable_lookup(
struct rhashtable *ht, const void *key,
const struct rhashtable_params params,
const enum rht_lookup_freq freq)
+ __must_hold_shared(RCU)
{
struct rhashtable_compare_arg arg = {
.ht = ht,
@@ -666,6 +672,7 @@ static __always_inline struct rhash_head *__rhashtable_lookup(
static __always_inline void *rhashtable_lookup(
struct rhashtable *ht, const void *key,
const struct rhashtable_params params)
+ __must_hold_shared(RCU)
{
struct rhash_head *he = __rhashtable_lookup(ht, key, params,
RHT_LOOKUP_NORMAL);
@@ -676,6 +683,7 @@ static __always_inline void *rhashtable_lookup(
static __always_inline void *rhashtable_lookup_likely(
struct rhashtable *ht, const void *key,
const struct rhashtable_params params)
+ __must_hold_shared(RCU)
{
struct rhash_head *he = __rhashtable_lookup(ht, key, params,
RHT_LOOKUP_LIKELY);
@@ -727,6 +735,7 @@ static __always_inline void *rhashtable_lookup_fast(
static __always_inline struct rhlist_head *rhltable_lookup(
struct rhltable *hlt, const void *key,
const struct rhashtable_params params)
+ __must_hold_shared(RCU)
{
struct rhash_head *he = __rhashtable_lookup(&hlt->ht, key, params,
RHT_LOOKUP_NORMAL);
@@ -737,6 +746,7 @@ static __always_inline struct rhlist_head *rhltable_lookup(
static __always_inline struct rhlist_head *rhltable_lookup_likely(
struct rhltable *hlt, const void *key,
const struct rhashtable_params params)
+ __must_hold_shared(RCU)
{
struct rhash_head *he = __rhashtable_lookup(&hlt->ht, key, params,
RHT_LOOKUP_LIKELY);
diff --git a/lib/Makefile b/lib/Makefile
index e755eee4e76f..22d8742bba57 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -50,6 +50,8 @@ lib-$(CONFIG_MIN_HEAP) += min_heap.o
lib-y += kobject.o klist.o
obj-y += lockref.o
+CONTEXT_ANALYSIS_rhashtable.o := y
+
obj-y += bcd.o sort.o parser.o debug_locks.o random32.o \
bust_spinlocks.o kasprintf.o bitmap.o scatterlist.o \
list_sort.o uuid.o iov_iter.o clz_ctz.o \
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index fde0f0e556f8..6074ed5f66f3 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -358,6 +358,7 @@ static int rhashtable_rehash_table(struct rhashtable *ht)
static int rhashtable_rehash_alloc(struct rhashtable *ht,
struct bucket_table *old_tbl,
unsigned int size)
+ __must_hold(&ht->mutex)
{
struct bucket_table *new_tbl;
int err;
@@ -392,6 +393,7 @@ static int rhashtable_rehash_alloc(struct rhashtable *ht,
* bucket locks or concurrent RCU protected lookups and traversals.
*/
static int rhashtable_shrink(struct rhashtable *ht)
+ __must_hold(&ht->mutex)
{
struct bucket_table *old_tbl = rht_dereference(ht->tbl, ht);
unsigned int nelems = atomic_read(&ht->nelems);
@@ -724,7 +726,7 @@ EXPORT_SYMBOL_GPL(rhashtable_walk_exit);
* resize events and always continue.
*/
int rhashtable_walk_start_check(struct rhashtable_iter *iter)
- __acquires(RCU)
+ __acquires_shared(RCU)
{
struct rhashtable *ht = iter->ht;
bool rhlist = ht->rhlist;
@@ -940,7 +942,6 @@ EXPORT_SYMBOL_GPL(rhashtable_walk_peek);
* hash table.
*/
void rhashtable_walk_stop(struct rhashtable_iter *iter)
- __releases(RCU)
{
struct rhashtable *ht;
struct bucket_table *tbl = iter->walker.tbl;
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
* [PATCH v5 31/36] stackdepot: Enable context analysis
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
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
Enable context analysis for stackdepot.
Signed-off-by: Marco Elver <elver@google.com>
---
v4:
* Rename capability -> context analysis.
v2:
* Remove disable/enable_context_analysis() around headers.
---
lib/Makefile | 1 +
lib/stackdepot.c | 20 ++++++++++++++------
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/lib/Makefile b/lib/Makefile
index 89defefbf6c0..e755eee4e76f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -250,6 +250,7 @@ obj-$(CONFIG_POLYNOMIAL) += polynomial.o
# Prevent the compiler from calling builtins like memcmp() or bcmp() from this
# file.
CFLAGS_stackdepot.o += -fno-builtin
+CONTEXT_ANALYSIS_stackdepot.o := y
obj-$(CONFIG_STACKDEPOT) += stackdepot.o
KASAN_SANITIZE_stackdepot.o := n
# In particular, instrumenting stackdepot.c with KMSAN will result in infinite
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index de0b0025af2b..166f50ad8391 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -61,18 +61,18 @@ static unsigned int stack_bucket_number_order;
/* Hash mask for indexing the table. */
static unsigned int stack_hash_mask;
+/* The lock must be held when performing pool or freelist modifications. */
+static DEFINE_RAW_SPINLOCK(pool_lock);
/* Array of memory regions that store stack records. */
-static void **stack_pools;
+static void **stack_pools __pt_guarded_by(&pool_lock);
/* Newly allocated pool that is not yet added to stack_pools. */
static void *new_pool;
/* Number of pools in stack_pools. */
static int pools_num;
/* Offset to the unused space in the currently used pool. */
-static size_t pool_offset = DEPOT_POOL_SIZE;
+static size_t pool_offset __guarded_by(&pool_lock) = DEPOT_POOL_SIZE;
/* Freelist of stack records within stack_pools. */
-static LIST_HEAD(free_stacks);
-/* The lock must be held when performing pool or freelist modifications. */
-static DEFINE_RAW_SPINLOCK(pool_lock);
+static __guarded_by(&pool_lock) LIST_HEAD(free_stacks);
/* Statistics counters for debugfs. */
enum depot_counter_id {
@@ -291,6 +291,7 @@ EXPORT_SYMBOL_GPL(stack_depot_init);
* Initializes new stack pool, and updates the list of pools.
*/
static bool depot_init_pool(void **prealloc)
+ __must_hold(&pool_lock)
{
lockdep_assert_held(&pool_lock);
@@ -338,6 +339,7 @@ static bool depot_init_pool(void **prealloc)
/* Keeps the preallocated memory to be used for a new stack depot pool. */
static void depot_keep_new_pool(void **prealloc)
+ __must_hold(&pool_lock)
{
lockdep_assert_held(&pool_lock);
@@ -357,6 +359,7 @@ static void depot_keep_new_pool(void **prealloc)
* the current pre-allocation.
*/
static struct stack_record *depot_pop_free_pool(void **prealloc, size_t size)
+ __must_hold(&pool_lock)
{
struct stack_record *stack;
void *current_pool;
@@ -391,6 +394,7 @@ static struct stack_record *depot_pop_free_pool(void **prealloc, size_t size)
/* Try to find next free usable entry from the freelist. */
static struct stack_record *depot_pop_free(void)
+ __must_hold(&pool_lock)
{
struct stack_record *stack;
@@ -428,6 +432,7 @@ static inline size_t depot_stack_record_size(struct stack_record *s, unsigned in
/* Allocates a new stack in a stack depot pool. */
static struct stack_record *
depot_alloc_stack(unsigned long *entries, unsigned int nr_entries, u32 hash, depot_flags_t flags, void **prealloc)
+ __must_hold(&pool_lock)
{
struct stack_record *stack = NULL;
size_t record_size;
@@ -486,6 +491,7 @@ depot_alloc_stack(unsigned long *entries, unsigned int nr_entries, u32 hash, dep
}
static struct stack_record *depot_fetch_stack(depot_stack_handle_t handle)
+ __must_not_hold(&pool_lock)
{
const int pools_num_cached = READ_ONCE(pools_num);
union handle_parts parts = { .handle = handle };
@@ -502,7 +508,8 @@ static struct stack_record *depot_fetch_stack(depot_stack_handle_t handle)
return NULL;
}
- pool = stack_pools[pool_index];
+ /* @pool_index either valid, or user passed in corrupted value. */
+ pool = context_unsafe(stack_pools[pool_index]);
if (WARN_ON(!pool))
return NULL;
@@ -515,6 +522,7 @@ static struct stack_record *depot_fetch_stack(depot_stack_handle_t handle)
/* Links stack into the freelist. */
static void depot_free_stack(struct stack_record *stack)
+ __must_not_hold(&pool_lock)
{
unsigned long flags;
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
* [PATCH v5 30/36] kcsan: Enable context analysis
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
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
Enable context analysis for the KCSAN subsystem.
Signed-off-by: Marco Elver <elver@google.com>
---
v4:
* Rename capability -> context analysis.
v3:
* New patch.
---
kernel/kcsan/Makefile | 2 ++
kernel/kcsan/report.c | 11 ++++++++---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
index a45f3dfc8d14..824f30c93252 100644
--- a/kernel/kcsan/Makefile
+++ b/kernel/kcsan/Makefile
@@ -1,4 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
+CONTEXT_ANALYSIS := y
+
KCSAN_SANITIZE := n
KCOV_INSTRUMENT := n
UBSAN_SANITIZE := n
diff --git a/kernel/kcsan/report.c b/kernel/kcsan/report.c
index e95ce7d7a76e..11a48b78f8d1 100644
--- a/kernel/kcsan/report.c
+++ b/kernel/kcsan/report.c
@@ -116,6 +116,7 @@ static DEFINE_RAW_SPINLOCK(report_lock);
* been reported since (now - KCSAN_REPORT_ONCE_IN_MS).
*/
static bool rate_limit_report(unsigned long frame1, unsigned long frame2)
+ __must_hold(&report_lock)
{
struct report_time *use_entry = &report_times[0];
unsigned long invalid_before;
@@ -366,6 +367,7 @@ static int sym_strcmp(void *addr1, void *addr2)
static void
print_stack_trace(unsigned long stack_entries[], int num_entries, unsigned long reordered_to)
+ __must_hold(&report_lock)
{
stack_trace_print(stack_entries, num_entries, 0);
if (reordered_to)
@@ -373,6 +375,7 @@ print_stack_trace(unsigned long stack_entries[], int num_entries, unsigned long
}
static void print_verbose_info(struct task_struct *task)
+ __must_hold(&report_lock)
{
if (!task)
return;
@@ -389,6 +392,7 @@ static void print_report(enum kcsan_value_change value_change,
const struct access_info *ai,
struct other_info *other_info,
u64 old, u64 new, u64 mask)
+ __must_hold(&report_lock)
{
unsigned long reordered_to = 0;
unsigned long stack_entries[NUM_STACK_ENTRIES] = { 0 };
@@ -496,6 +500,7 @@ static void print_report(enum kcsan_value_change value_change,
}
static void release_report(unsigned long *flags, struct other_info *other_info)
+ __releases(&report_lock)
{
/*
* Use size to denote valid/invalid, since KCSAN entirely ignores
@@ -507,13 +512,11 @@ static void release_report(unsigned long *flags, struct other_info *other_info)
/*
* Sets @other_info->task and awaits consumption of @other_info.
- *
- * Precondition: report_lock is held.
- * Postcondition: report_lock is held.
*/
static void set_other_info_task_blocking(unsigned long *flags,
const struct access_info *ai,
struct other_info *other_info)
+ __must_hold(&report_lock)
{
/*
* We may be instrumenting a code-path where current->state is already
@@ -572,6 +575,7 @@ static void set_other_info_task_blocking(unsigned long *flags,
static void prepare_report_producer(unsigned long *flags,
const struct access_info *ai,
struct other_info *other_info)
+ __must_not_hold(&report_lock)
{
raw_spin_lock_irqsave(&report_lock, *flags);
@@ -603,6 +607,7 @@ static void prepare_report_producer(unsigned long *flags,
static bool prepare_report_consumer(unsigned long *flags,
const struct access_info *ai,
struct other_info *other_info)
+ __cond_acquires(true, &report_lock)
{
raw_spin_lock_irqsave(&report_lock, *flags);
--
2.52.0.322.g1dd061c0dc-goog
^ permalink raw reply related
* [PATCH v5 29/36] kcov: Enable context analysis
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
In-Reply-To: <20251219154418.3592607-1-elver@google.com>
Enable context analysis for the KCOV subsystem.
Signed-off-by: Marco Elver <elver@google.com>
---
v4:
* Fix new temporary variable type.
* Rename capability -> context analysis.
v2:
* Remove disable/enable_context_analysis() around headers.
---
kernel/Makefile | 2 ++
kernel/kcov.c | 36 +++++++++++++++++++++++++-----------
2 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/kernel/Makefile b/kernel/Makefile
index e83669841b8c..6785982013dc 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -43,6 +43,8 @@ KASAN_SANITIZE_kcov.o := n
KCSAN_SANITIZE_kcov.o := n
UBSAN_SANITIZE_kcov.o := n
KMSAN_SANITIZE_kcov.o := n
+
+CONTEXT_ANALYSIS_kcov.o := y
CFLAGS_kcov.o := $(call cc-option, -fno-conserve-stack) -fno-stack-protector
obj-y += sched/
diff --git a/kernel/kcov.c b/kernel/kcov.c
index 6563141f5de9..6cbc6e2d8aee 100644
--- a/kernel/kcov.c
+++ b/kernel/kcov.c
@@ -55,13 +55,13 @@ struct kcov {
refcount_t refcount;
/* The lock protects mode, size, area and t. */
spinlock_t lock;
- enum kcov_mode mode;
+ enum kcov_mode mode __guarded_by(&lock);
/* Size of arena (in long's). */
- unsigned int size;
+ unsigned int size __guarded_by(&lock);
/* Coverage buffer shared with user space. */
- void *area;
+ void *area __guarded_by(&lock);
/* Task for which we collect coverage, or NULL. */
- struct task_struct *t;
+ struct task_struct *t __guarded_by(&lock);
/* Collecting coverage from remote (background) threads. */
bool remote;
/* Size of remote area (in long's). */
@@ -391,6 +391,7 @@ void kcov_task_init(struct task_struct *t)
}
static void kcov_reset(struct kcov *kcov)
+ __must_hold(&kcov->lock)
{
kcov->t = NULL;
kcov->mode = KCOV_MODE_INIT;
@@ -400,6 +401,7 @@ static void kcov_reset(struct kcov *kcov)
}
static void kcov_remote_reset(struct kcov *kcov)
+ __must_hold(&kcov->lock)
{
int bkt;
struct kcov_remote *remote;
@@ -419,6 +421,7 @@ static void kcov_remote_reset(struct kcov *kcov)
}
static void kcov_disable(struct task_struct *t, struct kcov *kcov)
+ __must_hold(&kcov->lock)
{
kcov_task_reset(t);
if (kcov->remote)
@@ -435,8 +438,11 @@ static void kcov_get(struct kcov *kcov)
static void kcov_put(struct kcov *kcov)
{
if (refcount_dec_and_test(&kcov->refcount)) {
- kcov_remote_reset(kcov);
- vfree(kcov->area);
+ /* Context-safety: no references left, object being destroyed. */
+ context_unsafe(
+ kcov_remote_reset(kcov);
+ vfree(kcov->area);
+ );
kfree(kcov);
}
}
@@ -491,6 +497,7 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma)
unsigned long size, off;
struct page *page;
unsigned long flags;
+ void *area;
spin_lock_irqsave(&kcov->lock, flags);
size = kcov->size * sizeof(unsigned long);
@@ -499,10 +506,11 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma)
res = -EINVAL;
goto exit;
}
+ area = kcov->area;
spin_unlock_irqrestore(&kcov->lock, flags);
vm_flags_set(vma, VM_DONTEXPAND);
for (off = 0; off < size; off += PAGE_SIZE) {
- page = vmalloc_to_page(kcov->area + off);
+ page = vmalloc_to_page(area + off);
res = vm_insert_page(vma, vma->vm_start + off, page);
if (res) {
pr_warn_once("kcov: vm_insert_page() failed\n");
@@ -522,10 +530,10 @@ static int kcov_open(struct inode *inode, struct file *filep)
kcov = kzalloc(sizeof(*kcov), GFP_KERNEL);
if (!kcov)
return -ENOMEM;
+ spin_lock_init(&kcov->lock);
kcov->mode = KCOV_MODE_DISABLED;
kcov->sequence = 1;
refcount_set(&kcov->refcount, 1);
- spin_lock_init(&kcov->lock);
filep->private_data = kcov;
return nonseekable_open(inode, filep);
}
@@ -556,6 +564,7 @@ static int kcov_get_mode(unsigned long arg)
* vmalloc fault handling path is instrumented.
*/
static void kcov_fault_in_area(struct kcov *kcov)
+ __must_hold(&kcov->lock)
{
unsigned long stride = PAGE_SIZE / sizeof(unsigned long);
unsigned long *area = kcov->area;
@@ -584,6 +593,7 @@ static inline bool kcov_check_handle(u64 handle, bool common_valid,
static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
unsigned long arg)
+ __must_hold(&kcov->lock)
{
struct task_struct *t;
unsigned long flags, unused;
@@ -814,6 +824,7 @@ static inline bool kcov_mode_enabled(unsigned int mode)
}
static void kcov_remote_softirq_start(struct task_struct *t)
+ __must_hold(&kcov_percpu_data.lock)
{
struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
unsigned int mode;
@@ -831,6 +842,7 @@ static void kcov_remote_softirq_start(struct task_struct *t)
}
static void kcov_remote_softirq_stop(struct task_struct *t)
+ __must_hold(&kcov_percpu_data.lock)
{
struct kcov_percpu_data *data = this_cpu_ptr(&kcov_percpu_data);
@@ -896,10 +908,12 @@ void kcov_remote_start(u64 handle)
/* Put in kcov_remote_stop(). */
kcov_get(kcov);
/*
- * Read kcov fields before unlock to prevent races with
- * KCOV_DISABLE / kcov_remote_reset().
+ * Read kcov fields before unlocking kcov_remote_lock to prevent races
+ * with KCOV_DISABLE and kcov_remote_reset(); cannot acquire kcov->lock
+ * here, because it might lead to deadlock given kcov_remote_lock is
+ * acquired _after_ kcov->lock elsewhere.
*/
- mode = kcov->mode;
+ mode = context_unsafe(kcov->mode);
sequence = kcov->sequence;
if (in_task()) {
size = kcov->remote_size;
--
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