* Re: [PATCH] integrity: avoid using __weak functions
From: Nathan Chancellor @ 2026-03-06 22:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Madhavan Srinivasan, Michael Ellerman, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Arnd Bergmann, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Paul Moore, James Morris,
Serge E. Hallyn, Jarkko Sakkinen, Ard Biesheuvel, Coiby Xu,
Nicholas Piggin, Christophe Leroy (CS GROUP),
Christian Borntraeger, Sven Schnelle, Eric Snowberg,
Nick Desaulniers, Bill Wendling, Justin Stitt, Andrew Donnellan,
linuxppc-dev, linux-kernel, linux-s390, linux-arch,
linux-integrity, linux-security-module, keyrings, llvm
In-Reply-To: <20260306150421.270124-1-arnd@kernel.org>
On Fri, Mar 06, 2026 at 04:03:24PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The security/integrity/secure_boot.c file containing only a __weak function
> leads to a build failure with clang:
>
> Cannot find symbol for section 2: .text.
> security/integrity/secure_boot.o: failed
>
> Moving the function into another file that has at least one non-__weak
> symbol would solve this, but this is always fragile.
>
> Avoid __weak definitions entirely and instead move the stub helper into
> an asm-generic header that gets used by default on architectures that
> do not provide their own version. This is consistent with how a lot
> of other architecture specific functionality works, and is more reliable.
>
> Fixes: a0f87ede3bf4 ("integrity: Make arch_ima_get_secureboot integrity-wide")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This is a larger change than I had hoped for.
>
> If you prefer a different way to address the build failure, please
> treat this as a Reported-by when you apply your own fix
> ---
> arch/powerpc/include/asm/secure_boot.h | 6 +++
> arch/powerpc/kernel/secure_boot.c | 1 -
> arch/s390/include/asm/secure_boot.h | 9 +++++
> include/asm-generic/Kbuild | 1 +
> include/asm-generic/secure_boot.h | 37 +++++++++++++++++++
> include/linux/secure_boot.h | 8 +---
> security/integrity/Makefile | 2 +-
> .../integrity/platform_certs/load_powerpc.c | 2 +-
> security/integrity/secure_boot.c | 16 --------
> 9 files changed, 56 insertions(+), 26 deletions(-)
> create mode 100644 arch/s390/include/asm/secure_boot.h
> create mode 100644 include/asm-generic/secure_boot.h
> delete mode 100644 security/integrity/secure_boot.c
Thanks, I noticed this as well. The version I came up with and have been
locally testing is the following, which is a little bit more compact.
arch/Kconfig | 3 +++
arch/powerpc/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/s390/kernel/ipl.c | 10 +++++-----
include/linux/secure_boot.h | 4 ++++
security/integrity/Makefile | 2 +-
security/integrity/secure_boot.c | 16 ----------------
7 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 102ddbd4298e..a6d1c8cc1d64 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1841,4 +1841,7 @@ config ARCH_WANTS_PRE_LINK_VMLINUX
config ARCH_HAS_CPU_ATTACK_VECTORS
bool
+config HAVE_ARCH_GET_SECUREBOOT
+ def_bool EFI
+
endmenu
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c28776660246..e76d6cf0c403 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -1062,6 +1062,7 @@ config PPC_SECURE_BOOT
depends on IMA_ARCH_POLICY
imply IMA_SECURE_AND_OR_TRUSTED_BOOT
select PSERIES_PLPKS if PPC_PSERIES
+ select HAVE_ARCH_GET_SECUREBOOT
help
Systems with firmware secure boot enabled need to define security
policies to extend secure boot to the OS. This config allows a user
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 24695ea29d5b..76f191dd208b 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -181,6 +181,7 @@ config S390
select GENERIC_IOREMAP if PCI
select HAVE_ALIGNED_STRUCT_PAGE
select HAVE_ARCH_AUDITSYSCALL
+ select HAVE_ARCH_GET_SECUREBOOT
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN
diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
index 2d01a1713938..3c346b02ceb9 100644
--- a/arch/s390/kernel/ipl.c
+++ b/arch/s390/kernel/ipl.c
@@ -2388,6 +2388,11 @@ void __no_stack_protector s390_reset_system(void)
diag_amode31_ops.diag308_reset();
}
+bool arch_get_secureboot(void)
+{
+ return ipl_secure_flag;
+}
+
#ifdef CONFIG_KEXEC_FILE
int ipl_report_add_component(struct ipl_report *report, struct kexec_buf *kbuf,
@@ -2505,11 +2510,6 @@ void *ipl_report_finish(struct ipl_report *report)
return buf;
}
-bool arch_get_secureboot(void)
-{
- return ipl_secure_flag;
-}
-
int ipl_report_free(struct ipl_report *report)
{
struct ipl_report_component *comp, *ncomp;
diff --git a/include/linux/secure_boot.h b/include/linux/secure_boot.h
index 3ded3f03655c..d17e92351567 100644
--- a/include/linux/secure_boot.h
+++ b/include/linux/secure_boot.h
@@ -10,10 +10,14 @@
#include <linux/types.h>
+#ifdef CONFIG_HAVE_ARCH_GET_SECUREBOOT
/*
* Returns true if the platform secure boot is enabled.
* Returns false if disabled or not supported.
*/
bool arch_get_secureboot(void);
+#else
+static inline bool arch_get_secureboot(void) { return false; }
+#endif
#endif /* _LINUX_SECURE_BOOT_H */
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 548665e2b702..45dfdedbdad4 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -5,7 +5,7 @@
obj-$(CONFIG_INTEGRITY) += integrity.o
-integrity-y := iint.o secure_boot.o
+integrity-y := iint.o
integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
diff --git a/security/integrity/secure_boot.c b/security/integrity/secure_boot.c
deleted file mode 100644
index fc2693c286f8..000000000000
--- a/security/integrity/secure_boot.c
+++ /dev/null
@@ -1,16 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (C) 2026 Red Hat, Inc. All Rights Reserved.
- *
- * Author: Coiby Xu <coxu@redhat.com>
- */
-#include <linux/secure_boot.h>
-
-/*
- * Default weak implementation.
- * Architectures that support secure boot must override this.
- */
-__weak bool arch_get_secureboot(void)
-{
- return false;
-}
^ permalink raw reply related
* Re: [PATCH v3 00/15] Further centralising of directory locking for name ops.
From: NeilBrown @ 2026-03-06 21:16 UTC (permalink / raw)
To: Christian Brauner
Cc: Alexander Viro, David Howells, Jan Kara, Chuck Lever, Jeff Layton,
Miklos Szeredi, Amir Goldstein, John Johansen, Paul Moore,
James Morris, Serge E. Hallyn, Stephen Smalley, Darrick J. Wong,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260306-wildfremd-wildfremd-43848a9e91cd@brauner>
On Fri, 06 Mar 2026, Christian Brauner wrote:
> On Thu, Mar 05, 2026 at 12:24:38PM +1100, NeilBrown wrote:
> >
> > Hi Christian,
> > do you have thoughts about this series? Any idea when you might have
> > time to review and (hopefully) apply them?
>
> Sorry, for the delay I picked it up but have two minor comments.
>
Thanks! I'll take a little while to examine the cachefiles code.
Thanks for the thorough review!
NeilBrown
^ permalink raw reply
* Re: [PATCH v3 05/15] Apparmor: Use simple_start_creating() / simple_done_creating()
From: NeilBrown @ 2026-03-06 21:12 UTC (permalink / raw)
To: Christian Brauner
Cc: Alexander Viro, David Howells, Jan Kara, Chuck Lever, Jeff Layton,
Miklos Szeredi, Amir Goldstein, John Johansen, Paul Moore,
James Morris, Serge E. Hallyn, Stephen Smalley, Darrick J. Wong,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260306-fastnacht-kernig-3b350bd2fab0@brauner>
On Fri, 06 Mar 2026, Christian Brauner wrote:
> On Wed, Feb 25, 2026 at 09:16:50AM +1100, NeilBrown wrote:
> > From: NeilBrown <neil@brown.name>
> >
> > Instead of explicitly locking the parent and performing a look up in
> > apparmor, use simple_start_creating(), and then simple_done_creating()
> > to unlock and drop the dentry.
> >
> > This removes the need to check for an existing entry (as
> > simple_start_creating() acts like an exclusive create and can return
> > -EEXIST), simplifies error paths, and keeps dir locking code
> > centralised.
> >
> > Reviewed-by: Jeff Layton <jlayton@kernel.org>
> > Signed-off-by: NeilBrown <neil@brown.name>
> > ---
>
> Fwiw, I think this fixes a reference count leak:
>
> The old aafs_create returned dentries with refcount=2 (one from
> lookup_noperm, one from dget in __aafs_setup_d_inode). The cleanup path
> aafs_remove puts one reference leaving one reference that didn't get
> cleaned up.
>
> After your changes this is now correct as simple_done_creating() puts
> the lookup reference.
>
Yes, I think you are correct. I remember reviewing how ->dents was used
to confirm that the new refcounting was correct. I didn't notice at the
time that the old was wrong.
Thanks,
NeilBrown
^ permalink raw reply
* Re: LSM namespacing API
From: Casey Schaufler @ 2026-03-06 21:01 UTC (permalink / raw)
To: Dr. Greg, Paul Moore
Cc: Stephen Smalley, Ondrej Mosnacek, linux-security-module, selinux,
John Johansen
In-Reply-To: <20260306174815.GA9953@wind.enjellic.com>
On 3/6/2026 9:48 AM, Dr. Greg wrote:
> On Tue, Mar 03, 2026 at 11:46:53AM -0500, Paul Moore wrote:
>
> Good morning, I hope the week is winding down well for everyone.
>
>> On Tue, Mar 3, 2026 at 8:30???AM Stephen Smalley
>>> I think my only caveat here is that your proposal is quite a bit more
>>> complex than what I implemented here:
>>> [1] https://lore.kernel.org/selinux/20251003190959.3288-2-stephen.smalley.work@gmail.com/
>>> [2] https://lore.kernel.org/selinux/20251003191328.3605-1-stephen.smalley.work@gmail.com/
>>> and I'm not sure the extra complexity is worth it.
>>>
>>> In particular:
>>> 1. Immediately unsharing the namespace upon lsm_set_self_attr() allows
>>> the caller to immediately and unambiguously know if the operation is
>>> supported and allowed ...
>> Performing the unshare operation immediately looks much less like a
>> LSM attribute and more like its own syscall. That isn't a problem
>> in my eyes, it just means if this is the direction we want to go we
>> should implement a lsm_unshare(2) API, or something similar.
> Stephen's take on this is correct, the least complicated path forward
> is a simple call, presumably lsm_unshare(2), that instructs the LSM(s)
> to carry out whatever is needed to create a new security namespace.
>
> There are only two public implementations of what can be referred to
> as major security namespacing efforts; Stephen's work with SeLinux and
> our TSEM implementation.
Please be just a tiny bit careful before you make this sort of assertion:
https://lwn.net/Articles/645403/
^ permalink raw reply
* Re: LSM namespacing API
From: Dr. Greg @ 2026-03-06 17:48 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, Ondrej Mosnacek, linux-security-module, selinux,
John Johansen
In-Reply-To: <CAHC9VhTGruOPJ+NWZT8vw1bjXzkB4DSPFmWd1pC=J2jTYHP5BA@mail.gmail.com>
On Tue, Mar 03, 2026 at 11:46:53AM -0500, Paul Moore wrote:
Good morning, I hope the week is winding down well for everyone.
> On Tue, Mar 3, 2026 at 8:30???AM Stephen Smalley
> > I think my only caveat here is that your proposal is quite a bit more
> > complex than what I implemented here:
> > [1] https://lore.kernel.org/selinux/20251003190959.3288-2-stephen.smalley.work@gmail.com/
> > [2] https://lore.kernel.org/selinux/20251003191328.3605-1-stephen.smalley.work@gmail.com/
> > and I'm not sure the extra complexity is worth it.
> >
> > In particular:
> > 1. Immediately unsharing the namespace upon lsm_set_self_attr() allows
> > the caller to immediately and unambiguously know if the operation is
> > supported and allowed ...
> Performing the unshare operation immediately looks much less like a
> LSM attribute and more like its own syscall. That isn't a problem
> in my eyes, it just means if this is the direction we want to go we
> should implement a lsm_unshare(2) API, or something similar.
Stephen's take on this is correct, the least complicated path forward
is a simple call, presumably lsm_unshare(2), that instructs the LSM(s)
to carry out whatever is needed to create a new security namespace.
There are only two public implementations of what can be referred to
as major security namespacing efforts; Stephen's work with SeLinux and
our TSEM implementation. Given that both initiatives, which are
significantly different, independently settled on the same approach
seems to suggest it is a mechanism that enjoys successful field
experience.
The larger and more important question would seem to be what type of
common namespace infrastructure actions would be amortizable across
all the different types of security models that are supported by the
LSM infrastructure.
We haven't seen any further comments or patches out of what Christian
and Lennart are attempting to implement with respect to the concept of
a security namespace in order to support their Amutable initiative.
It seems reasonable that one of the common pieces of infrastructure
would be the allocation of a security namespace specific context of
data, i.e. an LSM security blob, that Christian had proposed patches
for.
Given the potential security implications of creating a security
namespace, we assume that the 'unshare' operation would call an LSM
hook that interrogated the current LSM stack for permission to do so.
For example, it would seem reasonable that the Lockdown LSM would want
the ability to block attempts to create alternate security namespaces.
> > 3. We don't need to introduce a new CLONE_* flag at all or introduce
> > new or changed LSM hooks to clone/unshare,
>
> While I think we could get away with a new lsm_unshare(2) syscall, I
> have zero interest in an lsm_clone(2) syscall. If we do away with the
> namespace related LSM attributes and rely entirely on a lsm_unshare(2)
> syscall, would everyone be okay with that?
We assume that there is agreement on the fact that an orchestrator
needs to have the ability to specify attributes or policy for the new
namespace that is being created.
Given that, there will be a need to be able specify the
characteristics that configure the new namespace which will go into
effect, atomically, at the time the proposed lsm_unshare(s) system
call executes.
For example, now that IMA is an LSM, it would be reasonable to assume
that the cryptographic hash function used for the integrity
measurements could be specified as part of the namespace setup call
for a new IMA namespace.
> (I think we would still want/need the procfs API)
That is only needed if there is a desire to support the ability of a
process to enter a security namespace that it is not part of.
That is potentially a useful feature, suffice it to say however, there
are a host of issues involved with this. We've had significant field
experience with this concept and its implications, we don't get the
sense that the mainline LSM's have had the opportunity to understand
the implications of what this would mean.
> > All that said, I'm willing to wire up the SELinux namespaces
> > implementation to the proposed interface if someone implements the
> > necessary plumbing, but I'm not sure it's better.
> I'd really like to hear from some of the other LSMs before we start
> diving into the code. It may sound funny, but from my perspective
> doing the work to get the API definition "right" is far more
> important than implementing it.
It isn't funny, it is pragmatic, particularily in this case.
The primary challenge will be the tension that exists between the fact
that there is no practical field experience with security namespacing,
particularily with respect to its security implications, in the
mainline kernel and the Linux userspace guarantee policy, that once an
API/ABI becomes visible to userspace it cannot be changed.
At this point in time the most basic and common security namespace
infrastructure to get right and lay down would seem to be three fold:
1.) A security check for whether or not namespace creation should be
allowed.
2.) The ability to specify characteristics of a new security namespace
and to invoke those atomically when a process requests that a new
security namespace be created.
3.) Allocation of an LSM namespace 'blob' of memory that can be used
to implement the security context for the new namespace.
TSEM obviously isn't in the kernel, so understandably, our perspective
may not hold much value in these quarters.
It is, however, the most significant security namespace implementation
that has been done for Linux, with respect to its scope and
capabilities. Perhaps most importantly, we have had ten years of
experience dealing with all of these issues and their implications,
particularly from a security perspective.
Given that, we would be happy to test fire TSEM against any proposed
infrastructure changes that focus on the generic needs of security
namespacing.
> paul-moore.com
Have a good weekend.
As always,
Dr. Greg
The Quixote Project - Flailing at the Travails of Cybersecurity
https://github.com/Quixote-Project
^ permalink raw reply
* [PATCH] integrity: avoid using __weak functions
From: Arnd Bergmann @ 2026-03-06 15:03 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Arnd Bergmann, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Paul Moore, James Morris,
Serge E. Hallyn, Jarkko Sakkinen, Nathan Chancellor,
Ard Biesheuvel, Coiby Xu
Cc: Nicholas Piggin, Christophe Leroy (CS GROUP),
Christian Borntraeger, Sven Schnelle, Eric Snowberg,
Nick Desaulniers, Bill Wendling, Justin Stitt, Andrew Donnellan,
linuxppc-dev, linux-kernel, linux-s390, linux-arch,
linux-integrity, linux-security-module, keyrings, llvm
From: Arnd Bergmann <arnd@arndb.de>
The security/integrity/secure_boot.c file containing only a __weak function
leads to a build failure with clang:
Cannot find symbol for section 2: .text.
security/integrity/secure_boot.o: failed
Moving the function into another file that has at least one non-__weak
symbol would solve this, but this is always fragile.
Avoid __weak definitions entirely and instead move the stub helper into
an asm-generic header that gets used by default on architectures that
do not provide their own version. This is consistent with how a lot
of other architecture specific functionality works, and is more reliable.
Fixes: a0f87ede3bf4 ("integrity: Make arch_ima_get_secureboot integrity-wide")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This is a larger change than I had hoped for.
If you prefer a different way to address the build failure, please
treat this as a Reported-by when you apply your own fix
---
arch/powerpc/include/asm/secure_boot.h | 6 +++
arch/powerpc/kernel/secure_boot.c | 1 -
arch/s390/include/asm/secure_boot.h | 9 +++++
include/asm-generic/Kbuild | 1 +
include/asm-generic/secure_boot.h | 37 +++++++++++++++++++
include/linux/secure_boot.h | 8 +---
security/integrity/Makefile | 2 +-
.../integrity/platform_certs/load_powerpc.c | 2 +-
security/integrity/secure_boot.c | 16 --------
9 files changed, 56 insertions(+), 26 deletions(-)
create mode 100644 arch/s390/include/asm/secure_boot.h
create mode 100644 include/asm-generic/secure_boot.h
delete mode 100644 security/integrity/secure_boot.c
diff --git a/arch/powerpc/include/asm/secure_boot.h b/arch/powerpc/include/asm/secure_boot.h
index a2ff556916c6..db72dcdf5bb3 100644
--- a/arch/powerpc/include/asm/secure_boot.h
+++ b/arch/powerpc/include/asm/secure_boot.h
@@ -10,11 +10,17 @@
#ifdef CONFIG_PPC_SECURE_BOOT
+bool arch_get_secureboot(void);
bool is_ppc_secureboot_enabled(void);
bool is_ppc_trustedboot_enabled(void);
#else
+static inline bool arch_get_secureboot(void)
+{
+ return false;
+}
+
static inline bool is_ppc_secureboot_enabled(void)
{
return false;
diff --git a/arch/powerpc/kernel/secure_boot.c b/arch/powerpc/kernel/secure_boot.c
index 28436c1599e0..e3ea46124180 100644
--- a/arch/powerpc/kernel/secure_boot.c
+++ b/arch/powerpc/kernel/secure_boot.c
@@ -7,7 +7,6 @@
#include <linux/of.h>
#include <linux/secure_boot.h>
#include <linux/string_choices.h>
-#include <asm/secure_boot.h>
static struct device_node *get_ppc_fw_sb_node(void)
{
diff --git a/arch/s390/include/asm/secure_boot.h b/arch/s390/include/asm/secure_boot.h
new file mode 100644
index 000000000000..4086fdfb9e5c
--- /dev/null
+++ b/arch/s390/include/asm/secure_boot.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_S390_SECURE_BOOT_H
+#define _ASM_S390_SECURE_BOOT_H
+
+#include <linux/types.h
+
+bool arch_get_secureboot(void);
+
+#endif
diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
index 0f97f7b594c3..8c0a499141fb 100644
--- a/include/asm-generic/Kbuild
+++ b/include/asm-generic/Kbuild
@@ -51,6 +51,7 @@ mandatory-y += rqspinlock.h
mandatory-y += runtime-const.h
mandatory-y += rwonce.h
mandatory-y += sections.h
+mandatory-y += secure_boot.h
mandatory-y += serial.h
mandatory-y += shmparam.h
mandatory-y += simd.h
diff --git a/include/asm-generic/secure_boot.h b/include/asm-generic/secure_boot.h
new file mode 100644
index 000000000000..08d8e294576c
--- /dev/null
+++ b/include/asm-generic/secure_boot.h
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026 Red Hat, Inc. All Rights Reserved.
+ *
+ * Author: Coiby Xu <coxu@redhat.com>
+ */
+#ifndef _ASM_SECURE_BOOT_H
+#define _ASM_SECURE_BOOT_H
+
+
+#include <linux/types.h>
+
+#ifdef CONFIG_EFI
+
+/*
+ * Default implementation.
+ * Architectures that support secure boot must override this.
+ *
+ * Returns true if the platform secure boot is enabled.
+ * Returns false if disabled or not supported.
+ */
+bool arch_get_secureboot(void);
+
+#else
+
+/*
+ * Default implementation.
+ * Architectures that support secure boot must override this.
+ */
+static inline bool arch_get_secureboot(void)
+{
+ return false;
+}
+
+#endif
+
+#endif
diff --git a/include/linux/secure_boot.h b/include/linux/secure_boot.h
index 3ded3f03655c..9ddfbe109b1d 100644
--- a/include/linux/secure_boot.h
+++ b/include/linux/secure_boot.h
@@ -8,12 +8,6 @@
#ifndef _LINUX_SECURE_BOOT_H
#define _LINUX_SECURE_BOOT_H
-#include <linux/types.h>
-
-/*
- * Returns true if the platform secure boot is enabled.
- * Returns false if disabled or not supported.
- */
-bool arch_get_secureboot(void);
+#include <asm/secure_boot.h>
#endif /* _LINUX_SECURE_BOOT_H */
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 548665e2b702..45dfdedbdad4 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -5,7 +5,7 @@
obj-$(CONFIG_INTEGRITY) += integrity.o
-integrity-y := iint.o secure_boot.o
+integrity-y := iint.o
integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c
index 714c961a00f5..ab74e947a8bc 100644
--- a/security/integrity/platform_certs/load_powerpc.c
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -10,7 +10,7 @@
#include <linux/cred.h>
#include <linux/err.h>
#include <linux/slab.h>
-#include <asm/secure_boot.h>
+#include <linux/secure_boot.h>
#include <asm/secvar.h>
#include "keyring_handler.h"
#include "../integrity.h"
diff --git a/security/integrity/secure_boot.c b/security/integrity/secure_boot.c
deleted file mode 100644
index fc2693c286f8..000000000000
--- a/security/integrity/secure_boot.c
+++ /dev/null
@@ -1,16 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (C) 2026 Red Hat, Inc. All Rights Reserved.
- *
- * Author: Coiby Xu <coxu@redhat.com>
- */
-#include <linux/secure_boot.h>
-
-/*
- * Default weak implementation.
- * Architectures that support secure boot must override this.
- */
-__weak bool arch_get_secureboot(void)
-{
- return false;
-}
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v1] landlock/tsync: fix null-ptr-deref in cancel_tsync_works()
From: Günther Noack @ 2026-03-06 13:39 UTC (permalink / raw)
To: Jiayuan Chen
Cc: linux-security-module, syzbot+911d99dc200feac03ea6,
Mickaël Salaün, Paul Moore, James Morris,
Serge E. Hallyn, linux-kernel
In-Reply-To: <20260306092214.63179-1-jiayuan.chen@linux.dev>
On Fri, Mar 06, 2026 at 05:22:13PM +0800, Jiayuan Chen wrote:
> cancel_tsync_works() iterates over works->works[0..size-1] and calls
> task_work_cancel() on each entry. task_work_cancel() leads to
> task_work_pending(), which dereferences task->task_works. If
> works->works[i]->task is NULL, this causes a null-ptr-deref:
>
> KASAN: null-ptr-deref in range [0x00000000000009a0-0x00000000000009a7]
> RIP: 0010:task_work_pending include/linux/task_work.h:26 [inline]
> RIP: 0010:task_work_cancel_match+0x86/0x250 kernel/task_work.c:124
> RSP: 0018:ffffc90003597ba0 EFLAGS: 00010202
> RAX: 0000000000000134 RBX: 0000000000000000 RCX: ffffc900106b1000
> RDX: 0000000000080000 RSI: ffffffff81d13236 RDI: 0000000000000000
> RBP: 1ffff920006b2f77 R08: 0000000000000007 R09: 0000000000000000
> R10: 0000000000000002 R11: 0000000000000000 R12: ffffffff81d12dd0
> R13: ffff88802c045100 R14: dffffc0000000000 R15: 00000000000009a0
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000000110c3ea90c CR3: 0000000037f63000 CR4: 00000000003526f0
> DR0: 0000000000000003 DR1: 00000000000001f8 DR2: 000000000000008e
> DR3: 000000000000057a DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> task_work_cancel+0x23/0x30 kernel/task_work.c:187
> cancel_tsync_works security/landlock/tsync.c:415 [inline]
> landlock_restrict_sibling_threads+0xafe/0x1280 security/landlock/tsync.c:533
> __do_sys_landlock_restrict_self+0x5c9/0x9e0 security/landlock/syscalls.c:574
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0x106/0xf80 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7f859b39c629
> RSP: 002b:00007f85991b2028 EFLAGS: 00000246 ORIG_RAX: 00000000000001be
> RAX: ffffffffffffffda RBX: 00007f859b616270 RCX: 00007f859b39c629
> RDX: 0000000000000000 RSI: 000000000000000a RDI: 0000000000000003
> RBP: 00007f859b432b39 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> R13: 00007f859b616308 R14: 00007f859b616270 R15: 00007ffcff084488
>
> The root cause is a race in schedule_task_work(). tsync_works_provide()
> increments works->size and stores the task reference in ctx->task *before*
> task_work_add() is called. A thread can race to call do_exit() in the
> window between the PF_EXITING check and task_work_add(), causing
> task_work_add() to return -ESRCH. The error path then drops the task
> reference and sets ctx->task = NULL, but works->size remains incremented.
> A subsequent call to cancel_tsync_works() iterates up to the stale size
> and passes the NULL task pointer to task_work_cancel().
>
> Fix this by decrementing works->size in the task_work_add() error path,
> so the failed slot is rolled back and cancel_tsync_works() never iterates
> over it. The slot is naturally reused in subsequent iterations since
> tsync_works_provide() always picks works->works[works->size].
>
> As a defensive measure, also add a WARN_ONCE() guard in cancel_tsync_works()
> to catch any future NULL task pointer before dereferencing it.
>
> Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
> Reported-by: syzbot+911d99dc200feac03ea6@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=911d99dc200feac03ea6
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
> security/landlock/tsync.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index 0d2b9c646030..e6d742529484 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -381,14 +381,14 @@ static bool schedule_task_work(struct tsync_works *works,
> err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
> if (err) {
> /*
> - * task_work_add() only fails if the task is about to exit. We
> - * checked that earlier, but it can happen as a race. Resume
> - * without setting an error, as the task is probably gone in the
> - * next loop iteration. For consistency, remove the task from ctx
> - * so that it does not look like we handed it a task_work.
> + * task_work_add() only fails if the task is about to exit.
> + * We checked PF_EXITING earlier, but the thread can race to
> + * exit between that check and task_work_add(). Roll back the
> + * slot so cancel_tsync_works() never sees a NULL task pointer.
> */
> put_task_struct(ctx->task);
> ctx->task = NULL;
> + works->size--;
>
> atomic_dec(&shared_ctx->num_preparing);
> atomic_dec(&shared_ctx->num_unfinished);
> @@ -412,6 +412,11 @@ static void cancel_tsync_works(struct tsync_works *works,
> int i;
>
> for (i = 0; i < works->size; i++) {
> + if (WARN_ONCE(!works->works[i]->task,
> + "landlock: unexpected NULL task in tsync slot %d\n",
> + i))
> + continue;
> +
> if (!task_work_cancel(works->works[i]->task,
> &works->works[i]->work))
> continue;
> --
> 2.43.0
>
Thanks for the patch!
This bug is already fixed on Mickaël's "next" branch.
The code review has happened in
https://lore.kernel.org/all/20260217122341.2359582-1-mic@digikod.net/
—Günther
^ permalink raw reply
* Re: [PATCH v3 01/12] vfs: widen inode hash/lookup functions to u64
From: Christian Brauner @ 2026-03-06 13:28 UTC (permalink / raw)
To: Jeff Layton
Cc: Christoph Hellwig, Alexander Viro, Jan Kara, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Dan Williams, Eric Biggers,
Theodore Y. Ts'o, Muchun Song, Oscar Salvador,
David Hildenbrand, David Howells, Paulo Alcantara, Andreas Dilger,
Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust, Anna Schumaker,
Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Steve French, Ronnie Sahlberg, Shyam Prasad N, Bharath SM,
Alexander Aring, Ryusuke Konishi, Viacheslav Dubeyko,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, David Sterba, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Ilya Dryomov, Alex Markuze, Jan Harkes, coda, Nicolas Pitre,
Tyler Hicks, Amir Goldstein, John Paul Adrian Glaubitz,
Yangtao Li, Mikulas Patocka, David Woodhouse, Richard Weinberger,
Dave Kleikamp, Konstantin Komarov, Mark Fasheh, Joel Becker,
Joseph Qi, Mike Marshall, Martin Brandenburg, Miklos Szeredi,
Anders Larsen, Zhihao Cheng, Damien Le Moal, Naohiro Aota,
Johannes Thumshirn, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Fan Wu, Stephen Smalley, Ondrej Mosnacek,
Casey Schaufler, Alex Deucher, Christian König, David Airlie,
Simona Vetter, Sumit Semwal, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Darrick J. Wong, Martin Schiller, Eric Paris,
Joerg Reuter, Marcel Holtmann, Johan Hedberg,
Luiz Augusto von Dentz, Oliver Hartkopp, Marc Kleine-Budde,
David Ahern, Neal Cardwell, Steffen Klassert, Herbert Xu,
Remi Denis-Courmont, Marcelo Ricardo Leitner, Xin Long,
Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, linux-fsdevel, linux-kernel, linux-trace-kernel,
nvdimm, fsverity, linux-mm, netfs, linux-ext4, linux-f2fs-devel,
linux-nfs, linux-cifs, samba-technical, linux-nilfs, v9fs,
linux-afs, autofs, ceph-devel, codalist, ecryptfs, linux-mtd,
jfs-discussion, ntfs3, ocfs2-devel, devel, linux-unionfs,
apparmor, linux-security-module, linux-integrity, selinux,
amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <c1845a4b8d35d367953ac6cbfcf91ac36958ba51.camel@kernel.org>
On Fri, Mar 06, 2026 at 07:03:15AM -0500, Jeff Layton wrote:
> On Thu, 2026-03-05 at 06:24 -0800, Christoph Hellwig wrote:
> > > extern struct inode *ilookup5_nowait(struct super_block *sb,
> > > - unsigned long hashval, int (*test)(struct inode *, void *),
> > > + u64 hashval, int (*test)(struct inode *, void *),
> > > void *data, bool *isnew);
> > > -extern struct inode *ilookup5(struct super_block *sb, unsigned long hashval,
> > > +extern struct inode *ilookup5(struct super_block *sb, u64 hashval,
> > > int (*test)(struct inode *, void *), void *data);
> >
> > ...
> >
> > Can you please drop all these pointless externs while you're at it?
> >
>
> I was planning to do that, but then Christian merged it!
>
> I'll do a patch on top of this that does this in the range of fs.h that
> the patch touches. Christian can throw it on top of the series, and
> that shouldn't be too bad for backports.
I can easily drop those so no need to resend for stuff like this as per
the usual protocol.
^ permalink raw reply
* Re: [PATCH v3 01/12] vfs: widen inode hash/lookup functions to u64
From: Jeff Layton @ 2026-03-06 12:03 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Alexander Viro, Christian Brauner, Jan Kara, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Dan Williams, Eric Biggers,
Theodore Y. Ts'o, Muchun Song, Oscar Salvador,
David Hildenbrand, David Howells, Paulo Alcantara, Andreas Dilger,
Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust, Anna Schumaker,
Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Steve French, Ronnie Sahlberg, Shyam Prasad N, Bharath SM,
Alexander Aring, Ryusuke Konishi, Viacheslav Dubeyko,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, David Sterba, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Ilya Dryomov, Alex Markuze, Jan Harkes, coda, Nicolas Pitre,
Tyler Hicks, Amir Goldstein, John Paul Adrian Glaubitz,
Yangtao Li, Mikulas Patocka, David Woodhouse, Richard Weinberger,
Dave Kleikamp, Konstantin Komarov, Mark Fasheh, Joel Becker,
Joseph Qi, Mike Marshall, Martin Brandenburg, Miklos Szeredi,
Anders Larsen, Zhihao Cheng, Damien Le Moal, Naohiro Aota,
Johannes Thumshirn, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Fan Wu, Stephen Smalley, Ondrej Mosnacek,
Casey Schaufler, Alex Deucher, Christian König, David Airlie,
Simona Vetter, Sumit Semwal, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Darrick J. Wong, Martin Schiller, Eric Paris,
Joerg Reuter, Marcel Holtmann, Johan Hedberg,
Luiz Augusto von Dentz, Oliver Hartkopp, Marc Kleine-Budde,
David Ahern, Neal Cardwell, Steffen Klassert, Herbert Xu,
Remi Denis-Courmont, Marcelo Ricardo Leitner, Xin Long,
Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, linux-fsdevel, linux-kernel, linux-trace-kernel,
nvdimm, fsverity, linux-mm, netfs, linux-ext4, linux-f2fs-devel,
linux-nfs, linux-cifs, samba-technical, linux-nilfs, v9fs,
linux-afs, autofs, ceph-devel, codalist, ecryptfs, linux-mtd,
jfs-discussion, ntfs3, ocfs2-devel, devel, linux-unionfs,
apparmor, linux-security-module, linux-integrity, selinux,
amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <aamSFgXhrORAJLBC@infradead.org>
On Thu, 2026-03-05 at 06:24 -0800, Christoph Hellwig wrote:
> > extern struct inode *ilookup5_nowait(struct super_block *sb,
> > - unsigned long hashval, int (*test)(struct inode *, void *),
> > + u64 hashval, int (*test)(struct inode *, void *),
> > void *data, bool *isnew);
> > -extern struct inode *ilookup5(struct super_block *sb, unsigned long hashval,
> > +extern struct inode *ilookup5(struct super_block *sb, u64 hashval,
> > int (*test)(struct inode *, void *), void *data);
>
> ...
>
> Can you please drop all these pointless externs while you're at it?
>
I was planning to do that, but then Christian merged it!
I'll do a patch on top of this that does this in the range of fs.h that
the patch touches. Christian can throw it on top of the series, and
that shouldn't be too bad for backports.
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
Thanks for the review!
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 00/15] Further centralising of directory locking for name ops.
From: Christian Brauner @ 2026-03-06 10:40 UTC (permalink / raw)
To: NeilBrown
Cc: Alexander Viro, David Howells, Jan Kara, Chuck Lever, Jeff Layton,
Miklos Szeredi, Amir Goldstein, John Johansen, Paul Moore,
James Morris, Serge E. Hallyn, Stephen Smalley, Darrick J. Wong,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <177267387855.7472.13497219877141601891@noble.neil.brown.name>
On Thu, Mar 05, 2026 at 12:24:38PM +1100, NeilBrown wrote:
>
> Hi Christian,
> do you have thoughts about this series? Any idea when you might have
> time to review and (hopefully) apply them?
Sorry, for the delay I picked it up but have two minor comments.
^ permalink raw reply
* Re: [PATCH v3 10/15] cachefiles: change cachefiles_bury_object to use start_renaming_dentry()
From: Christian Brauner @ 2026-03-06 10:03 UTC (permalink / raw)
To: NeilBrown
Cc: Alexander Viro, David Howells, Jan Kara, Chuck Lever, Jeff Layton,
Miklos Szeredi, Amir Goldstein, John Johansen, Paul Moore,
James Morris, Serge E. Hallyn, Stephen Smalley, Darrick J. Wong,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260224222542.3458677-11-neilb@ownmail.net>
On Wed, Feb 25, 2026 at 09:16:55AM +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> Rather then using lock_rename() and lookup_one() etc we can use
> the new start_renaming_dentry(). This is part of centralising dir
> locking and lookup so that locking rules can be changed.
>
> Some error check are removed as not necessary. Checks for rep being a
> non-dir or IS_DEADDIR and the check that ->graveyard is still a
> directory only provide slightly more informative errors and have been
> dropped.
>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/cachefiles/namei.c | 76 ++++++++-----------------------------------
> 1 file changed, 14 insertions(+), 62 deletions(-)
>
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index e5ec90dccc27..3af42ec78411 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -270,7 +270,8 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> struct dentry *rep,
> enum fscache_why_object_killed why)
> {
> - struct dentry *grave, *trap;
> + struct dentry *grave;
> + struct renamedata rd = {};
> struct path path, path_to_graveyard;
> char nbuffer[8 + 8 + 1];
> int ret;
> @@ -302,77 +303,36 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> (uint32_t) ktime_get_real_seconds(),
> (uint32_t) atomic_inc_return(&cache->gravecounter));
>
> - /* do the multiway lock magic */
> - trap = lock_rename(cache->graveyard, dir);
> - if (IS_ERR(trap))
> - return PTR_ERR(trap);
> -
> - /* do some checks before getting the grave dentry */
> - if (rep->d_parent != dir || IS_DEADDIR(d_inode(rep))) {
> - /* the entry was probably culled when we dropped the parent dir
> - * lock */
> - unlock_rename(cache->graveyard, dir);
> - _leave(" = 0 [culled?]");
> - return 0;
I think this is a subtle change in behavior?
If rep->d_parent != dir after lock_rename this returned 0 in the old
code. With your changes the same condition in __start_renaming_dentry
returns -EINVAL which means cachefiles_io_error() sets CACHEFILES_DEAD
and permanently disables the cache.
> - }
> -
> - if (!d_can_lookup(cache->graveyard)) {
> - unlock_rename(cache->graveyard, dir);
> - cachefiles_io_error(cache, "Graveyard no longer a directory");
> - return -EIO;
> - }
> -
> - if (trap == rep) {
> - unlock_rename(cache->graveyard, dir);
> - cachefiles_io_error(cache, "May not make directory loop");
> + rd.mnt_idmap = &nop_mnt_idmap;
> + rd.old_parent = dir;
> + rd.new_parent = cache->graveyard;
> + rd.flags = 0;
> + ret = start_renaming_dentry(&rd, 0, rep, &QSTR(nbuffer));
> + if (ret) {
> + cachefiles_io_error(cache, "Cannot lock/lookup in graveyard");
> return -EIO;
> }
>
> if (d_mountpoint(rep)) {
> - unlock_rename(cache->graveyard, dir);
> + end_renaming(&rd);
> cachefiles_io_error(cache, "Mountpoint in cache");
> return -EIO;
> }
>
> - grave = lookup_one(&nop_mnt_idmap, &QSTR(nbuffer), cache->graveyard);
> - if (IS_ERR(grave)) {
> - unlock_rename(cache->graveyard, dir);
> - trace_cachefiles_vfs_error(object, d_inode(cache->graveyard),
> - PTR_ERR(grave),
> - cachefiles_trace_lookup_error);
> -
> - if (PTR_ERR(grave) == -ENOMEM) {
> - _leave(" = -ENOMEM");
> - return -ENOMEM;
> - }
This too?
In the old code a -ENOMEM return from lookup_one() let the cache stay
alive and returned directly. The new code gets sent via
cachefiles_io_error() causing again CACHEFILES_DEAD to be set and
permanently disabling the cache.
Maybe both changes are intentional. If so we should probably note this
in the commit message or this should be fixed?
If you give me a fixup! on top of vfs-7.1.directory I can fold it.
^ permalink raw reply
* Re: [PATCH v3 05/15] Apparmor: Use simple_start_creating() / simple_done_creating()
From: Christian Brauner @ 2026-03-06 9:42 UTC (permalink / raw)
To: NeilBrown
Cc: Alexander Viro, David Howells, Jan Kara, Chuck Lever, Jeff Layton,
Miklos Szeredi, Amir Goldstein, John Johansen, Paul Moore,
James Morris, Serge E. Hallyn, Stephen Smalley, Darrick J. Wong,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260224222542.3458677-6-neilb@ownmail.net>
On Wed, Feb 25, 2026 at 09:16:50AM +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> Instead of explicitly locking the parent and performing a look up in
> apparmor, use simple_start_creating(), and then simple_done_creating()
> to unlock and drop the dentry.
>
> This removes the need to check for an existing entry (as
> simple_start_creating() acts like an exclusive create and can return
> -EEXIST), simplifies error paths, and keeps dir locking code
> centralised.
>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
Fwiw, I think this fixes a reference count leak:
The old aafs_create returned dentries with refcount=2 (one from
lookup_noperm, one from dget in __aafs_setup_d_inode). The cleanup path
aafs_remove puts one reference leaving one reference that didn't get
cleaned up.
After your changes this is now correct as simple_done_creating() puts
the lookup reference.
^ permalink raw reply
* [PATCH v1] landlock/tsync: fix null-ptr-deref in cancel_tsync_works()
From: Jiayuan Chen @ 2026-03-06 9:22 UTC (permalink / raw)
To: linux-security-module
Cc: Jiayuan Chen, syzbot+911d99dc200feac03ea6,
Mickaël Salaün, Günther Noack, Paul Moore,
James Morris, Serge E. Hallyn, linux-kernel
cancel_tsync_works() iterates over works->works[0..size-1] and calls
task_work_cancel() on each entry. task_work_cancel() leads to
task_work_pending(), which dereferences task->task_works. If
works->works[i]->task is NULL, this causes a null-ptr-deref:
KASAN: null-ptr-deref in range [0x00000000000009a0-0x00000000000009a7]
RIP: 0010:task_work_pending include/linux/task_work.h:26 [inline]
RIP: 0010:task_work_cancel_match+0x86/0x250 kernel/task_work.c:124
RSP: 0018:ffffc90003597ba0 EFLAGS: 00010202
RAX: 0000000000000134 RBX: 0000000000000000 RCX: ffffc900106b1000
RDX: 0000000000080000 RSI: ffffffff81d13236 RDI: 0000000000000000
RBP: 1ffff920006b2f77 R08: 0000000000000007 R09: 0000000000000000
R10: 0000000000000002 R11: 0000000000000000 R12: ffffffff81d12dd0
R13: ffff88802c045100 R14: dffffc0000000000 R15: 00000000000009a0
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000110c3ea90c CR3: 0000000037f63000 CR4: 00000000003526f0
DR0: 0000000000000003 DR1: 00000000000001f8 DR2: 000000000000008e
DR3: 000000000000057a DR6: 00000000ffff0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
task_work_cancel+0x23/0x30 kernel/task_work.c:187
cancel_tsync_works security/landlock/tsync.c:415 [inline]
landlock_restrict_sibling_threads+0xafe/0x1280 security/landlock/tsync.c:533
__do_sys_landlock_restrict_self+0x5c9/0x9e0 security/landlock/syscalls.c:574
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x106/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f859b39c629
RSP: 002b:00007f85991b2028 EFLAGS: 00000246 ORIG_RAX: 00000000000001be
RAX: ffffffffffffffda RBX: 00007f859b616270 RCX: 00007f859b39c629
RDX: 0000000000000000 RSI: 000000000000000a RDI: 0000000000000003
RBP: 00007f859b432b39 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f859b616308 R14: 00007f859b616270 R15: 00007ffcff084488
The root cause is a race in schedule_task_work(). tsync_works_provide()
increments works->size and stores the task reference in ctx->task *before*
task_work_add() is called. A thread can race to call do_exit() in the
window between the PF_EXITING check and task_work_add(), causing
task_work_add() to return -ESRCH. The error path then drops the task
reference and sets ctx->task = NULL, but works->size remains incremented.
A subsequent call to cancel_tsync_works() iterates up to the stale size
and passes the NULL task pointer to task_work_cancel().
Fix this by decrementing works->size in the task_work_add() error path,
so the failed slot is rolled back and cancel_tsync_works() never iterates
over it. The slot is naturally reused in subsequent iterations since
tsync_works_provide() always picks works->works[works->size].
As a defensive measure, also add a WARN_ONCE() guard in cancel_tsync_works()
to catch any future NULL task pointer before dereferencing it.
Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
Reported-by: syzbot+911d99dc200feac03ea6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=911d99dc200feac03ea6
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
security/landlock/tsync.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index 0d2b9c646030..e6d742529484 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -381,14 +381,14 @@ static bool schedule_task_work(struct tsync_works *works,
err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
if (err) {
/*
- * task_work_add() only fails if the task is about to exit. We
- * checked that earlier, but it can happen as a race. Resume
- * without setting an error, as the task is probably gone in the
- * next loop iteration. For consistency, remove the task from ctx
- * so that it does not look like we handed it a task_work.
+ * task_work_add() only fails if the task is about to exit.
+ * We checked PF_EXITING earlier, but the thread can race to
+ * exit between that check and task_work_add(). Roll back the
+ * slot so cancel_tsync_works() never sees a NULL task pointer.
*/
put_task_struct(ctx->task);
ctx->task = NULL;
+ works->size--;
atomic_dec(&shared_ctx->num_preparing);
atomic_dec(&shared_ctx->num_unfinished);
@@ -412,6 +412,11 @@ static void cancel_tsync_works(struct tsync_works *works,
int i;
for (i = 0; i < works->size; i++) {
+ if (WARN_ONCE(!works->works[i]->task,
+ "landlock: unexpected NULL task in tsync slot %d\n",
+ i))
+ continue;
+
if (!task_work_cancel(works->works[i]->task,
&works->works[i]->work))
continue;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3 00/12] vfs: change inode->i_ino from unsigned long to u64
From: Christian Brauner @ 2026-03-06 9:09 UTC (permalink / raw)
To: Jeff Layton
Cc: Christian Brauner, linux-fsdevel, linux-kernel,
linux-trace-kernel, nvdimm, fsverity, linux-mm, netfs, linux-ext4,
linux-f2fs-devel, linux-nfs, linux-cifs, samba-technical,
linux-nilfs, v9fs, linux-afs, autofs, ceph-devel, codalist,
ecryptfs, linux-mtd, jfs-discussion, ntfs3, ocfs2-devel, devel,
linux-unionfs, apparmor, linux-security-module, linux-integrity,
selinux, amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf,
Alexander Viro, Jan Kara, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Dan Williams, Eric Biggers,
Theodore Y. Ts'o, Muchun Song, Oscar Salvador,
David Hildenbrand, David Howells, Paulo Alcantara, Andreas Dilger,
Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust, Anna Schumaker,
Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Steve French, Ronnie Sahlberg, Shyam Prasad N, Bharath SM,
Alexander Aring, Ryusuke Konishi, Viacheslav Dubeyko,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, David Sterba, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Ilya Dryomov, Alex Markuze, Jan Harkes, coda, Nicolas Pitre,
Tyler Hicks, Amir Goldstein, Christoph Hellwig,
John Paul Adrian Glaubitz, Yangtao Li, Mikulas Patocka,
David Woodhouse, Richard Weinberger, Dave Kleikamp,
Konstantin Komarov, Mark Fasheh, Joel Becker, Joseph Qi,
Mike Marshall, Martin Brandenburg, Miklos Szeredi, Anders Larsen,
Zhihao Cheng, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, Casey Schaufler, Alex Deucher,
Christian König, David Airlie, Simona Vetter, Sumit Semwal,
Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman, Oleg Nesterov,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Darrick J. Wong,
Martin Schiller, Eric Paris, Joerg Reuter, Marcel Holtmann,
Johan Hedberg, Luiz Augusto von Dentz, Oliver Hartkopp,
Marc Kleine-Budde, David Ahern, Neal Cardwell, Steffen Klassert,
Herbert Xu, Remi Denis-Courmont, Marcelo Ricardo Leitner,
Xin Long, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend
In-Reply-To: <20260304-iino-u64-v3-0-2257ad83d372@kernel.org>
On Wed, 04 Mar 2026 10:32:30 -0500, Jeff Layton wrote:
> Christian said [1] to "just do it" when I proposed this, so here we are!
>
> For historical reasons, the inode->i_ino field is an unsigned long,
> which means that it's 32 bits on 32 bit architectures. This has caused a
> number of filesystems to implement hacks to hash a 64-bit identifier
> into a 32-bit field, and deprives us of a universal identifier field for
> an inode.
>
> [...]
This series makes me happy. We've been talking about this conversion for
a while and I'm thankful that you did this work. Without the automation
available this probably wouldn't have happened as quickly as it did now.
Let's see what bits and pieces it missed.
---
Applied to the vfs-7.1.kino branch of the vfs/vfs.git tree.
Patches in the vfs-7.1.kino branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.1.kino
[01/12] vfs: widen inode hash/lookup functions to u64
https://git.kernel.org/vfs/vfs/c/2412a9fa518a
[02/12] audit: widen ino fields to u64
https://git.kernel.org/vfs/vfs/c/a5e863be4d02
[03/12] net: change sock.sk_ino and sock_i_ino() to u64
https://git.kernel.org/vfs/vfs/c/c21144a0a33f
[04/12] vfs: widen trace event i_ino fields to u64
https://git.kernel.org/vfs/vfs/c/5e5c380870b2
[05/12] cachefiles: widen trace event i_ino fields to u64
https://git.kernel.org/vfs/vfs/c/25291f67aad7
[06/12] ext2: widen trace event i_ino fields to u64
https://git.kernel.org/vfs/vfs/c/797d04a355e3
[07/12] hugetlbfs: widen trace event i_ino fields to u64
https://git.kernel.org/vfs/vfs/c/3c976fb36a9a
[08/12] zonefs: widen trace event i_ino fields to u64
https://git.kernel.org/vfs/vfs/c/988f68c01b3a
[09/12] ext4: widen trace event i_ino fields to u64
https://git.kernel.org/vfs/vfs/c/1c1427c79bc2
[10/12] f2fs: widen trace event i_ino fields to u64
https://git.kernel.org/vfs/vfs/c/6e62bf74bd8a
[11/12] nilfs2: widen trace event i_ino fields to u64
https://git.kernel.org/vfs/vfs/c/6ce73711525a
[12/12] treewide: change inode->i_ino from unsigned long to u64
https://git.kernel.org/vfs/vfs/c/af82d143e869
^ permalink raw reply
* Re: [PATCH] landlock: Clarify LANDLOCK_RULE_PATH_BENEATH properties in documentation
From: Günther Noack @ 2026-03-06 7:39 UTC (permalink / raw)
To: Justin Suess
Cc: linux-security-module, Mickaël Salaün,
Günther Noack
In-Reply-To: <20260305151507.2563776-1-utilityemal77@gmail.com>
On Thu, Mar 05, 2026 at 10:15:07AM -0500, Justin Suess wrote:
> Add paragraph to Landlock userspace documentation clarifying the strictly
> cumulative property of access rights with respect to the file hierarchy.
>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
> Documentation/userspace-api/landlock.rst | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 13134bccdd39..d02036bb2893 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -173,6 +173,17 @@ this file descriptor.
> return 1;
> }
>
> +The effective access rights for a path are the union of the access rights on
> +the path and all its parents. For instance, in this example, ``/usr/bin/grep``
> +inherits rights granted on ``/usr``, in addition to any rights we choose to
> +grant on ``/usr/bin`` and ``/usr/bin/grep``. Because
> +``LANDLOCK_RULE_PATH_BENEATH`` rights are cumulative, they can only increase
> +down the file hierarchy. Therefore, child paths cannot have fewer effective
> +access rights than their parents. This cumulative behavior is a key property of
> +``LANDLOCK_RULE_PATH_BENEATH`` and requires careful ruleset design to minimize
> +granted accesses. Please see the :ref:`Good practices` section for more
> +details.
> +
> It may also be required to create rules following the same logic as explained
> for the ruleset creation, by filtering access rights according to the Landlock
> ABI version. In this example, this is not required because all of the requested
>
> base-commit: f300a1c3a8ae4abca60913b4d26c405a905e4702
> prerequisite-patch-id: 2b17c4f0b741a703f61294989a53677de0b1a54d
> --
> 2.51.0
>
Thanks! I think this is a good addition to the docs in this place. 👍
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
–Günther
^ permalink raw reply
* Re: [PATCH v1 2/4] landlock: Add missing kernel-doc "Return:" sections
From: Günther Noack @ 2026-03-06 7:35 UTC (permalink / raw)
To: Mickaël Salaün; +Cc: Günther Noack, linux-security-module
In-Reply-To: <20260304193134.250495-2-mic@digikod.net>
On Wed, Mar 04, 2026 at 08:31:25PM +0100, Mickaël Salaün wrote:
> The kernel-doc -Wreturn check warns about functions with documentation
> comments that lack a "Return:" section. Add "Return:" documentation to
> all functions missing it so that kernel-doc -Wreturn passes cleanly.
>
> Convert existing function descriptions into a formal "Return:" section.
> Also fix the inaccurate return documentation for
> landlock_merge_ruleset() which claimed to return @parent directly, and
> document the previously missing ERR_PTR() error return path. Document
> the ABI version and errata return paths for landlock_create_ruleset()
> which were previously only implied by the prose.
>
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> security/landlock/domain.c | 2 ++
> security/landlock/fs.c | 2 +-
> security/landlock/ruleset.c | 8 +++++---
> security/landlock/syscalls.c | 17 +++++++++++------
> security/landlock/task.c | 9 +++++----
> 5 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index 79cb3bbdf4c5..343a1aabaac6 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -115,6 +115,8 @@ static struct landlock_details *get_current_details(void)
> * restriction. The subjective credentials must not be in an overridden state.
> *
> * @hierarchy->parent and @hierarchy->usage should already be set.
> + *
> + * Return: 0 on success, -errno on failure.
> */
> int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy)
> {
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index e764470f588c..cfe69075bf4e 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1568,7 +1568,7 @@ static int hook_path_truncate(const struct path *const path)
> *
> * @file: File being opened.
> *
> - * Returns the access rights that are required for opening the given file,
> + * Return: The access rights that are required for opening the given file,
> * depending on the file type and open mode.
> */
> static access_mask_t
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index a61ced492f41..de8386af2f30 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -202,6 +202,8 @@ static void build_check_ruleset(void)
> * When merging a ruleset in a domain, or copying a domain, @layers will be
> * added to @ruleset as new constraints, similarly to a boolean AND between
> * access rights.
> + *
> + * Return: 0 on success, -errno on failure.
> */
> static int insert_rule(struct landlock_ruleset *const ruleset,
> const struct landlock_id id,
> @@ -531,8 +533,8 @@ void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset)
> * The current task is requesting to be restricted. The subjective credentials
> * must not be in an overridden state. cf. landlock_init_hierarchy_log().
> *
> - * Returns the intersection of @parent and @ruleset, or returns @parent if
> - * @ruleset is empty, or returns a duplicate of @ruleset if @parent is empty.
> + * Return: A new domain merging @parent and @ruleset on success, or ERR_PTR()
> + * on failure. If @parent is NULL, the new domain duplicates @ruleset.
> */
> struct landlock_ruleset *
> landlock_merge_ruleset(struct landlock_ruleset *const parent,
> @@ -623,7 +625,7 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
> * @rule: A rule that grants a set of access rights for each layer
> * @masks: A matrix of unfulfilled access rights for each layer
> *
> - * Returns true if the request is allowed (i.e. the access rights granted all
> + * Return: True if the request is allowed (i.e. the access rights granted all
> * remaining unfulfilled access rights and masks has no leftover set bits).
> */
> bool landlock_unmask_layers(const struct landlock_rule *const rule,
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 0d66a68677b7..3b33839b80c7 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -60,6 +60,8 @@ static bool is_initialized(void)
> * @ksize_min: Minimal required size to be copied.
> * @src: User space pointer or NULL.
> * @usize: (Alleged) size of the data pointed to by @src.
> + *
> + * Return: 0 on success, -errno on failure.
> */
> static __always_inline int
> copy_min_struct_from_user(void *const dst, const size_t ksize,
> @@ -178,16 +180,19 @@ const int landlock_abi_version = 8;
> * - %LANDLOCK_CREATE_RULESET_VERSION
> * - %LANDLOCK_CREATE_RULESET_ERRATA
> *
> - * This system call enables to create a new Landlock ruleset, and returns the
> - * related file descriptor on success.
> + * This system call enables to create a new Landlock ruleset.
> *
> * If %LANDLOCK_CREATE_RULESET_VERSION or %LANDLOCK_CREATE_RULESET_ERRATA is
> * set, then @attr must be NULL and @size must be 0.
> *
> - * Possible returned errors are:
> + * Return: The ruleset file descriptor on success, the Landlock ABI version if
> + * %LANDLOCK_CREATE_RULESET_VERSION is set, the errata value if
> + * %LANDLOCK_CREATE_RULESET_ERRATA is set, or -errno on failure. Possible
> + * returned errors are:
> *
> * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> - * - %EINVAL: unknown @flags, or unknown access, or unknown scope, or too small @size;
> + * - %EINVAL: unknown @flags, or unknown access, or unknown scope, or too small
> + * @size;
> * - %E2BIG: @attr or @size inconsistencies;
> * - %EFAULT: @attr or @size inconsistencies;
> * - %ENOMSG: empty &landlock_ruleset_attr.handled_access_fs.
> @@ -398,7 +403,7 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
> * This system call enables to define a new rule and add it to an existing
> * ruleset.
> *
> - * Possible returned errors are:
> + * Return: 0 on success, or -errno on failure. Possible returned errors are:
> *
> * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> * - %EAFNOSUPPORT: @rule_type is %LANDLOCK_RULE_NET_PORT but TCP/IP is not
> @@ -464,7 +469,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
> * namespace or is running with no_new_privs. This avoids scenarios where
> * unprivileged tasks can affect the behavior of privileged children.
> *
> - * Possible returned errors are:
> + * Return: 0 on success, or -errno on failure. Possible returned errors are:
> *
> * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> * - %EINVAL: @flags contains an unknown bit.
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 833bc0cfe5c9..bf7c3db7ce46 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -37,6 +37,9 @@
> *
> * Checks if the @parent domain is less or equal to (i.e. an ancestor, which
> * means a subset of) the @child domain.
> + *
> + * Return: True if @parent is an ancestor of or equal to @child, false
> + * otherwise.
> */
> static bool domain_scope_le(const struct landlock_ruleset *const parent,
> const struct landlock_ruleset *const child)
> @@ -79,8 +82,7 @@ static int domain_ptrace(const struct landlock_ruleset *const parent,
> * If the current task has Landlock rules, then the child must have at least
> * the same rules. Else denied.
> *
> - * Determines whether a process may access another, returning 0 if permission
> - * granted, -errno if denied.
> + * Return: 0 if permission is granted, -errno if denied.
Good simplification! (the removed text is already mentioned in the
short summary next to the function name).
> */
> static int hook_ptrace_access_check(struct task_struct *const child,
> const unsigned int mode)
> @@ -129,8 +131,7 @@ static int hook_ptrace_access_check(struct task_struct *const child,
> * If the parent has Landlock rules, then the current task must have the same
> * or more rules. Else denied.
> *
> - * Determines whether the nominated task is permitted to trace the current
> - * process, returning 0 if permission is granted, -errno if denied.
> + * Return: 0 if permission is granted, -errno if denied.
Ditto.
> */
> static int hook_ptrace_traceme(struct task_struct *const parent)
> {
> --
> 2.53.0
>
Looks good, thanks!
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
^ permalink raw reply
* Re: [PATCH v1 3/4] landlock: Improve kernel-doc "Return:" section consistency
From: Günther Noack @ 2026-03-06 7:30 UTC (permalink / raw)
To: Mickaël Salaün; +Cc: Günther Noack, linux-security-module
In-Reply-To: <20260304193134.250495-3-mic@digikod.net>
Just a few comment phrasing nits below
On Wed, Mar 04, 2026 at 08:31:26PM +0100, Mickaël Salaün wrote:
> The canonical kernel-doc form is "Return:" (singular, without trailing
> "s"). Normalize all existing "Returns:" occurrences across the Landlock
> source tree to the canonical form.
>
> Also fix capitalization for consistency. Balance descriptions to
> describe all possible returned values.
>
> Consolidate bullet-point return descriptions into inline text for
> functions with simple two-value or three-value returns for consistency.
>
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> security/landlock/cred.h | 2 +-
> security/landlock/domain.c | 4 ++--
> security/landlock/fs.c | 26 +++++++++++---------------
> security/landlock/id.c | 2 +-
> security/landlock/ruleset.c | 2 +-
> security/landlock/ruleset.h | 2 +-
> security/landlock/task.c | 4 ++--
> security/landlock/tsync.c | 17 ++++++-----------
> 8 files changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/security/landlock/cred.h b/security/landlock/cred.h
> index c10a06727eb1..f287c56b5fd4 100644
> --- a/security/landlock/cred.h
> +++ b/security/landlock/cred.h
> @@ -115,7 +115,7 @@ static inline bool landlocked(const struct task_struct *const task)
> * @handle_layer: returned youngest layer handling a subset of @masks. Not set
> * if the function returns NULL.
> *
> - * Returns: landlock_cred(@cred) if any access rights specified in @masks is
> + * Return: landlock_cred(@cred) if any access rights specified in @masks is
> * handled, or NULL otherwise.
> */
> static inline const struct landlock_cred_security *
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index 343a1aabaac6..8b9939005aa8 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -34,7 +34,7 @@
> * @exe_size: Returned size of @exe_str (including the trailing null
> * character), if any.
> *
> - * Returns: A pointer to an allocated buffer where @exe_str point to, %NULL if
> + * Return: A pointer to an allocated buffer where @exe_str point to, %NULL if
> * there is no executable path, or an error otherwise.
> */
> static const void *get_current_exe(const char **const exe_str,
> @@ -73,7 +73,7 @@ static const void *get_current_exe(const char **const exe_str,
> }
>
> /*
> - * Returns: A newly allocated object describing a domain, or an error
> + * Return: A newly allocated object describing a domain, or an error
> * otherwise.
> */
> static struct landlock_details *get_current_details(void)
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index cfe69075bf4e..a03ec664c78e 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -119,8 +119,8 @@ static const struct landlock_object_underops landlock_fs_underops = {
> * Any new IOCTL commands that are implemented in fs/ioctl.c's do_vfs_ioctl()
> * should be considered for inclusion here.
> *
> - * Returns: true if the IOCTL @cmd can not be restricted with Landlock for
> - * device files.
> + * Return: True if the IOCTL @cmd can not be restricted with Landlock for
> + * device files, false otherwise.
> */
> static __attribute_const__ bool is_masked_device_ioctl(const unsigned int cmd)
> {
> @@ -428,10 +428,10 @@ static bool may_refer(const struct layer_access_masks *const src_parent,
> * Check that a destination file hierarchy has more restrictions than a source
> * file hierarchy. This is only used for link and rename actions.
> *
> - * Returns: true if child1 may be moved from parent1 to parent2 without
> - * increasing its access rights. If child2 is set, an additional condition is
> + * Return: True if child1 may be moved from parent1 to parent2 without
> + * increasing its access rights (if child2 is set, an additional condition is
> * that child2 may be used from parent2 to parent1 without increasing its access
> - * rights.
> + * rights), false otherwise.
> */
> static bool no_more_access(const struct layer_access_masks *const parent1,
> const struct layer_access_masks *const child1,
> @@ -734,9 +734,7 @@ static void test_is_eacces_with_write(struct kunit *const test)
> * checks that the collected accesses and the remaining ones are enough to
> * allow the request.
> *
> - * Returns:
> - * - true if the access request is granted;
> - * - false otherwise.
> + * Return: True if the access request is granted, false otherwise.
> */
> static bool
> is_access_to_paths_allowed(const struct landlock_ruleset *const domain,
> @@ -1022,9 +1020,8 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
> * only handles walking on the same mount point and only checks one set of
> * accesses.
> *
> - * Returns:
> - * - true if all the domain access rights are allowed for @dir;
> - * - false if the walk reached @mnt_root.
> + * Return: True if all the domain access rights are allowed for @dir, false if
> + * the walk reached @mnt_root.
> */
> static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
> const struct dentry *const mnt_root,
> @@ -1120,10 +1117,9 @@ static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
> * ephemeral matrices take some space on the stack, which limits the number of
> * layers to a deemed reasonable number: 16.
> *
> - * Returns:
> - * - 0 if access is allowed;
> - * - -EXDEV if @old_dentry would inherit new access rights from @new_dir;
> - * - -EACCES if file removal or creation is denied.
> + * Return: 0 if access is allowed, -EXDEV if @old_dentry would inherit new
> + * access rights from @new_dir, or -EACCES if file removal or creation is
> + * denied.
> */
> static int current_check_refer_path(struct dentry *const old_dentry,
> const struct path *const new_dir,
> diff --git a/security/landlock/id.c b/security/landlock/id.c
> index 838c3ed7bb82..6c8769777fdc 100644
> --- a/security/landlock/id.c
> +++ b/security/landlock/id.c
> @@ -258,7 +258,7 @@ static void test_range2_rand16(struct kunit *const test)
> *
> * @number_of_ids: Number of IDs to hold. Must be greater than one.
> *
> - * Returns: The first ID in the range.
> + * Return: The first ID in the range.
> */
> u64 landlock_get_id_range(size_t number_of_ids)
> {
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index de8386af2f30..52e48ffcc3aa 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -675,7 +675,7 @@ get_access_mask_t(const struct landlock_ruleset *const ruleset,
> * @masks: Layer access masks to populate.
> * @key_type: The key type to switch between access masks of different types.
> *
> - * Returns: An access mask where each access right bit is set which is handled
> + * Return: An access mask where each access right bit is set which is handled
> * in any of the active layers in @domain.
> */
> access_mask_t
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 87d52031fb5a..5e63f78f7e1a 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -232,7 +232,7 @@ static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
> *
> * @domain: Landlock ruleset (used as a domain)
> *
> - * Returns: an access_masks result of the OR of all the domain's access masks.
> + * Return: An access_masks result of the OR of all the domain's access masks.
> */
> static inline struct access_masks
> landlock_union_access_masks(const struct landlock_ruleset *const domain)
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index bf7c3db7ce46..f2dbdebf2770 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -174,8 +174,8 @@ static int hook_ptrace_traceme(struct task_struct *const parent)
> * @server: IPC receiver domain.
> * @scope: The scope restriction criteria.
> *
> - * Returns: True if @server is in a different domain from @client, and @client
> - * is scoped to access @server (i.e. access should be denied).
> + * Return: True if @server is in a different domain from @client and @client
> + * is scoped to access @server (i.e. access should be denied), false otherwise.
> */
> static bool domain_is_scoped(const struct landlock_ruleset *const client,
> const struct landlock_ruleset *const server,
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index b06a0fa4cedb..359aecbb1e4b 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -183,10 +183,8 @@ struct tsync_works {
> * capacity. This can legitimately happen if new threads get started after we
> * grew the capacity.
> *
> - * Returns:
> - * A pointer to the preallocated context struct, with task filled in.
> - *
> - * NULL, if we ran out of preallocated context structs.
> + * Return: A pointer to the preallocated context struct with task filled in, or
> + * NULL if preallocated context structs ran out.
> */
> static struct tsync_work *tsync_works_provide(struct tsync_works *s,
> struct task_struct *task)
> @@ -243,11 +241,8 @@ static void tsync_works_trim(struct tsync_works *s)
> * On a successful return, the subsequent n calls to tsync_works_provide() are
> * guaranteed to succeed. (size + n <= capacity)
> *
> - * Returns:
> - * -ENOMEM if the (re)allocation fails
> -
> - * 0 if the allocation succeeds, partially succeeds, or no reallocation
> - * was needed
> + * Return: 0 on success or partial success, -ENOMEM if the (re)allocation
> + * fails.
tsync_works_grow_by:
I don't know what I meant when I wrote "partially succeeds" here in
the original patch. Would suggest this phrasing:
Return: 0 if sufficient space for n more elements could be provided,
-ENOMEM on allocation errors, -EOVERFLOW in case of integer
overflow.
With this function, the success criterium is that it can establish
that invariant. We also don't return a success if we only could
allocate space for fewer elements.
> */
> static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags)
> {
> @@ -363,8 +358,8 @@ static size_t count_additional_threads(const struct tsync_works *works)
> * For each added task_work, atomically increments shared_ctx->num_preparing and
> * shared_ctx->num_unfinished.
> *
> - * Returns:
> - * true, if at least one eligible sibling thread was found
> + * Return: True if at least one eligible sibling thread was found, false
> + * otherwise.
> */
> static bool schedule_task_work(struct tsync_works *works,
> struct tsync_shared_context *shared_ctx)
> --
> 2.53.0
>
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
^ permalink raw reply
* Re: [PATCH v1 4/4] landlock: Fix formatting in tsync.c
From: Günther Noack @ 2026-03-06 7:13 UTC (permalink / raw)
To: Mickaël Salaün; +Cc: Günther Noack, linux-security-module
In-Reply-To: <20260304193134.250495-4-mic@digikod.net>
On Wed, Mar 04, 2026 at 08:31:27PM +0100, Mickaël Salaün wrote:
> Fix comment formatting in tsync.c to fit in 80 columns.
>
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>
> My previous squashed fix was wrong.
> ---
> security/landlock/tsync.c | 121 +++++++++++++++++++++-----------------
> 1 file changed, 66 insertions(+), 55 deletions(-)
>
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index 359aecbb1e4b..50445ae167dd 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -85,12 +85,14 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
> /*
> * Switch out old_cred with new_cred, if possible.
> *
> - * In the common case, where all threads initially point to the same
> - * struct cred, this optimization avoids creating separate redundant
> - * credentials objects for each, which would all have the same contents.
> + * In the common case, where all threads initially point to the
> + * same struct cred, this optimization avoids creating separate
> + * redundant credentials objects for each, which would all have
> + * the same contents.
> *
> - * Note: We are intentionally dropping the const qualifier here, because
> - * it is required by commit_creds() and abort_creds().
> + * Note: We are intentionally dropping the const qualifier
> + * here, because it is required by commit_creds() and
> + * abort_creds().
> */
> cred = (struct cred *)get_cred(ctx->new_cred);
> } else {
> @@ -101,8 +103,8 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
> atomic_set(&ctx->preparation_error, -ENOMEM);
>
> /*
> - * Even on error, we need to adhere to the protocol and coordinate
> - * with concurrently running invocations.
> + * Even on error, we need to adhere to the protocol and
> + * coordinate with concurrently running invocations.
> */
> if (atomic_dec_return(&ctx->num_preparing) == 0)
> complete_all(&ctx->all_prepared);
> @@ -135,9 +137,9 @@ static void restrict_one_thread(struct tsync_shared_context *ctx)
> }
>
> /*
> - * Make sure that all sibling tasks fulfill the no_new_privs prerequisite.
> - * (This is in line with Seccomp's SECCOMP_FILTER_FLAG_TSYNC logic in
> - * kernel/seccomp.c)
> + * Make sure that all sibling tasks fulfill the no_new_privs
> + * prerequisite. (This is in line with Seccomp's
> + * SECCOMP_FILTER_FLAG_TSYNC logic in kernel/seccomp.c)
> */
> if (ctx->set_no_new_privs)
> task_set_no_new_privs(current);
> @@ -221,16 +223,17 @@ static void tsync_works_trim(struct tsync_works *s)
> ctx = s->works[s->size - 1];
>
> /*
> - * For consistency, remove the task from ctx so that it does not look like
> - * we handed it a task_work.
> + * For consistency, remove the task from ctx so that it does not look
> + * like we handed it a task_work.
> */
> put_task_struct(ctx->task);
> *ctx = (typeof(*ctx)){};
>
> /*
> - * Cancel the tsync_works_provide() change to recycle the reserved memory
> - * for the next thread, if any. This also ensures that cancel_tsync_works()
> - * and tsync_works_release() do not see any NULL task pointers.
> + * Cancel the tsync_works_provide() change to recycle the reserved
> + * memory for the next thread, if any. This also ensures that
> + * cancel_tsync_works() and tsync_works_release() do not see any NULL
> + * task pointers.
> */
> s->size--;
> }
> @@ -388,17 +391,17 @@ static bool schedule_task_work(struct tsync_works *works,
> continue;
>
> /*
> - * We found a sibling thread that is not doing its task_work yet, and
> - * which might spawn new threads before our task work runs, so we need
> - * at least one more round in the outer loop.
> + * We found a sibling thread that is not doing its task_work
> + * yet, and which might spawn new threads before our task work
> + * runs, so we need at least one more round in the outer loop.
> */
> found_more_threads = true;
>
> ctx = tsync_works_provide(works, thread);
> if (!ctx) {
> /*
> - * We ran out of preallocated contexts -- we need to try again with
> - * this thread at a later time!
> + * We ran out of preallocated contexts -- we need to
> + * try again with this thread at a later time!
> * found_more_threads is already true at this point.
> */
> break;
> @@ -413,10 +416,10 @@ static bool schedule_task_work(struct tsync_works *works,
> err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
> if (unlikely(err)) {
> /*
> - * task_work_add() only fails if the task is about to exit. We
> - * checked that earlier, but it can happen as a race. Resume
> - * without setting an error, as the task is probably gone in the
> - * next loop iteration.
> + * task_work_add() only fails if the task is about to
> + * exit. We checked that earlier, but it can happen as
> + * a race. Resume without setting an error, as the
> + * task is probably gone in the next loop iteration.
> */
> tsync_works_trim(works);
>
> @@ -497,24 +500,25 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> * After this barrier is reached, it's safe to read
> * shared_ctx.preparation_error.
> *
> - * 4) reads shared_ctx.preparation_error and then either does commit_creds()
> - * or abort_creds().
> + * 4) reads shared_ctx.preparation_error and then either does
> + * commit_creds() or abort_creds().
> *
> * 5) signals that it's done altogether (barrier synchronization
> * "all_finished")
> *
> - * Unlike seccomp, which modifies sibling tasks directly, we do not need to
> - * acquire the cred_guard_mutex and sighand->siglock:
> + * Unlike seccomp, which modifies sibling tasks directly, we do not
> + * need to acquire the cred_guard_mutex and sighand->siglock:
> *
> - * - As in our case, all threads are themselves exchanging their own struct
> - * cred through the credentials API, no locks are needed for that.
> + * - As in our case, all threads are themselves exchanging their own
> + * struct cred through the credentials API, no locks are needed for
> + * that.
> * - Our for_each_thread() loops are protected by RCU.
> - * - We do not acquire a lock to keep the list of sibling threads stable
> - * between our for_each_thread loops. If the list of available sibling
> - * threads changes between these for_each_thread loops, we make up for
> - * that by continuing to look for threads until they are all discovered
> - * and have entered their task_work, where they are unable to spawn new
> - * threads.
> + * - We do not acquire a lock to keep the list of sibling threads
> + * stable between our for_each_thread loops. If the list of
> + * available sibling threads changes between these for_each_thread
> + * loops, we make up for that by continuing to look for threads until
> + * they are all discovered and have entered their task_work, where
> + * they are unable to spawn new threads.
> */
> do {
> /* In RCU read-lock, count the threads we need. */
> @@ -531,43 +535,50 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> }
>
> /*
> - * The "all_prepared" barrier is used locally to the loop body, this use
> - * of for_each_thread(). We can reset it on each loop iteration because
> - * all previous loop iterations are done with it already.
> + * The "all_prepared" barrier is used locally to the loop body,
> + * this use of for_each_thread(). We can reset it on each loop
> + * iteration because all previous loop iterations are done with
> + * it already.
> *
> - * num_preparing is initialized to 1 so that the counter can not go to 0
> - * and mark the completion as done before all task works are registered.
> - * We decrement it at the end of the loop body.
> + * num_preparing is initialized to 1 so that the counter can
> + * not go to 0 and mark the completion as done before all task
> + * works are registered. We decrement it at the end of the
> + * loop body.
> */
> atomic_set(&shared_ctx.num_preparing, 1);
> reinit_completion(&shared_ctx.all_prepared);
>
> /*
> - * In RCU read-lock, schedule task work on newly discovered sibling
> - * tasks.
> + * In RCU read-lock, schedule task work on newly discovered
> + * sibling tasks.
> */
> found_more_threads = schedule_task_work(&works, &shared_ctx);
>
> /*
> - * Decrement num_preparing for current, to undo that we initialized it
> - * to 1 a few lines above.
> + * Decrement num_preparing for current, to undo that we
> + * initialized it to 1 a few lines above.
> */
> if (atomic_dec_return(&shared_ctx.num_preparing) > 0) {
> if (wait_for_completion_interruptible(
> &shared_ctx.all_prepared)) {
> - /* In case of interruption, we need to retry the system call. */
> + /*
> + * In case of interruption, we need to retry
> + * the system call.
> + */
> atomic_set(&shared_ctx.preparation_error,
> -ERESTARTNOINTR);
>
> /*
> - * Cancel task works for tasks that did not start running yet,
> - * and decrement all_prepared and num_unfinished accordingly.
> + * Cancel task works for tasks that did not
> + * start running yet, and decrement
> + * all_prepared and num_unfinished accordingly.
> */
> cancel_tsync_works(&works, &shared_ctx);
>
> /*
> - * The remaining task works have started running, so waiting for
> - * their completion will finish.
> + * The remaining task works have started
> + * running, so waiting for their completion
> + * will finish.
> */
> wait_for_completion(&shared_ctx.all_prepared);
> }
> @@ -576,14 +587,14 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> !atomic_read(&shared_ctx.preparation_error));
>
> /*
> - * We now have all sibling threads blocking and in "prepared" state in the
> - * task work. Ask all threads to commit.
> + * We now have all sibling threads blocking and in "prepared" state in
> + * the task work. Ask all threads to commit.
> */
> complete_all(&shared_ctx.ready_to_commit);
>
> /*
> - * Decrement num_unfinished for current, to undo that we initialized it to 1
> - * at the beginning.
> + * Decrement num_unfinished for current, to undo that we initialized it
> + * to 1 at the beginning.
> */
> if (atomic_dec_return(&shared_ctx.num_unfinished) > 0)
> wait_for_completion(&shared_ctx.all_finished);
> --
> 2.53.0
>
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
Thanks! (It's irritating that the default clang-format configuration
does not fix these. Do you use a special tool for this?)
–Günther
^ permalink raw reply
* Re: [PATCH v5 2/2] landlock: Clean up interrupted thread logic in TSYNC
From: Günther Noack @ 2026-03-06 7:00 UTC (permalink / raw)
To: Yihan Ding
Cc: jannh, linux-kernel, linux-security-module, m, mic, paul,
syzbot+7ea2f5e9dfd468201817, utilityemal77
In-Reply-To: <20260306021651.744723-3-dingyihan@uniontech.com>
On Fri, Mar 06, 2026 at 10:16:51AM +0800, Yihan Ding wrote:
> In landlock_restrict_sibling_threads(), when the calling thread is
> interrupted while waiting for sibling threads to prepare, it executes
> a recovery path.
>
> Previously, this path included a wait_for_completion() call on
> all_prepared to prevent a Use-After-Free of the local shared_ctx.
> However, this wait is redundant. Exiting the main do-while loop
> already leads to a bottom cleanup section that unconditionally waits
> for all_finished. Therefore, replacing the wait with a simple break
> is safe, prevents UAF, and correctly unblocks the remaining task_works.
>
> Clean up the error path by breaking the loop and updating the
> surrounding comments to accurately reflect the state machine.
>
> Suggested-by: Günther Noack <gnoack3000@gmail.com>
> Signed-off-by: Yihan Ding <dingyihan@uniontech.com>
> ---
> Changes in v3, v4, v5:
> - No changes.
>
> Changes in v2:
> - Replaced wait_for_completion(&shared_ctx.all_prepared) with a break
> statement based on the realization that the bottom wait for 'all_finished'
> already guards against UAF.
> - Updated comments for clarity.
> ---
> security/landlock/tsync.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index 1f460b9ec833..d52583ee1d93 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -536,24 +536,27 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> -ERESTARTNOINTR);
>
> /*
> - * Cancel task works for tasks that did not start running yet,
> - * and decrement all_prepared and num_unfinished accordingly.
> + * Opportunistic improvement: try to cancel task works for
> + * tasks that did not start running yet. We do not have a
> + * guarantee that it cancels any of the enqueued task works
> + * because task_work_run() might already have dequeued them.
> */
> cancel_tsync_works(&works, &shared_ctx);
>
> /*
> - * The remaining task works have started running, so waiting for
> - * their completion will finish.
> + * Break the loop with error. The cleanup code after the loop
> + * unblocks the remaining task_works.
> */
> - wait_for_completion(&shared_ctx.all_prepared);
> + break;
> }
> }
> } while (found_more_threads &&
> !atomic_read(&shared_ctx.preparation_error));
>
> /*
> - * We now have all sibling threads blocking and in "prepared" state in the
> - * task work. Ask all threads to commit.
> + * We now have either (a) all sibling threads blocking and in "prepared"
> + * state in the task work, or (b) the preparation error is set. Ask all
> + * threads to commit (or abort).
> */
> complete_all(&shared_ctx.ready_to_commit);
>
> --
> 2.20.1
>
Thank you!
Tested-by: Günther Noack <gnoack3000@gmail.com>
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
^ permalink raw reply
* Re: [PATCH v5 1/2] landlock: Serialize TSYNC thread restriction
From: Günther Noack @ 2026-03-06 7:00 UTC (permalink / raw)
To: Yihan Ding
Cc: jannh, linux-kernel, linux-security-module, m, mic, paul,
syzbot+7ea2f5e9dfd468201817, utilityemal77
In-Reply-To: <20260306021651.744723-2-dingyihan@uniontech.com>
On Fri, Mar 06, 2026 at 10:16:50AM +0800, Yihan Ding wrote:
> syzbot found a deadlock in landlock_restrict_sibling_threads().
> When multiple threads concurrently call landlock_restrict_self() with
> sibling thread restriction enabled, they can deadlock by mutually
> queueing task_works on each other and then blocking in kernel space
> (waiting for the other to finish).
>
> Fix this by serializing the TSYNC operations within the same process
> using the exec_update_lock. This prevents concurrent invocations
> from deadlocking.
>
> We use down_write_trylock() and restart the syscall if the lock
> cannot be acquired immediately. This ensures that if a thread fails
> to get the lock, it will return to userspace, allowing it to process
> any pending TSYNC task_works from the lock holder, and then
> transparently restart the syscall.
>
> Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
> Reported-by: syzbot+7ea2f5e9dfd468201817@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7ea2f5e9dfd468201817
> Reported-by: Justin Suess <utilityemal77@gmail.com>
> Closes: https://lore.kernel.org/all/aacKOr1wywSSOAVv@suesslenovo/
> Suggested-by: Günther Noack <gnoack3000@gmail.com>
> Suggested-by: Tingmao Wang <m@maowtm.org>
> Tested-by: Justin Suess <utilityemal77@gmail.com>
> Signed-off-by: Yihan Ding <dingyihan@uniontech.com>
> ---
> Changes in v5:
> - Just simple formatting changes, no code changes.
>
> Changes in v4:
> - Use restart_syscall() instead of returning -ERESTARTNOINTR.
> This ensures the syscall is properly restarted without leaking the
> internal error code to userspace, fixing a test failure in
> tsync_test.competing_enablement. (Caught by Justin Suess, suggested
> by Tingmao Wang).
>
> Changes in v3:
> - Replaced down_write_killable() with down_write_trylock() and
> returned -ERESTARTNOINTR to avoid a secondary deadlock caused by
> blocking the execution of task_works. (Caught by Günther Noack).
>
> ---
> security/landlock/tsync.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index de01aa899751..1f460b9ec833 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -446,6 +446,15 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> shared_ctx.old_cred = old_cred;
> shared_ctx.new_cred = new_cred;
> shared_ctx.set_no_new_privs = task_no_new_privs(current);
> + /*
> + * Serialize concurrent TSYNC operations to prevent deadlocks when
> + * multiple threads call landlock_restrict_self() simultaneously.
> + * If the lock is already held, we gracefully yield by restarting the
> + * syscall. This allows the current thread to process pending
> + * task_works before retrying.
> + */
> + if (!down_write_trylock(¤t->signal->exec_update_lock))
> + return restart_syscall();
>
> /*
> * We schedule a pseudo-signal task_work for each of the calling task's
> @@ -556,6 +565,6 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
> wait_for_completion(&shared_ctx.all_finished);
>
> tsync_works_release(&works);
> -
> + up_write(¤t->signal->exec_update_lock);
> return atomic_read(&shared_ctx.preparation_error);
> }
> --
> 2.20.1
>
Thank you!
Tested-by: Günther Noack <gnoack3000@gmail.com>
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
^ permalink raw reply
* Re: [PATCH v3 02/12] audit: widen ino fields to u64
From: Paul Moore @ 2026-03-06 3:09 UTC (permalink / raw)
To: Jeff Layton
Cc: Alexander Viro, Christian Brauner, Jan Kara, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Dan Williams, Eric Biggers,
Theodore Y. Ts'o, Muchun Song, Oscar Salvador,
David Hildenbrand, David Howells, Paulo Alcantara, Andreas Dilger,
Jan Kara, Jaegeuk Kim, Chao Yu, Trond Myklebust, Anna Schumaker,
Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Steve French, Ronnie Sahlberg, Shyam Prasad N, Bharath SM,
Alexander Aring, Ryusuke Konishi, Viacheslav Dubeyko,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, David Sterba, Marc Dionne, Ian Kent,
Luis de Bethencourt, Salah Triki, Tigran A. Aivazian,
Ilya Dryomov, Alex Markuze, Jan Harkes, coda, Nicolas Pitre,
Tyler Hicks, Amir Goldstein, Christoph Hellwig,
John Paul Adrian Glaubitz, Yangtao Li, Mikulas Patocka,
David Woodhouse, Richard Weinberger, Dave Kleikamp,
Konstantin Komarov, Mark Fasheh, Joel Becker, Joseph Qi,
Mike Marshall, Martin Brandenburg, Miklos Szeredi, Anders Larsen,
Zhihao Cheng, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
John Johansen, James Morris, Serge E. Hallyn, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, Casey Schaufler, Alex Deucher,
Christian König, David Airlie, Simona Vetter, Sumit Semwal,
Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman, Oleg Nesterov,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Darrick J. Wong,
Martin Schiller, Eric Paris, Joerg Reuter, Marcel Holtmann,
Johan Hedberg, Luiz Augusto von Dentz, Oliver Hartkopp,
Marc Kleine-Budde, David Ahern, Neal Cardwell, Steffen Klassert,
Herbert Xu, Remi Denis-Courmont, Marcelo Ricardo Leitner,
Xin Long, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, linux-fsdevel, linux-kernel, linux-trace-kernel,
nvdimm, fsverity, linux-mm, netfs, linux-ext4, linux-f2fs-devel,
linux-nfs, linux-cifs, samba-technical, linux-nilfs, v9fs,
linux-afs, autofs, ceph-devel, codalist, ecryptfs, linux-mtd,
jfs-discussion, ntfs3, ocfs2-devel, devel, linux-unionfs,
apparmor, linux-security-module, linux-integrity, selinux,
amd-gfx, dri-devel, linux-media, linaro-mm-sig, netdev,
linux-perf-users, linux-fscrypt, linux-xfs, linux-hams, linux-x25,
audit, linux-bluetooth, linux-can, linux-sctp, bpf
In-Reply-To: <20260304-iino-u64-v3-2-2257ad83d372@kernel.org>
On Wed, Mar 4, 2026 at 10:33 AM Jeff Layton <jlayton@kernel.org> wrote:
>
> inode->i_ino is being widened from unsigned long to u64. The audit
> subsystem uses unsigned long ino in struct fields, function parameters,
> and local variables that store inode numbers from arbitrary filesystems.
> On 32-bit platforms this truncates inode numbers that exceed 32 bits,
> which will cause incorrect audit log entries and broken watch/mark
> comparisons.
>
> Widen all audit ino fields, parameters, and locals to u64, and update
> the inode format string from %lu to %llu to match.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> include/linux/audit.h | 2 +-
> kernel/audit.h | 13 ++++++-------
> kernel/audit_fsnotify.c | 4 ++--
> kernel/audit_watch.c | 12 ++++++------
> kernel/auditsc.c | 4 ++--
> 5 files changed, 17 insertions(+), 18 deletions(-)
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* [PATCH v5 1/2] landlock: Serialize TSYNC thread restriction
From: Yihan Ding @ 2026-03-06 2:16 UTC (permalink / raw)
To: gnoack3000
Cc: dingyihan, jannh, linux-kernel, linux-security-module, m, mic,
paul, syzbot+7ea2f5e9dfd468201817, utilityemal77
In-Reply-To: <20260306021651.744723-1-dingyihan@uniontech.com>
syzbot found a deadlock in landlock_restrict_sibling_threads().
When multiple threads concurrently call landlock_restrict_self() with
sibling thread restriction enabled, they can deadlock by mutually
queueing task_works on each other and then blocking in kernel space
(waiting for the other to finish).
Fix this by serializing the TSYNC operations within the same process
using the exec_update_lock. This prevents concurrent invocations
from deadlocking.
We use down_write_trylock() and restart the syscall if the lock
cannot be acquired immediately. This ensures that if a thread fails
to get the lock, it will return to userspace, allowing it to process
any pending TSYNC task_works from the lock holder, and then
transparently restart the syscall.
Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
Reported-by: syzbot+7ea2f5e9dfd468201817@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7ea2f5e9dfd468201817
Reported-by: Justin Suess <utilityemal77@gmail.com>
Closes: https://lore.kernel.org/all/aacKOr1wywSSOAVv@suesslenovo/
Suggested-by: Günther Noack <gnoack3000@gmail.com>
Suggested-by: Tingmao Wang <m@maowtm.org>
Tested-by: Justin Suess <utilityemal77@gmail.com>
Signed-off-by: Yihan Ding <dingyihan@uniontech.com>
---
Changes in v5:
- Just simple formatting changes, no code changes.
Changes in v4:
- Use restart_syscall() instead of returning -ERESTARTNOINTR.
This ensures the syscall is properly restarted without leaking the
internal error code to userspace, fixing a test failure in
tsync_test.competing_enablement. (Caught by Justin Suess, suggested
by Tingmao Wang).
Changes in v3:
- Replaced down_write_killable() with down_write_trylock() and
returned -ERESTARTNOINTR to avoid a secondary deadlock caused by
blocking the execution of task_works. (Caught by Günther Noack).
---
security/landlock/tsync.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index de01aa899751..1f460b9ec833 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -446,6 +446,15 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
shared_ctx.old_cred = old_cred;
shared_ctx.new_cred = new_cred;
shared_ctx.set_no_new_privs = task_no_new_privs(current);
+ /*
+ * Serialize concurrent TSYNC operations to prevent deadlocks when
+ * multiple threads call landlock_restrict_self() simultaneously.
+ * If the lock is already held, we gracefully yield by restarting the
+ * syscall. This allows the current thread to process pending
+ * task_works before retrying.
+ */
+ if (!down_write_trylock(¤t->signal->exec_update_lock))
+ return restart_syscall();
/*
* We schedule a pseudo-signal task_work for each of the calling task's
@@ -556,6 +565,6 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
wait_for_completion(&shared_ctx.all_finished);
tsync_works_release(&works);
-
+ up_write(¤t->signal->exec_update_lock);
return atomic_read(&shared_ctx.preparation_error);
}
--
2.20.1
^ permalink raw reply related
* [PATCH v5 0/2] landlock: Fix TSYNC deadlock and clean up error path
From: Yihan Ding @ 2026-03-06 2:16 UTC (permalink / raw)
To: gnoack3000
Cc: dingyihan, jannh, linux-kernel, linux-security-module, m, mic,
paul, syzbot+7ea2f5e9dfd468201817, utilityemal77
Hello,
This patch series fixes a deadlock in the Landlock TSYNC multithreading
support, originally reported by syzbot, and cleans up the associated
interrupt recovery path.
The deadlock occurs when multiple threads concurrently call
landlock_restrict_self() with sibling thread restriction enabled,
causing them to mutually queue task_works on each other and block
indefinitely.
* Patch 1 fixes the root cause by serializing the TSYNC operations
within the same process using the exec_update_lock.
* Patch 2 cleans up the interrupt recovery path by replacing an
unnecessary wait_for_completion() with a straightforward loop break,
avoiding Use-After-Free while unblocking remaining task_works.
---
Changes in v5:
- Just simple formatting changes, no code changes.
Changes in v4:
- Patch 1: Use restart_syscall() instead of returning -ERESTARTNOINTR.
This ensures the syscall is properly restarted without leaking the
internal error code to userspace, fixing a test failure in
tsync_test.competing_enablement. (Caught by Justin Suess, suggested
by Tingmao Wang).
- Patch 1 and 2: Wrap comments to fit in 80 columns
Changes in v3:
- Patch 1: Changed down_write_killable() to down_write_trylock() and
return -ERESTARTNOINTR on failure. This avoids a secondary deadlock
where a blocking wait prevents a sibling thread from waking up to
execute the requested TSYNC task_work. (Noted by Günther Noack.
down_write_interruptible() was also suggested but is not implemented
for rw_semaphores in the kernel).
- Patch 2: No changes.
Changes in v2:
- Split the changes into a 2-patch series.
- Patch 1: Adopted down_write_killable() instead of down_write().
- Patch 2: Removed wait_for_completion(&shared_ctx.all_prepared) and
replaced it with a `break` to prevent UAF.
Link to v4: https://lore.kernel.org/all/20260304095418.465594-1-dingyihan@uniontech.com/
Link to v3: https://lore.kernel.org/all/20260226015903.3158620-1-dingyihan@uniontech.com/
Link to v2: https://lore.kernel.org/all/20260225024734.3024732-1-dingyihan@uniontech.com/
Link to v1: https://lore.kernel.org/all/20260224062729.2908692-1-dingyihan@uniontech.com/
Yihan Ding (2):
landlock: Serialize TSYNC thread restriction
landlock: Clean up interrupted thread logic in TSYNC
security/landlock/tsync.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH v5 2/2] landlock: Clean up interrupted thread logic in TSYNC
From: Yihan Ding @ 2026-03-06 2:16 UTC (permalink / raw)
To: gnoack3000
Cc: dingyihan, jannh, linux-kernel, linux-security-module, m, mic,
paul, syzbot+7ea2f5e9dfd468201817, utilityemal77
In-Reply-To: <20260306021651.744723-1-dingyihan@uniontech.com>
In landlock_restrict_sibling_threads(), when the calling thread is
interrupted while waiting for sibling threads to prepare, it executes
a recovery path.
Previously, this path included a wait_for_completion() call on
all_prepared to prevent a Use-After-Free of the local shared_ctx.
However, this wait is redundant. Exiting the main do-while loop
already leads to a bottom cleanup section that unconditionally waits
for all_finished. Therefore, replacing the wait with a simple break
is safe, prevents UAF, and correctly unblocks the remaining task_works.
Clean up the error path by breaking the loop and updating the
surrounding comments to accurately reflect the state machine.
Suggested-by: Günther Noack <gnoack3000@gmail.com>
Signed-off-by: Yihan Ding <dingyihan@uniontech.com>
---
Changes in v3, v4, v5:
- No changes.
Changes in v2:
- Replaced wait_for_completion(&shared_ctx.all_prepared) with a break
statement based on the realization that the bottom wait for 'all_finished'
already guards against UAF.
- Updated comments for clarity.
---
security/landlock/tsync.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index 1f460b9ec833..d52583ee1d93 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -536,24 +536,27 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
-ERESTARTNOINTR);
/*
- * Cancel task works for tasks that did not start running yet,
- * and decrement all_prepared and num_unfinished accordingly.
+ * Opportunistic improvement: try to cancel task works for
+ * tasks that did not start running yet. We do not have a
+ * guarantee that it cancels any of the enqueued task works
+ * because task_work_run() might already have dequeued them.
*/
cancel_tsync_works(&works, &shared_ctx);
/*
- * The remaining task works have started running, so waiting for
- * their completion will finish.
+ * Break the loop with error. The cleanup code after the loop
+ * unblocks the remaining task_works.
*/
- wait_for_completion(&shared_ctx.all_prepared);
+ break;
}
}
} while (found_more_threads &&
!atomic_read(&shared_ctx.preparation_error));
/*
- * We now have all sibling threads blocking and in "prepared" state in the
- * task work. Ask all threads to commit.
+ * We now have either (a) all sibling threads blocking and in "prepared"
+ * state in the task work, or (b) the preparation error is set. Ask all
+ * threads to commit (or abort).
*/
complete_all(&shared_ctx.ready_to_commit);
--
2.20.1
^ permalink raw reply related
* [PATCH v2 net-next 06/15] smack: Remove IPPROTO_UDPLITE support in security_sock_rcv_skb().
From: Kuniyuki Iwashima @ 2026-03-05 21:49 UTC (permalink / raw)
To: Willem de Bruijn, David Ahern, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Florian Westphal, Kuniyuki Iwashima,
Kuniyuki Iwashima, netdev, Willem de Bruijn, Casey Schaufler,
Paul Moore, James Morris, Serge E. Hallyn, linux-security-module
In-Reply-To: <20260305215013.2984628-1-kuniyu@google.com>
smack_socket_sock_rcv_skb() is registered as socket_sock_rcv_skb,
which is called as security_sock_rcv_skb() in sk_filter_trim_cap().
Now that UDP-Lite is gone, let's remove the IPPROTO_UDPLITE support
in smack_socket_sock_rcv_skb().
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
---
Cc: Paul Moore <paul@paul-moore.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: linux-security-module@vger.kernel.org
---
security/smack/smack_lsm.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 98af9d7b9434..e581d6465946 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4176,7 +4176,6 @@ static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)
sip->sin6_port = th->source;
break;
case IPPROTO_UDP:
- case IPPROTO_UDPLITE:
uh = skb_header_pointer(skb, offset, sizeof(_udph), &_udph);
if (uh != NULL)
sip->sin6_port = uh->source;
@@ -4301,8 +4300,7 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
#if IS_ENABLED(CONFIG_IPV6)
case PF_INET6:
proto = smk_skb_to_addr_ipv6(skb, &sadd);
- if (proto != IPPROTO_UDP && proto != IPPROTO_UDPLITE &&
- proto != IPPROTO_TCP)
+ if (proto != IPPROTO_UDP && proto != IPPROTO_TCP)
break;
#ifdef SMACK_IPV6_SECMARK_LABELING
skp = smack_from_skb(skb);
--
2.53.0.473.g4a7958ca14-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