* 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 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 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 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 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
* [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: 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
* 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: [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: [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] 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] integrity: avoid using __weak functions
From: Arnd Bergmann @ 2026-03-06 23:28 UTC (permalink / raw)
To: Nathan Chancellor, Arnd Bergmann
Cc: Madhavan Srinivasan, Michael Ellerman, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Paul Moore, James Morris, Serge E. Hallyn,
Jarkko Sakkinen, Ard Biesheuvel, Coiby Xu, Nicholas Piggin,
Christophe Leroy, 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: <20260306225648.GC2746259@ax162>
On Fri, Mar 6, 2026, at 23:56, Nathan Chancellor wrote:
> On Fri, Mar 06, 2026 at 04:03:24PM +0100, Arnd Bergmann wrote:
>
> 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(-)
>
Right, your version looks good to me as well.
Arnd
^ permalink raw reply
* [PATCH] landlock: add missing task != NULL check in cancel_tsync_works()
From: Tetsuo Handa @ 2026-03-07 5:21 UTC (permalink / raw)
To: linux-security-module, Günther Noack; +Cc: syzkaller-bugs, syzbot
In-Reply-To: <69abb4e3.050a0220.13f275.003d.GAE@google.com>
syzbot is reporting NULL pointer dereference at cancel_tsync_works(), for
tsync_works_release() checks for works->works[i]->task != NULL but
cancel_tsync_works() does not.
works->works[i]->task becomes NULL when tsync_works_provide() incremented
works->size and then task_work_add() returned an error. Therefore,
cancel_tsync_works() needs to check for works->works[i]->task != NULL.
Reported-by: syzbot <syzbot+741e2278ef71fef03a10@syzkaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=741e2278ef71fef03a10
Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
security/landlock/tsync.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index de01aa899751..8925acbef8a5 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -412,6 +412,8 @@ static void cancel_tsync_works(struct tsync_works *works,
int i;
for (i = 0; i < works->size; i++) {
+ if (!works->works[i]->task)
+ continue;
if (!task_work_cancel(works->works[i]->task,
&works->works[i]->work))
continue;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] landlock: add missing task != NULL check in cancel_tsync_works()
From: Mickaël Salaün @ 2026-03-07 9:01 UTC (permalink / raw)
To: Tetsuo Handa
Cc: linux-security-module, Günther Noack, syzkaller-bugs, syzbot
In-Reply-To: <a96efa12-003b-46ed-9444-40b69d84fa05@I-love.SAKURA.ne.jp>
Thanks. This issue was fixed in -next with
https://lore.kernel.org/all/20260217122341.2359582-1-mic@digikod.net/
I'll send a PR next week.
On Sat, Mar 07, 2026 at 02:21:32PM +0900, Tetsuo Handa wrote:
> syzbot is reporting NULL pointer dereference at cancel_tsync_works(), for
> tsync_works_release() checks for works->works[i]->task != NULL but
> cancel_tsync_works() does not.
>
> works->works[i]->task becomes NULL when tsync_works_provide() incremented
> works->size and then task_work_add() returned an error. Therefore,
> cancel_tsync_works() needs to check for works->works[i]->task != NULL.
>
> Reported-by: syzbot <syzbot+741e2278ef71fef03a10@syzkaller.appspotmail.com>
> Closes: https://syzkaller.appspot.com/bug?extid=741e2278ef71fef03a10
> Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> security/landlock/tsync.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index de01aa899751..8925acbef8a5 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -412,6 +412,8 @@ static void cancel_tsync_works(struct tsync_works *works,
> int i;
>
> for (i = 0; i < works->size; i++) {
> + if (!works->works[i]->task)
> + continue;
> if (!task_work_cancel(works->works[i]->task,
> &works->works[i]->work))
> continue;
> --
> 2.53.0
>
>
^ permalink raw reply
* [PATCH] docs: security: ipe: fix typos and grammar
From: Evan Ducas @ 2026-03-08 3:16 UTC (permalink / raw)
To: wufan, corbet, skhan
Cc: linux-security-module, linux-doc, linux-kernel, Evan Ducas
Fix several spelling and grammar mistakes in the IPE
documentation.
No functional change.
Signed-off-by: Evan Ducas <evan.j.ducas@gmail.com>
---
Documentation/security/ipe.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/security/ipe.rst b/Documentation/security/ipe.rst
index 4a7d953abcdc..d29824d7fd2d 100644
--- a/Documentation/security/ipe.rst
+++ b/Documentation/security/ipe.rst
@@ -18,7 +18,7 @@ strong integrity guarantees over both the executable code, and specific
*data files* on the system, that were critical to its function. These
specific data files would not be readable unless they passed integrity
policy. A mandatory access control system would be present, and
-as a result, xattrs would have to be protected. This lead to a selection
+as a result, xattrs would have to be protected. This led to a selection
of what would provide the integrity claims. At the time, there were two
main mechanisms considered that could guarantee integrity for the system
with these requirements:
@@ -195,7 +195,7 @@ of the policy to apply the minute usermode starts. Generally, that storage
can be handled in one of three ways:
1. The policy file(s) live on disk and the kernel loads the policy prior
- to an code path that would result in an enforcement decision.
+ to a code path that would result in an enforcement decision.
2. The policy file(s) are passed by the bootloader to the kernel, who
parses the policy.
3. There is a policy file that is compiled into the kernel that is
@@ -235,7 +235,7 @@ Updatable, Rebootless Policy
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
As requirements change over time (vulnerabilities are found in previously
-trusted applications, keys roll, etcetera). Updating a kernel to change the
+trusted applications, keys roll, etcetera). Updating a kernel to change to
meet those security goals is not always a suitable option, as updates are not
always risk-free, and blocking a security update leaves systems vulnerable.
This means IPE requires a policy that can be completely updated (allowing
@@ -370,7 +370,7 @@ Simplified Policy:
Finally, IPE's policy is designed for sysadmins, not kernel developers. Instead
of covering individual LSM hooks (or syscalls), IPE covers operations. This means
instead of sysadmins needing to know that the syscalls ``mmap``, ``mprotect``,
-``execve``, and ``uselib`` must have rules protecting them, they must simple know
+``execve``, and ``uselib`` must have rules protecting them, they must simply know
that they want to restrict code execution. This limits the amount of bypasses that
could occur due to a lack of knowledge of the underlying system; whereas the
maintainers of IPE, being kernel developers can make the correct choice to determine
--
2.43.0
^ permalink raw reply related
* Re: [syzbot] [fuse?] general protection fault in task_work_cancel
From: syzbot @ 2026-03-08 3:46 UTC (permalink / raw)
To: gnoack, linux-fsdevel, linux-kernel, linux-security-module, mic,
miklos, penguin-kernel, syzkaller-bugs
In-Reply-To: <69abb4e3.050a0220.13f275.003d.GAE@google.com>
syzbot has found a reproducer for the following issue on:
HEAD commit: 4ae12d8bd9a8 Merge tag 'kbuild-fixes-7.0-2' of git://git.k..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17dc475a580000
kernel config: https://syzkaller.appspot.com/x/.config?x=779072223d02a312
dashboard link: https://syzkaller.appspot.com/bug?extid=741e2278ef71fef03a10
compiler: Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=130b075a580000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/010ac4052aed/disk-4ae12d8b.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/2aad8bef9031/vmlinux-4ae12d8b.xz
kernel image: https://storage.googleapis.com/syzbot-assets/fd350ec4896a/bzImage-4ae12d8b.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+741e2278ef71fef03a10@syzkaller.appspotmail.com
Oops: general protection fault, probably for non-canonical address 0xdffffc000000013c: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x00000000000009e0-0x00000000000009e7]
CPU: 1 UID: 0 PID: 13249 Comm: syz.1.1775 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2026
RIP: 0010:task_work_pending include/linux/task_work.h:26 [inline]
RIP: 0010:task_work_cancel_match kernel/task_work.c:124 [inline]
RIP: 0010:task_work_cancel+0x8a/0x220 kernel/task_work.c:187
Code: b8 f1 f1 f1 f1 f8 f3 f3 f3 4b 89 44 25 00 e8 ad b9 35 00 43 c6 44 25 04 00 49 89 de 48 81 c3 e0 09 00 00 49 89 df 49 c1 ef 03 <43> 80 3c 27 00 74 08 48 89 df e8 17 fe 9f 00 48 83 3b 00 75 51 e8
RSP: 0018:ffffc9000ddffb20 EFLAGS: 00010216
RAX: ffffffff818fdfc3 RBX: 00000000000009e0 RCX: ffff88805d5c3d00
RDX: 0000000000000000 RSI: ffff888032f5c540 RDI: 0000000000000000
RBP: ffffc9000ddffbd0 R08: ffffc9000ddffc97 R09: 1ffff92001bbff92
R10: dffffc0000000000 R11: fffff52001bbff93 R12: dffffc0000000000
R13: 1ffff92001bbff68 R14: 0000000000000000 R15: 000000000000013c
FS: 00007f9c8a1cb6c0(0000) GS:ffff888125561000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000200000010000 CR3: 0000000059bf0000 CR4: 0000000000350ef0
Call Trace:
<TASK>
cancel_tsync_works security/landlock/tsync.c:415 [inline]
landlock_restrict_sibling_threads+0xdc4/0x11f0 security/landlock/tsync.c:533
__do_sys_landlock_restrict_self security/landlock/syscalls.c:574 [inline]
__se_sys_landlock_restrict_self+0x540/0x810 security/landlock/syscalls.c:482
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x14d/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f9c8939c799
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f9c8a1cb028 EFLAGS: 00000246 ORIG_RAX: 00000000000001be
RAX: ffffffffffffffda RBX: 00007f9c89616180 RCX: 00007f9c8939c799
RDX: 0000000000000000 RSI: 0000000000000008 RDI: 0000000000000003
RBP: 00007f9c89432bd9 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f9c89616218 R14: 00007f9c89616180 R15: 00007ffee5d65d48
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:task_work_pending include/linux/task_work.h:26 [inline]
RIP: 0010:task_work_cancel_match kernel/task_work.c:124 [inline]
RIP: 0010:task_work_cancel+0x8a/0x220 kernel/task_work.c:187
Code: b8 f1 f1 f1 f1 f8 f3 f3 f3 4b 89 44 25 00 e8 ad b9 35 00 43 c6 44 25 04 00 49 89 de 48 81 c3 e0 09 00 00 49 89 df 49 c1 ef 03 <43> 80 3c 27 00 74 08 48 89 df e8 17 fe 9f 00 48 83 3b 00 75 51 e8
RSP: 0018:ffffc9000ddffb20 EFLAGS: 00010216
RAX: ffffffff818fdfc3 RBX: 00000000000009e0 RCX: ffff88805d5c3d00
RDX: 0000000000000000 RSI: ffff888032f5c540 RDI: 0000000000000000
RBP: ffffc9000ddffbd0 R08: ffffc9000ddffc97 R09: 1ffff92001bbff92
R10: dffffc0000000000 R11: fffff52001bbff93 R12: dffffc0000000000
R13: 1ffff92001bbff68 R14: 0000000000000000 R15: 000000000000013c
FS: 00007f9c8a1cb6c0(0000) GS:ffff888125561000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f810d34da08 CR3: 0000000059bf0000 CR4: 0000000000350ef0
----------------
Code disassembly (best guess):
0: b8 f1 f1 f1 f1 mov $0xf1f1f1f1,%eax
5: f8 clc
6: f3 f3 f3 4b 89 44 25 repz repz xrelease mov %rax,0x0(%r13,%r12,1)
d: 00
e: e8 ad b9 35 00 call 0x35b9c0
13: 43 c6 44 25 04 00 movb $0x0,0x4(%r13,%r12,1)
19: 49 89 de mov %rbx,%r14
1c: 48 81 c3 e0 09 00 00 add $0x9e0,%rbx
23: 49 89 df mov %rbx,%r15
26: 49 c1 ef 03 shr $0x3,%r15
* 2a: 43 80 3c 27 00 cmpb $0x0,(%r15,%r12,1) <-- trapping instruction
2f: 74 08 je 0x39
31: 48 89 df mov %rbx,%rdi
34: e8 17 fe 9f 00 call 0x9ffe50
39: 48 83 3b 00 cmpq $0x0,(%rbx)
3d: 75 51 jne 0x90
3f: e8 .byte 0xe8
---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
^ permalink raw reply
* Re: [PATCH] docs: security: ipe: fix typos and grammar
From: Randy Dunlap @ 2026-03-08 6:17 UTC (permalink / raw)
To: Evan Ducas, wufan, corbet, skhan
Cc: linux-security-module, linux-doc, linux-kernel
In-Reply-To: <20260308031633.28890-1-evan.j.ducas@gmail.com>
Hi,
On 3/7/26 7:16 PM, Evan Ducas wrote:
> @@ -235,7 +235,7 @@ Updatable, Rebootless Policy
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> As requirements change over time (vulnerabilities are found in previously
> -trusted applications, keys roll, etcetera). Updating a kernel to change the
> +trusted applications, keys roll, etcetera). Updating a kernel to change to
What is the first sentence in the paragraph above?
Maybe s/. U/, u/ ?
> meet those security goals is not always a suitable option, as updates are not
> always risk-free, and blocking a security update leaves systems vulnerable.
> This means IPE requires a policy that can be completely updated (allowing
--
~Randy
^ permalink raw reply
* Re: [PATCH] docs: security: ipe: fix typos and grammar
From: Bagas Sanjaya @ 2026-03-08 7:48 UTC (permalink / raw)
To: Evan Ducas, wufan, corbet, skhan
Cc: linux-security-module, linux-doc, linux-kernel
In-Reply-To: <20260308031633.28890-1-evan.j.ducas@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 721 bytes --]
On Sat, Mar 07, 2026 at 10:16:33PM -0500, Evan Ducas wrote:
> As requirements change over time (vulnerabilities are found in previously
> -trusted applications, keys roll, etcetera). Updating a kernel to change the
> +trusted applications, keys roll, etcetera). Updating a kernel to change to
> meet those security goals is not always a suitable option, as updates are not
> always risk-free, and blocking a security update leaves systems vulnerable.
> This means IPE requires a policy that can be completely updated (allowing
As requirements change over time ..., updating a kernel to meet ..., yet
blocking a security update ... .
Thanks.
--
An old man doll... just what I always wanted! - Clara
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v5 2/9] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-03-08 9:09 UTC (permalink / raw)
To: Günther Noack
Cc: Günther Noack, John Johansen, Tingmao Wang, Justin Suess,
Jann Horn, linux-security-module, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Demi Marie Obenour, Alyssa Ross, Tahera Fahimi
In-Reply-To: <aZcXSmhZRVcRCvum@google.com>
On Thu, Feb 19, 2026 at 02:59:38PM +0100, Günther Noack wrote:
> On Thu, Feb 19, 2026 at 10:45:44AM +0100, Mickaël Salaün wrote:
> > On Wed, Feb 18, 2026 at 10:37:16AM +0100, Mickaël Salaün wrote:
> > > On Sun, Feb 15, 2026 at 11:51:50AM +0100, Günther Noack wrote:
> > > > * Add a new access right LANDLOCK_ACCESS_FS_RESOLVE_UNIX, which
> > > > controls the look up operations for named UNIX domain sockets. The
> > > > resolution happens during connect() and sendmsg() (depending on
> > > > socket type).
> > > > * Hook into the path lookup in unix_find_bsd() in af_unix.c, using a
> > > > LSM hook. Make policy decisions based on the new access rights
> > > > * Increment the Landlock ABI version.
> > > > * Minor test adaptions to keep the tests working.
> > > >
> > > > With this access right, access is granted if either of the following
> > > > conditions is met:
> > > >
> > > > * The target socket's filesystem path was allow-listed using a
> > > > LANDLOCK_RULE_PATH_BENEATH rule, *or*:
> > > > * The target socket was created in the same Landlock domain in which
> > > > LANDLOCK_ACCESS_FS_RESOLVE_UNIX was restricted.
> > > >
> > > > In case of a denial, connect() and sendmsg() return EACCES, which is
> > > > the same error as it is returned if the user does not have the write
> > > > bit in the traditional Unix file system permissions of that file.
> > > >
> > > > This feature was created with substantial discussion and input from
> > > > Justin Suess, Tingmao Wang and Mickaël Salaün.
> > > >
> > > > Cc: Tingmao Wang <m@maowtm.org>
> > > > Cc: Justin Suess <utilityemal77@gmail.com>
> > > > Cc: Mickaël Salaün <mic@digikod.net>
> > > > Suggested-by: Jann Horn <jannh@google.com>
> > > > Link: https://github.com/landlock-lsm/linux/issues/36
> > > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > > ---
> > > > include/uapi/linux/landlock.h | 10 ++
> > > > security/landlock/access.h | 11 +-
> > > > security/landlock/audit.c | 1 +
> > > > security/landlock/fs.c | 102 ++++++++++++++++++-
> > > > security/landlock/limits.h | 2 +-
> > > > security/landlock/syscalls.c | 2 +-
> > > > tools/testing/selftests/landlock/base_test.c | 2 +-
> > > > tools/testing/selftests/landlock/fs_test.c | 5 +-
> > > > 8 files changed, 128 insertions(+), 7 deletions(-)
> >
> > > > index 60ff217ab95b..8d0edf94037d 100644
> > > > --- a/security/landlock/audit.c
> > > > +++ b/security/landlock/audit.c
> > > > @@ -37,6 +37,7 @@ static const char *const fs_access_strings[] = {
> > > > [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
> > > > [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
> > > > [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
> > > > + [BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX)] = "fs.resolve_unix",
> > > > };
> > > >
> > > > static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> > > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > > > index e764470f588c..76035c6f2bf1 100644
> > > > --- a/security/landlock/fs.c
> > > > +++ b/security/landlock/fs.c
> > > > @@ -27,6 +27,7 @@
> > > > #include <linux/lsm_hooks.h>
> > > > #include <linux/mount.h>
> > > > #include <linux/namei.h>
> > > > +#include <linux/net.h>
> > > > #include <linux/path.h>
> > > > #include <linux/pid.h>
> > > > #include <linux/rcupdate.h>
> > > > @@ -314,7 +315,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> > > > LANDLOCK_ACCESS_FS_WRITE_FILE | \
> > > > LANDLOCK_ACCESS_FS_READ_FILE | \
> > > > LANDLOCK_ACCESS_FS_TRUNCATE | \
> > > > - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> > > > + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> > > > + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
> > > > /* clang-format on */
> > > >
> > > > /*
> > > > @@ -1561,6 +1563,103 @@ static int hook_path_truncate(const struct path *const path)
> > > > return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
> > > > }
> > > >
> > > > +/**
> > > > + * unmask_scoped_access - Remove access right bits in @masks in all layers
> > > > + * where @client and @server have the same domain
> > > > + *
> > > > + * This does the same as domain_is_scoped(), but unmasks bits in @masks.
> > > > + * It can not return early as domain_is_scoped() does.
> >
> > Why can't we use the same logic as for other scopes?
>
> The other scopes, for which this is implemented in domain_is_scoped(),
> do not need to do this layer-by-layer.
>
> I have to admit, in my initial implementation, I was using
> domain_is_scoped() directly, and the logic at the end of the hook was
> roughly:
>
> --- BUGGY CODE START ---
> // ...
>
> if (!domain_is_scoped(..., ..., LANDLOCK_ACCESS_FS_RESOLVE_UNIX))
> return 0; /* permitted */
>
> return current_check_access_path(path, LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
> }
> --- BUGGY CODE END ---
>
> Unfortunately, that is a logic error though -- it implements the formula
>
> Access granted if:
> (FOR-ALL l ∈ layers scoped-access-ok(l)) OR (FOR-ALL l ∈ layers path-access-ok(l)) (WRONG!)
>
> but the formula we want is:
>
> Access granted if:
> FOR-ALL l ∈ layers (scoped-access-ok(l) OR path-access-ok(l)) (CORRECT!)
It is worth it to add this explanation to the unmask_scoped_access()
description, also pointing to the test that check this case.
>
> This makes a difference in the case where (pseudocode):
>
> 1. landlock_restrict_self(RESOLVE_UNIX) // d1
> 2. create_unix_server("./sock")
> 3. landlock_restrict_self(RESOLVE_UNIX, rule=Allow(".", RESOLVE_UNIX)) // d2
> 4. connect_unix("./sock")
>
> ,------------------------------------------------d1--,
> | |
> | ./sock server |
> | ^ |
> | | |
> | ,------------------------------------------d2--, |
> | | | | |
> | | client | |
> | | | |
> | '----------------------------------------------' |
> | |
> '----------------------------------------------------'
>
> (BTW, this scenario is covered in the selftests, that is why there is
> a variant of these selftests where instead of applying "no domain", we
> apply a domain with an exception rule like in step 3 in the pseudocode
> above. Applying that domain should behave the same as applying no
> domain at all.)
>
> Intuitively, it is clear that the access should be granted:
>
> - d1 does not restrict access to the server,
> because the socket was created within d1 itself.
> - d2 does not restrict access to the server,
> because it has a rule to allow it
>
> But the "buggy code" logic above comes to a different conclusion:
>
> - the domain_is_scoped() check denies the access, because the server
> is in a more privileged domain relative to the client domain.
> - the current_check_access_path() check denies the access as well,
> because the socket's path is not allow-listed in d1.
>
> In the 'intuitive' reasoning above, we are checking d1 and d2
> independently of each other. While Landlock is not implemented like
> that internally, we need to stay consistent with it so that domains
> compose correctly. The way to do that is to track is access check
> results on a per-layer basis again, and that is why
> unmask_scoped_access() uses a layer mask for tracking. The original
> domain_is_scoped() does not use a layer mask, but that also means that
> it can return early in some scenarios -- if for any of the relevant
> layer depths, the client and server domains are not the same, it exits
> early with failure because it's overall not fulfillable any more. In
> the RESOLVE_UNIX case though, we need to remember in which layers we
> failed (both high an low ones), because these layers can still be
> fulfilled with a PATH_BENEATH rule later.
>
> Summary:
>
> Option 1: We *can* unify this if you want. It just might come at a
> small performance penalty for domain_is_scoped(), which now uses the
> larger layer mask data structure and can't do the same early returns
> any more as before.
>
> Option 2: Alternatively, if we move the two functions into the same
> module, we can keep them separate but still test them against each
> other to make sure they are in-line:
>
> This invocation should return true...
>
> domain_is_scoped(cli, srv, access)
>
> ...in the exactly the same situations where this invocation leaves any
> bits set in layer_masks:
>
> landlock_init_layer_masks(dom, access, &layer_masks, LL_KEY_INODE);
> unmask_scoped_access(cli, srv, &layer_masks, access);
>
> What do you prefer?
I was thinking about factoring out domain_is_scoped() with
unmask_scoped_access() but, after some tests, it is not worth it. Your
approach is simple and good.
>
>
> > > > + *
> > > > + * @client: Client domain
> > > > + * @server: Server domain
> > > > + * @masks: Layer access masks to unmask
> > > > + * @access: Access bit that controls scoping
> > > > + */
> > > > +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> > > > + const struct landlock_ruleset *const server,
> > > > + struct layer_access_masks *const masks,
> > > > + const access_mask_t access)
> > >
> > > This helper should be moved to task.c and factored out with
> > > domain_is_scoped(). This should be a dedicated patch.
> >
> > Well, if domain_is_scoped() can be refactored and made generic, it would
> > make more sense to move it to domain.c
> >
> > >
> > > > +{
> > > > + int client_layer, server_layer;
> > > > + const struct landlock_hierarchy *client_walker, *server_walker;
> > > > +
> > > > + if (WARN_ON_ONCE(!client))
> > > > + return; /* should not happen */
Please no comment after ";"
> > > > +
> > > > + if (!server)
> > > > + return; /* server has no Landlock domain; nothing to clear */
> > > > +
> > > > + client_layer = client->num_layers - 1;
> > > > + client_walker = client->hierarchy;
> > > > + server_layer = server->num_layers - 1;
> > > > + server_walker = server->hierarchy;
> > > > +
> > > > + /*
> > > > + * Clears the access bits at all layers where the client domain is the
> > > > + * same as the server domain. We start the walk at min(client_layer,
> > > > + * server_layer). The layer bits until there can not be cleared because
> > > > + * either the client or the server domain is missing.
> > > > + */
> > > > + for (; client_layer > server_layer; client_layer--)
> > > > + client_walker = client_walker->parent;
> > > > +
> > > > + for (; server_layer > client_layer; server_layer--)
> > > > + server_walker = server_walker->parent;
> > > > +
> > > > + for (; client_layer >= 0; client_layer--) {
> > > > + if (masks->access[client_layer] & access &&
> > > > + client_walker == server_walker)
I'd prefer to first check client_walker == server_walker and then the
access. My main concern is that only one bit of access matching
masks->access[client_layer] clear all the access request bits. In
practice there is only one, for now, but this code should be more strict
by following a defensive approach.
> > > > + masks->access[client_layer] &= ~access;
> > > > +
> > > > + client_walker = client_walker->parent;
> > > > + server_walker = server_walker->parent;
> > > > + }
> > > > +}
>
^ permalink raw reply
* Re: [PATCH v5 2/9] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-03-08 9:09 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Tingmao Wang, Justin Suess, Jann Horn,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Tahera Fahimi
In-Reply-To: <20260215105158.28132-3-gnoack3000@gmail.com>
On Sun, Feb 15, 2026 at 11:51:50AM +0100, Günther Noack wrote:
> * Add a new access right LANDLOCK_ACCESS_FS_RESOLVE_UNIX, which
> controls the look up operations for named UNIX domain sockets. The
> resolution happens during connect() and sendmsg() (depending on
> socket type).
> * Hook into the path lookup in unix_find_bsd() in af_unix.c, using a
> LSM hook. Make policy decisions based on the new access rights
> * Increment the Landlock ABI version.
> * Minor test adaptions to keep the tests working.
>
> With this access right, access is granted if either of the following
> conditions is met:
>
> * The target socket's filesystem path was allow-listed using a
> LANDLOCK_RULE_PATH_BENEATH rule, *or*:
> * The target socket was created in the same Landlock domain in which
> LANDLOCK_ACCESS_FS_RESOLVE_UNIX was restricted.
>
> In case of a denial, connect() and sendmsg() return EACCES, which is
> the same error as it is returned if the user does not have the write
> bit in the traditional Unix file system permissions of that file.
It is not the same error code as for scoped abstract unix socket
(EPERM), but it makes sense because the scope restrictions are closer to
ambient rights (i.e. similar to a network isolation), whereas here the
final denial comes from a missing FS rule (and all FS access checks may
return EACCES). It would be worth mentioning this difference in the
user documentation.
>
> This feature was created with substantial discussion and input from
> Justin Suess, Tingmao Wang and Mickaël Salaün.
>
> Cc: Tingmao Wang <m@maowtm.org>
> Cc: Justin Suess <utilityemal77@gmail.com>
> Cc: Mickaël Salaün <mic@digikod.net>
> Suggested-by: Jann Horn <jannh@google.com>
> Link: https://github.com/landlock-lsm/linux/issues/36
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> include/uapi/linux/landlock.h | 10 ++
> security/landlock/access.h | 11 +-
> security/landlock/audit.c | 1 +
> security/landlock/fs.c | 102 ++++++++++++++++++-
> security/landlock/limits.h | 2 +-
> security/landlock/syscalls.c | 2 +-
> tools/testing/selftests/landlock/base_test.c | 2 +-
> tools/testing/selftests/landlock/fs_test.c | 5 +-
> 8 files changed, 128 insertions(+), 7 deletions(-)
> +static int hook_unix_find(const struct path *const path, struct sock *other,
> + int flags)
> +{
> + const struct landlock_ruleset *dom_other;
> + const struct landlock_cred_security *subject;
> + struct layer_access_masks layer_masks;
> + struct landlock_request request = {};
> + static const struct access_masks fs_resolve_unix = {
> + .fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> + };
> +
> + /* Lookup for the purpose of saving coredumps is OK. */
> + if (unlikely(flags & SOCK_COREDUMP))
> + return 0;
> +
> + /* Access to the same (or a lower) domain is always allowed. */
> + subject = landlock_get_applicable_subject(current_cred(),
> + fs_resolve_unix, NULL);
> +
> + if (!subject)
> + return 0;
> +
> + if (!landlock_init_layer_masks(subject->domain, fs_resolve_unix.fs,
> + &layer_masks, LANDLOCK_KEY_INODE))
> + return 0;
> +
> + /* Checks the layers in which we are connecting within the same domain. */
> + dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
> + unmask_scoped_access(subject->domain, dom_other, &layer_masks,
> + fs_resolve_unix.fs);
> +
> + if (layer_access_masks_empty(&layer_masks))
I don't see the point of this helper and this call wrt the following
is_access_to_paths_allowed() call and the is_layer_masks_allowed()
check.
> + return 0;
> +
> + /* Checks the connections to allow-listed paths. */
> + if (is_access_to_paths_allowed(subject->domain, path,
> + fs_resolve_unix.fs, &layer_masks,
> + &request, NULL, 0, NULL, NULL, NULL))
> + return 0;
> +
> + landlock_log_denial(subject, &request);
> + return -EACCES;
> +}
^ permalink raw reply
* Re: [PATCH v5 2/9] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-03-08 9:18 UTC (permalink / raw)
To: Günther Noack, Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni,
Willem de Bruijn, Sebastian Andrzej Siewior, Jason Xing
Cc: John Johansen, Tingmao Wang, Justin Suess, Jann Horn,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Tahera Fahimi, netdev
In-Reply-To: <20260220.82a8adda6f95@gnoack.org>
On Fri, Feb 20, 2026 at 03:33:28PM +0100, Günther Noack wrote:
> +netdev, we could use some advice on the locking approach in af_unix (see below)
>
> On Wed, Feb 18, 2026 at 10:37:14AM +0100, Mickaël Salaün wrote:
> > On Sun, Feb 15, 2026 at 11:51:50AM +0100, Günther Noack wrote:
> > > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> > > index f88fa1f68b77..3a8fc3af0d64 100644
> > > --- a/include/uapi/linux/landlock.h
> > > +++ b/include/uapi/linux/landlock.h
> > > @@ -248,6 +248,15 @@ struct landlock_net_port_attr {
> > > *
> > > * This access right is available since the fifth version of the Landlock
> > > * ABI.
> > > + * - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX: Look up pathname UNIX domain sockets
> > > + * (:manpage:`unix(7)`). On UNIX domain sockets, this restricts both calls to
> > > + * :manpage:`connect(2)` as well as calls to :manpage:`sendmsg(2)` with an
> > > + * explicit recipient address.
> > > + *
> > > + * This access right only applies to connections to UNIX server sockets which
> > > + * were created outside of the newly created Landlock domain (e.g. from within
> > > + * a parent domain or from an unrestricted process). Newly created UNIX
> > > + * servers within the same Landlock domain continue to be accessible.
> >
> > It might help to add a reference to the explicit scope mechanism.
> >
> > Please squash patch 9/9 into this one and also add a reference here to
> > the rationale described in security/landlock.rst
>
> Sounds good, will do.
>
>
> > > +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> > > + const struct landlock_ruleset *const server,
> > > + struct layer_access_masks *const masks,
> > > + const access_mask_t access)
> >
> > This helper should be moved to task.c and factored out with
> > domain_is_scoped(). This should be a dedicated patch.
>
> (already discussed in another follow-up mail)
>
>
> > > +static int hook_unix_find(const struct path *const path, struct sock *other,
> > > + int flags)
> > > +{
> > > + const struct landlock_ruleset *dom_other;
> > > + const struct landlock_cred_security *subject;
> > > + struct layer_access_masks layer_masks;
> > > + struct landlock_request request = {};
> > > + static const struct access_masks fs_resolve_unix = {
> > > + .fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> > > + };
> > > +
> > > + /* Lookup for the purpose of saving coredumps is OK. */
> > > + if (unlikely(flags & SOCK_COREDUMP))
> > > + return 0;
> > > +
> > > + /* Access to the same (or a lower) domain is always allowed. */
> > > + subject = landlock_get_applicable_subject(current_cred(),
> > > + fs_resolve_unix, NULL);
> > > +
> > > + if (!subject)
> > > + return 0;
> > > +
> > > + if (!landlock_init_layer_masks(subject->domain, fs_resolve_unix.fs,
> > > + &layer_masks, LANDLOCK_KEY_INODE))
> > > + return 0;
> > > +
> > > + /* Checks the layers in which we are connecting within the same domain. */
> > > + dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
> >
> > We need to call unix_state_lock(other) before reading it, and check for
> > SOCK_DEAD, and check sk_socket before dereferencing it. Indeed,
> > the socket can be make orphan (see unix_dgram_sendmsg and
> > unix_stream_connect). I *think* a socket cannot be "resurrected" or
> > recycled once dead, so we may assume there is no race condition wrt
> > dom_other, but please double check. This lockless call should be made
> > clear in the LSM hook. It's OK to not lock the socket before
> > security_unix_find() (1) because no LSM might implement and (2) they
> > might not need to lock the socket (e.g. if the caller is not sandboxed).
> >
> > The updated code should look something like this:
> >
> > unix_state_unlock(other);
unix_state_lock(other) of course...
> > if (unlikely(sock_flag(other, SOCK_DEAD) || !other->sk_socket)) {
> > unix_state_unlock(other);
> > return 0;
> > }
> >
> > dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
> > unix_state_unlock(other);
>
> Thank you for spotting the locking concern!
>
> @anyone from netdev, could you please advise on the correct locking
> approach here?
>
> * Do we need ot check SOCK_DEAD?
>
> You are saying that we need to do that, but it's not clear to me
> why.
>
> If you look at the places where unix_find_other() is called in
> af_unix.c, then you'll find that all of them check for SOCK_DEAD and
> then restart from unix_find_other() if they do actually discover
> that the socket is dead. I think that this is catching this race
> condition scenario:
>
> * a server socket exists and is alive
> * A client connects: af_unix.c's unix_stream_connect() calls
> unix_find_other() and finds the server socket...
> * (Concurrently): The server closes the socket and enters
> unix_release_sock(). This function:
> 1. disassociates the server sock from the named socket inode
> number in the hash table (=> future unix_find_other() calls
> will fail).
> 2. calls sock_orphan(), which sets SOCK_DEAD.
> * ...(client connection resuming): unix_stream_connect() continues,
> grabs the unix_state_lock(), which apparently protects everything
> here, checks that the socket is not dead - and discovers that it
> IS suddenly dead. This was not supposed to happen. The code
> recovers from that by retrying everything starting with the
> unix_find_other() call. From unix_release_sock(), we know that
> the inode is not associated with the sock any more -- so the
> unix_find_socket_by_inode() call should be failing on the next
> attempt.
>
> (This works with unix_dgram_connect() and unix_dgram_sendmsg() as
> well.)
>
> The comments next to the SOCK_DEAD checks are also suggesting this.
>
> * What lock to use
>
> I am having trouble reasoning about what lock is used for what in
> this code.
It's not clear to me neither, and it looks like it's not consistent
across protocols.
>
> Is it possible that the lock protecting ->sk_socket is the
> ->sk_callback_lock, not the unix_state_lock()? The only callers to
> sk_set_socket are either sock_orphan/sock_graft (both grabbing
> sk_callback_lock), or they are creating new struct sock objects that
> they own exclusively, and don't need locks yet.
>
> Admittedly, in af_unix.c, sock_orphan() and sock_graft() only get
> called in contexts where the unix_state_lock() is held, so it would
> probably work as well to lock that, but it is maybe a more
> fine-grained approach to use sk_callback_lock?
>
>
> So... how about a scheme where we only check for ->sk_socket not being
> NULL:
>
> read_lock_bh(&other->sk_callback_lock);
> struct sock *other_sk = other->sk_socket;
> if (!other_sk) {
> read_unlock_bh(&other->sk_callback_lock);
> return 0;
> }
> /* XXX double check whether we need a lock here too */
> struct file *file = other_sk->file;
> if (!other_file) {
> read_unlock_bh(&other->sk_callback_lock);
> return 0;
> }
> read_unlock_bh(&other->sk_callback_lock);
>
> If this fails, that would in my understanding also be because the
> socket has died after the path lookup. We'd then return 0 (success),
> because we know that the surrounding SOCK_DEAD logic will repeat
> everything starting from the path lookup operation (this time likely
> failing with ECONNREFUSED, but maybe also with a success, if another
> server process was quick enough).
>
> Does this sound reasonable?
Actually, since commit 983512f3a87f ("net: Drop the lock in
skb_may_tx_timestamp()"), we can just use RCU + READ_ONCE(sk_socket) +
READ_ONCE(file). The socket and file should only be freed after the RCU
grace periode. As a safeguard, this commit should be a Depends-on.
However, it is safer to return -ECONNREFULED when sk_socket or file are
NULL.
I would be good to hear from netdev folks though.
TIL, there is an LSM hook for sock_graft().
> –Günther
>
^ permalink raw reply
* Re: [PATCH v5 2/9] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-03-08 11:50 UTC (permalink / raw)
To: Günther Noack
Cc: Günther Noack, John Johansen, Tingmao Wang, Justin Suess,
Jann Horn, linux-security-module, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Demi Marie Obenour, Alyssa Ross, Tahera Fahimi
In-Reply-To: <20260307.aeth4weik2Ah@digikod.net>
On Sun, Mar 08, 2026 at 10:09:52AM +0100, Mickaël Salaün wrote:
> On Thu, Feb 19, 2026 at 02:59:38PM +0100, Günther Noack wrote:
> > On Thu, Feb 19, 2026 at 10:45:44AM +0100, Mickaël Salaün wrote:
> > > On Wed, Feb 18, 2026 at 10:37:16AM +0100, Mickaël Salaün wrote:
> > > > On Sun, Feb 15, 2026 at 11:51:50AM +0100, Günther Noack wrote:
> > > > > * Add a new access right LANDLOCK_ACCESS_FS_RESOLVE_UNIX, which
> > > > > controls the look up operations for named UNIX domain sockets. The
> > > > > resolution happens during connect() and sendmsg() (depending on
> > > > > socket type).
> > > > > * Hook into the path lookup in unix_find_bsd() in af_unix.c, using a
> > > > > LSM hook. Make policy decisions based on the new access rights
> > > > > * Increment the Landlock ABI version.
> > > > > * Minor test adaptions to keep the tests working.
> > > > >
> > > > > With this access right, access is granted if either of the following
> > > > > conditions is met:
> > > > >
> > > > > * The target socket's filesystem path was allow-listed using a
> > > > > LANDLOCK_RULE_PATH_BENEATH rule, *or*:
> > > > > * The target socket was created in the same Landlock domain in which
> > > > > LANDLOCK_ACCESS_FS_RESOLVE_UNIX was restricted.
> > > > >
> > > > > In case of a denial, connect() and sendmsg() return EACCES, which is
> > > > > the same error as it is returned if the user does not have the write
> > > > > bit in the traditional Unix file system permissions of that file.
> > > > >
> > > > > This feature was created with substantial discussion and input from
> > > > > Justin Suess, Tingmao Wang and Mickaël Salaün.
> > > > >
> > > > > Cc: Tingmao Wang <m@maowtm.org>
> > > > > Cc: Justin Suess <utilityemal77@gmail.com>
> > > > > Cc: Mickaël Salaün <mic@digikod.net>
> > > > > Suggested-by: Jann Horn <jannh@google.com>
> > > > > Link: https://github.com/landlock-lsm/linux/issues/36
> > > > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > > > ---
> > > > > include/uapi/linux/landlock.h | 10 ++
> > > > > security/landlock/access.h | 11 +-
> > > > > security/landlock/audit.c | 1 +
> > > > > security/landlock/fs.c | 102 ++++++++++++++++++-
> > > > > security/landlock/limits.h | 2 +-
> > > > > security/landlock/syscalls.c | 2 +-
> > > > > tools/testing/selftests/landlock/base_test.c | 2 +-
> > > > > tools/testing/selftests/landlock/fs_test.c | 5 +-
> > > > > 8 files changed, 128 insertions(+), 7 deletions(-)
> > >
> > > > > index 60ff217ab95b..8d0edf94037d 100644
> > > > > --- a/security/landlock/audit.c
> > > > > +++ b/security/landlock/audit.c
> > > > > @@ -37,6 +37,7 @@ static const char *const fs_access_strings[] = {
> > > > > [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
> > > > > [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
> > > > > [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
> > > > > + [BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX)] = "fs.resolve_unix",
> > > > > };
> > > > >
> > > > > static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> > > > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > > > > index e764470f588c..76035c6f2bf1 100644
> > > > > --- a/security/landlock/fs.c
> > > > > +++ b/security/landlock/fs.c
> > > > > @@ -27,6 +27,7 @@
> > > > > #include <linux/lsm_hooks.h>
> > > > > #include <linux/mount.h>
> > > > > #include <linux/namei.h>
> > > > > +#include <linux/net.h>
> > > > > #include <linux/path.h>
> > > > > #include <linux/pid.h>
> > > > > #include <linux/rcupdate.h>
> > > > > @@ -314,7 +315,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> > > > > LANDLOCK_ACCESS_FS_WRITE_FILE | \
> > > > > LANDLOCK_ACCESS_FS_READ_FILE | \
> > > > > LANDLOCK_ACCESS_FS_TRUNCATE | \
> > > > > - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> > > > > + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> > > > > + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
> > > > > /* clang-format on */
> > > > >
> > > > > /*
> > > > > @@ -1561,6 +1563,103 @@ static int hook_path_truncate(const struct path *const path)
> > > > > return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
> > > > > }
> > > > >
> > > > > +/**
> > > > > + * unmask_scoped_access - Remove access right bits in @masks in all layers
> > > > > + * where @client and @server have the same domain
> > > > > + *
> > > > > + * This does the same as domain_is_scoped(), but unmasks bits in @masks.
> > > > > + * It can not return early as domain_is_scoped() does.
> > >
> > > Why can't we use the same logic as for other scopes?
> >
> > The other scopes, for which this is implemented in domain_is_scoped(),
> > do not need to do this layer-by-layer.
> >
> > I have to admit, in my initial implementation, I was using
> > domain_is_scoped() directly, and the logic at the end of the hook was
> > roughly:
> >
> > --- BUGGY CODE START ---
> > // ...
> >
> > if (!domain_is_scoped(..., ..., LANDLOCK_ACCESS_FS_RESOLVE_UNIX))
> > return 0; /* permitted */
> >
> > return current_check_access_path(path, LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
> > }
> > --- BUGGY CODE END ---
> >
> > Unfortunately, that is a logic error though -- it implements the formula
> >
> > Access granted if:
> > (FOR-ALL l ∈ layers scoped-access-ok(l)) OR (FOR-ALL l ∈ layers path-access-ok(l)) (WRONG!)
> >
> > but the formula we want is:
> >
> > Access granted if:
> > FOR-ALL l ∈ layers (scoped-access-ok(l) OR path-access-ok(l)) (CORRECT!)
>
> It is worth it to add this explanation to the unmask_scoped_access()
> description, also pointing to the test that check this case.
>
> >
> > This makes a difference in the case where (pseudocode):
> >
> > 1. landlock_restrict_self(RESOLVE_UNIX) // d1
> > 2. create_unix_server("./sock")
> > 3. landlock_restrict_self(RESOLVE_UNIX, rule=Allow(".", RESOLVE_UNIX)) // d2
> > 4. connect_unix("./sock")
> >
> > ,------------------------------------------------d1--,
> > | |
> > | ./sock server |
> > | ^ |
> > | | |
> > | ,------------------------------------------d2--, |
> > | | | | |
> > | | client | |
> > | | | |
> > | '----------------------------------------------' |
> > | |
> > '----------------------------------------------------'
> >
> > (BTW, this scenario is covered in the selftests, that is why there is
> > a variant of these selftests where instead of applying "no domain", we
> > apply a domain with an exception rule like in step 3 in the pseudocode
> > above. Applying that domain should behave the same as applying no
> > domain at all.)
> >
> > Intuitively, it is clear that the access should be granted:
> >
> > - d1 does not restrict access to the server,
> > because the socket was created within d1 itself.
> > - d2 does not restrict access to the server,
> > because it has a rule to allow it
> >
> > But the "buggy code" logic above comes to a different conclusion:
> >
> > - the domain_is_scoped() check denies the access, because the server
> > is in a more privileged domain relative to the client domain.
> > - the current_check_access_path() check denies the access as well,
> > because the socket's path is not allow-listed in d1.
> >
> > In the 'intuitive' reasoning above, we are checking d1 and d2
> > independently of each other. While Landlock is not implemented like
> > that internally, we need to stay consistent with it so that domains
> > compose correctly. The way to do that is to track is access check
> > results on a per-layer basis again, and that is why
> > unmask_scoped_access() uses a layer mask for tracking. The original
> > domain_is_scoped() does not use a layer mask, but that also means that
> > it can return early in some scenarios -- if for any of the relevant
> > layer depths, the client and server domains are not the same, it exits
> > early with failure because it's overall not fulfillable any more. In
> > the RESOLVE_UNIX case though, we need to remember in which layers we
> > failed (both high an low ones), because these layers can still be
> > fulfilled with a PATH_BENEATH rule later.
> >
> > Summary:
> >
> > Option 1: We *can* unify this if you want. It just might come at a
> > small performance penalty for domain_is_scoped(), which now uses the
> > larger layer mask data structure and can't do the same early returns
> > any more as before.
> >
> > Option 2: Alternatively, if we move the two functions into the same
> > module, we can keep them separate but still test them against each
> > other to make sure they are in-line:
> >
> > This invocation should return true...
> >
> > domain_is_scoped(cli, srv, access)
> >
> > ...in the exactly the same situations where this invocation leaves any
> > bits set in layer_masks:
> >
> > landlock_init_layer_masks(dom, access, &layer_masks, LL_KEY_INODE);
> > unmask_scoped_access(cli, srv, &layer_masks, access);
> >
> > What do you prefer?
>
> I was thinking about factoring out domain_is_scoped() with
> unmask_scoped_access() but, after some tests, it is not worth it. Your
> approach is simple and good.
>
> >
> >
> > > > > + *
> > > > > + * @client: Client domain
> > > > > + * @server: Server domain
> > > > > + * @masks: Layer access masks to unmask
> > > > > + * @access: Access bit that controls scoping
> > > > > + */
> > > > > +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> > > > > + const struct landlock_ruleset *const server,
> > > > > + struct layer_access_masks *const masks,
> > > > > + const access_mask_t access)
> > > >
> > > > This helper should be moved to task.c and factored out with
> > > > domain_is_scoped(). This should be a dedicated patch.
> > >
> > > Well, if domain_is_scoped() can be refactored and made generic, it would
> > > make more sense to move it to domain.c
> > >
> > > >
> > > > > +{
> > > > > + int client_layer, server_layer;
> > > > > + const struct landlock_hierarchy *client_walker, *server_walker;
> > > > > +
> > > > > + if (WARN_ON_ONCE(!client))
> > > > > + return; /* should not happen */
>
> Please no comment after ";"
>
> > > > > +
> > > > > + if (!server)
> > > > > + return; /* server has no Landlock domain; nothing to clear */
> > > > > +
> > > > > + client_layer = client->num_layers - 1;
> > > > > + client_walker = client->hierarchy;
> > > > > + server_layer = server->num_layers - 1;
> > > > > + server_walker = server->hierarchy;
> > > > > +
> > > > > + /*
> > > > > + * Clears the access bits at all layers where the client domain is the
> > > > > + * same as the server domain. We start the walk at min(client_layer,
> > > > > + * server_layer). The layer bits until there can not be cleared because
> > > > > + * either the client or the server domain is missing.
> > > > > + */
> > > > > + for (; client_layer > server_layer; client_layer--)
> > > > > + client_walker = client_walker->parent;
> > > > > +
> > > > > + for (; server_layer > client_layer; server_layer--)
> > > > > + server_walker = server_walker->parent;
> > > > > +
> > > > > + for (; client_layer >= 0; client_layer--) {
> > > > > + if (masks->access[client_layer] & access &&
> > > > > + client_walker == server_walker)
>
> I'd prefer to first check client_walker == server_walker and then the
> access. My main concern is that only one bit of access matching
> masks->access[client_layer] clear all the access request bits. In
> practice there is only one, for now, but this code should be more strict
> by following a defensive approach.
>
> > > > > + masks->access[client_layer] &= ~access;
Actually, why not removing the access argument and just reset
masks->access[client_layer]? The doc would need some updates.
> > > > > +
> > > > > + client_walker = client_walker->parent;
> > > > > + server_walker = server_walker->parent;
> > > > > + }
> > > > > +}
> >
^ permalink raw reply
* [PATCH v2] docs: security: ipe: fix typos and grammar
From: Evan Ducas @ 2026-03-08 18:07 UTC (permalink / raw)
To: wufan, corbet, skhan
Cc: rdunlap, bagasdotme, linux-security-module, linux-doc,
linux-kernel, Evan Ducas
Fix several spelling and grammar mistakes in the IPE
documentation.
No functional change.
Signed-off-by: Evan Ducas <evan.j.ducas@gmail.com>
---
Documentation/security/ipe.rst | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/security/ipe.rst b/Documentation/security/ipe.rst
index 4a7d953abcdc..5eb3e6265fbd 100644
--- a/Documentation/security/ipe.rst
+++ b/Documentation/security/ipe.rst
@@ -18,7 +18,7 @@ strong integrity guarantees over both the executable code, and specific
*data files* on the system, that were critical to its function. These
specific data files would not be readable unless they passed integrity
policy. A mandatory access control system would be present, and
-as a result, xattrs would have to be protected. This lead to a selection
+as a result, xattrs would have to be protected. This led to a selection
of what would provide the integrity claims. At the time, there were two
main mechanisms considered that could guarantee integrity for the system
with these requirements:
@@ -195,7 +195,7 @@ of the policy to apply the minute usermode starts. Generally, that storage
can be handled in one of three ways:
1. The policy file(s) live on disk and the kernel loads the policy prior
- to an code path that would result in an enforcement decision.
+ to a code path that would result in an enforcement decision.
2. The policy file(s) are passed by the bootloader to the kernel, who
parses the policy.
3. There is a policy file that is compiled into the kernel that is
@@ -235,8 +235,8 @@ Updatable, Rebootless Policy
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
As requirements change over time (vulnerabilities are found in previously
-trusted applications, keys roll, etcetera). Updating a kernel to change the
-meet those security goals is not always a suitable option, as updates are not
+trusted applications, keys roll, etcetera), updating a kernel to meet
+those security goals is not always a suitable option, as updates are not
always risk-free, and blocking a security update leaves systems vulnerable.
This means IPE requires a policy that can be completely updated (allowing
revocations of existing policy) from a source external to the kernel (allowing
@@ -370,7 +370,7 @@ Simplified Policy:
Finally, IPE's policy is designed for sysadmins, not kernel developers. Instead
of covering individual LSM hooks (or syscalls), IPE covers operations. This means
instead of sysadmins needing to know that the syscalls ``mmap``, ``mprotect``,
-``execve``, and ``uselib`` must have rules protecting them, they must simple know
+``execve``, and ``uselib`` must have rules protecting them, they must simply know
that they want to restrict code execution. This limits the amount of bypasses that
could occur due to a lack of knowledge of the underlying system; whereas the
maintainers of IPE, being kernel developers can make the correct choice to determine
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] docs: security: ipe: fix typos and grammar
From: Randy Dunlap @ 2026-03-08 19:41 UTC (permalink / raw)
To: Evan Ducas, wufan, corbet, skhan
Cc: bagasdotme, linux-security-module, linux-doc, linux-kernel
In-Reply-To: <20260308180734.5792-1-evan.j.ducas@gmail.com>
On 3/8/26 11:07 AM, Evan Ducas wrote:
> Fix several spelling and grammar mistakes in the IPE
> documentation.
>
> No functional change.
>
> Signed-off-by: Evan Ducas <evan.j.ducas@gmail.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Thanks.
> ---
> Documentation/security/ipe.rst | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/security/ipe.rst b/Documentation/security/ipe.rst
> index 4a7d953abcdc..5eb3e6265fbd 100644
> --- a/Documentation/security/ipe.rst
> +++ b/Documentation/security/ipe.rst
> @@ -18,7 +18,7 @@ strong integrity guarantees over both the executable code, and specific
> *data files* on the system, that were critical to its function. These
> specific data files would not be readable unless they passed integrity
> policy. A mandatory access control system would be present, and
> -as a result, xattrs would have to be protected. This lead to a selection
> +as a result, xattrs would have to be protected. This led to a selection
> of what would provide the integrity claims. At the time, there were two
> main mechanisms considered that could guarantee integrity for the system
> with these requirements:
> @@ -195,7 +195,7 @@ of the policy to apply the minute usermode starts. Generally, that storage
> can be handled in one of three ways:
>
> 1. The policy file(s) live on disk and the kernel loads the policy prior
> - to an code path that would result in an enforcement decision.
> + to a code path that would result in an enforcement decision.
> 2. The policy file(s) are passed by the bootloader to the kernel, who
> parses the policy.
> 3. There is a policy file that is compiled into the kernel that is
> @@ -235,8 +235,8 @@ Updatable, Rebootless Policy
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> As requirements change over time (vulnerabilities are found in previously
> -trusted applications, keys roll, etcetera). Updating a kernel to change the
> -meet those security goals is not always a suitable option, as updates are not
> +trusted applications, keys roll, etcetera), updating a kernel to meet
> +those security goals is not always a suitable option, as updates are not
> always risk-free, and blocking a security update leaves systems vulnerable.
> This means IPE requires a policy that can be completely updated (allowing
> revocations of existing policy) from a source external to the kernel (allowing
> @@ -370,7 +370,7 @@ Simplified Policy:
> Finally, IPE's policy is designed for sysadmins, not kernel developers. Instead
> of covering individual LSM hooks (or syscalls), IPE covers operations. This means
> instead of sysadmins needing to know that the syscalls ``mmap``, ``mprotect``,
> -``execve``, and ``uselib`` must have rules protecting them, they must simple know
> +``execve``, and ``uselib`` must have rules protecting them, they must simply know
> that they want to restrict code execution. This limits the amount of bypasses that
> could occur due to a lack of knowledge of the underlying system; whereas the
> maintainers of IPE, being kernel developers can make the correct choice to determine
--
~Randy
^ permalink raw reply
* [PATCH] FIXUP: cachefiles: change cachefiles_bury_object to use start_renaming_dentry()
From: NeilBrown @ 2026-03-08 20:57 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-stolz-verzichten-2ee626da4503@brauner>
From: NeilBrown <neil@brown.name>
[[This fixup for f242581e611e in vfs/vfs-7.1.directory provides a new
commit description has preserves the error returns and log message, and
importantly calls cachefiles_io_error() in exactly the same
circumstances as the original - thanks]]
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 conditions are checked in start_renaming_dentry() but need to
be re-checked when an error is reported to ensure correct handling.
The check that ->graveyard is still d_can_lookup() is dropped as this
was checked when ->graveyard was assigned, and it cannot be changed.
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20260224222542.3458677-11-neilb@ownmail.net
---
fs/cachefiles/namei.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 3af42ec78411..c464c72a51cb 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -309,7 +309,26 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
rd.flags = 0;
ret = start_renaming_dentry(&rd, 0, rep, &QSTR(nbuffer));
if (ret) {
- cachefiles_io_error(cache, "Cannot lock/lookup in graveyard");
+ /* Some errors aren't fatal */
+ if (ret == -EXDEV)
+ /* double-lock failed */
+ return ret;
+ if (d_unhashed(rep) || rep->d_parent != dir || IS_DEADDIR(d_inode(rep))) {
+ /* the entry was probably culled when we dropped the parent dir
+ * lock */
+ _leave(" = 0 [culled?]");
+ return 0;
+ }
+ if (ret == -EINVAL || ret == -ENOTEMPTY) {
+ cachefiles_io_error(cache, "May not make directory loop");
+ return -EIO;
+ }
+ if (ret == -ENOMEM) {
+ _leave(" = -ENOMEM");
+ return -ENOMEM;
+ }
+
+ cachefiles_io_error(cache, "Lookup error %d", ret);
return -EIO;
}
--
2.50.0.107.gf914562f5916.dirty
^ 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