* [PATCH v2 06/15] selinux: Use simple_start_creating() / simple_done_creating()
From: NeilBrown @ 2026-02-23 1:06 UTC (permalink / raw)
To: Christian Brauner, 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
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260223011210.3853517-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
Instead of explicitly locking the parent and performing a lookup in
selinux, use simple_start_creating(), and then use
simple_done_creating() to unlock.
This extends the region that the directory is locked for, and also
performs a lookup.
The lock extension is of no real consequence.
The lookup uses simple_lookup() and so always succeeds. Thus when
d_make_persistent() is called the dentry will already be hashed.
d_make_persistent() handles this case.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Acked-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: NeilBrown <neil@brown.name>
---
security/selinux/selinuxfs.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 3245cc531555..7d4f90e5b12a 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -1931,15 +1931,16 @@ static const struct inode_operations swapover_dir_inode_operations = {
static struct dentry *sel_make_swapover_dir(struct super_block *sb,
unsigned long *ino)
{
- struct dentry *dentry = d_alloc_name(sb->s_root, ".swapover");
+ struct dentry *dentry;
struct inode *inode;
- if (!dentry)
+ inode = sel_make_inode(sb, S_IFDIR);
+ if (!inode)
return ERR_PTR(-ENOMEM);
- inode = sel_make_inode(sb, S_IFDIR);
- if (!inode) {
- dput(dentry);
+ dentry = simple_start_creating(sb->s_root, ".swapover");
+ if (!dentry) {
+ iput(inode);
return ERR_PTR(-ENOMEM);
}
@@ -1947,11 +1948,9 @@ static struct dentry *sel_make_swapover_dir(struct super_block *sb,
inode->i_ino = ++(*ino);
/* directory inodes start off with i_nlink == 2 (for "." entry) */
inc_nlink(inode);
- inode_lock(sb->s_root->d_inode);
d_make_persistent(dentry, inode);
inc_nlink(sb->s_root->d_inode);
- inode_unlock(sb->s_root->d_inode);
- dput(dentry);
+ simple_done_creating(dentry);
return dentry; // borrowed
}
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH v2 05/15] Apparmor: Use simple_start_creating() / simple_done_creating()
From: NeilBrown @ 2026-02-23 1:06 UTC (permalink / raw)
To: Christian Brauner, 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
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260223011210.3853517-1-neilb@ownmail.net>
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>
---
security/apparmor/apparmorfs.c | 35 ++++++++--------------------------
1 file changed, 8 insertions(+), 27 deletions(-)
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 2f84bd23edb6..f93c4f31d02a 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -282,32 +282,20 @@ static struct dentry *aafs_create(const char *name, umode_t mode,
dir = d_inode(parent);
- inode_lock(dir);
- dentry = lookup_noperm(&QSTR(name), parent);
+ dentry = simple_start_creating(parent, name);
if (IS_ERR(dentry)) {
error = PTR_ERR(dentry);
- goto fail_lock;
- }
-
- if (d_really_is_positive(dentry)) {
- error = -EEXIST;
- goto fail_dentry;
+ goto fail;
}
error = __aafs_setup_d_inode(dir, dentry, mode, data, link, fops, iops);
+ simple_done_creating(dentry);
if (error)
- goto fail_dentry;
- inode_unlock(dir);
-
+ goto fail;
return dentry;
-fail_dentry:
- dput(dentry);
-
-fail_lock:
- inode_unlock(dir);
+fail:
simple_release_fs(&aafs_mnt, &aafs_count);
-
return ERR_PTR(error);
}
@@ -2585,8 +2573,7 @@ static int aa_mk_null_file(struct dentry *parent)
if (error)
return error;
- inode_lock(d_inode(parent));
- dentry = lookup_noperm(&QSTR(NULL_FILE_NAME), parent);
+ dentry = simple_start_creating(parent, NULL_FILE_NAME);
if (IS_ERR(dentry)) {
error = PTR_ERR(dentry);
goto out;
@@ -2594,7 +2581,7 @@ static int aa_mk_null_file(struct dentry *parent)
inode = new_inode(parent->d_inode->i_sb);
if (!inode) {
error = -ENOMEM;
- goto out1;
+ goto out;
}
inode->i_ino = get_next_ino();
@@ -2606,18 +2593,12 @@ static int aa_mk_null_file(struct dentry *parent)
aa_null.dentry = dget(dentry);
aa_null.mnt = mntget(mount);
- error = 0;
-
-out1:
- dput(dentry);
out:
- inode_unlock(d_inode(parent));
+ simple_done_creating(dentry);
simple_release_fs(&mount, &count);
return error;
}
-
-
static const char *policy_get_link(struct dentry *dentry,
struct inode *inode,
struct delayed_call *done)
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH v2 04/15] libfs: change simple_done_creating() to use end_creating()
From: NeilBrown @ 2026-02-23 1:06 UTC (permalink / raw)
To: Christian Brauner, 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
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260223011210.3853517-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
simple_done_creating() and end_creating() are identical.
So change the former to use the latter. This further centralises
unlocking of directories.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/libfs.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/libfs.c b/fs/libfs.c
index 74134ba2e8d1..63b4fb082435 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -2318,7 +2318,6 @@ EXPORT_SYMBOL(simple_start_creating);
/* parent must have been held exclusive since simple_start_creating() */
void simple_done_creating(struct dentry *child)
{
- inode_unlock(child->d_parent->d_inode);
- dput(child);
+ end_creating(child);
}
EXPORT_SYMBOL(simple_done_creating);
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH v2 03/15] VFS: move the start_dirop() kerndoc comment to before start_dirop()
From: NeilBrown @ 2026-02-23 1:06 UTC (permalink / raw)
To: Christian Brauner, 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
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260223011210.3853517-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
This kerneldoc comment was always meant for start_dirop(), not for
__start_dirop() which is a static function and doesn't need
documentation.
It was in the wrong place and was then incorrectly renamed (instead of
moved) and useless "documentation" was added for "@state" was provided.
This patch reverts the name, removes the mention of @state, and moves
the comment to where it belongs.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/namei.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index e4ac07a4090e..d80b81a1f06a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2893,20 +2893,6 @@ static int filename_parentat(int dfd, struct filename *name,
return __filename_parentat(dfd, name, flags, parent, last, type, NULL);
}
-/**
- * __start_dirop - begin a create or remove dirop, performing locking and lookup
- * @parent: the dentry of the parent in which the operation will occur
- * @name: a qstr holding the name within that parent
- * @lookup_flags: intent and other lookup flags.
- * @state: task state bitmask
- *
- * The lookup is performed and necessary locks are taken so that, on success,
- * the returned dentry can be operated on safely.
- * The qstr must already have the hash value calculated.
- *
- * Returns: a locked dentry, or an error.
- *
- */
static struct dentry *__start_dirop(struct dentry *parent, struct qstr *name,
unsigned int lookup_flags,
unsigned int state)
@@ -2928,6 +2914,19 @@ static struct dentry *__start_dirop(struct dentry *parent, struct qstr *name,
return dentry;
}
+/**
+ * start_dirop - begin a create or remove dirop, performing locking and lookup
+ * @parent: the dentry of the parent in which the operation will occur
+ * @name: a qstr holding the name within that parent
+ * @lookup_flags: intent and other lookup flags.
+ *
+ * The lookup is performed and necessary locks are taken so that, on success,
+ * the returned dentry can be operated on safely.
+ * The qstr must already have the hash value calculated.
+ *
+ * Returns: a locked dentry, or an error.
+ *
+ */
struct dentry *start_dirop(struct dentry *parent, struct qstr *name,
unsigned int lookup_flags)
{
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH v2 02/15] fs/proc: Don't lock root inode when creating "self" and "thread-self"
From: NeilBrown @ 2026-02-23 1:06 UTC (permalink / raw)
To: Christian Brauner, 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
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260223011210.3853517-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
proc_setup_self() and proc_setup_thread_self() are only called from
proc_fill_super() which is before the filesystem is "live". So there is
no need to lock the root directory when adding "self" and "thread-self".
This is clear from simple_fill_super() which provides similar
functionality for other filesystems and does not lock anything.
The locking is not harmful, except that it may be confusing to a reader.
As part of an effort to centralise all locking for directories for
name-based operations (prior to changing some locking rules), it is
simplest to remove the locking here.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/proc/self.c | 3 ---
fs/proc/thread_self.c | 3 ---
2 files changed, 6 deletions(-)
diff --git a/fs/proc/self.c b/fs/proc/self.c
index 62d2c0cfe35c..56adf1c68f7a 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -35,11 +35,9 @@ unsigned self_inum __ro_after_init;
int proc_setup_self(struct super_block *s)
{
- struct inode *root_inode = d_inode(s->s_root);
struct dentry *self;
int ret = -ENOMEM;
- inode_lock(root_inode);
self = d_alloc_name(s->s_root, "self");
if (self) {
struct inode *inode = new_inode(s);
@@ -55,7 +53,6 @@ int proc_setup_self(struct super_block *s)
}
dput(self);
}
- inode_unlock(root_inode);
if (ret)
pr_err("proc_fill_super: can't allocate /proc/self\n");
diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index d6113dbe58e0..61ac62c3fd9f 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -35,11 +35,9 @@ unsigned thread_self_inum __ro_after_init;
int proc_setup_thread_self(struct super_block *s)
{
- struct inode *root_inode = d_inode(s->s_root);
struct dentry *thread_self;
int ret = -ENOMEM;
- inode_lock(root_inode);
thread_self = d_alloc_name(s->s_root, "thread-self");
if (thread_self) {
struct inode *inode = new_inode(s);
@@ -55,7 +53,6 @@ int proc_setup_thread_self(struct super_block *s)
}
dput(thread_self);
}
- inode_unlock(root_inode);
if (ret)
pr_err("proc_fill_super: can't allocate /proc/thread-self\n");
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH v2 01/15] VFS: note error returns is documentation for various lookup functions
From: NeilBrown @ 2026-02-23 1:06 UTC (permalink / raw)
To: Christian Brauner, 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
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260223011210.3853517-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
Darrick recently noted that try_lookup_noperm() is documented as
"Look up a dentry by name in the dcache, returning NULL if it does not
currently exist." but it can in fact return an error.
So update the documentation for that and related function.
Link: https://lore.kernel.org/all/20260218234917.GA6490@frogsfrogsfrogs/
Cc: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/namei.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/fs/namei.c b/fs/namei.c
index 58f715f7657e..e4ac07a4090e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3124,7 +3124,8 @@ static int lookup_one_common(struct mnt_idmap *idmap,
* @base: base directory to lookup from
*
* Look up a dentry by name in the dcache, returning NULL if it does not
- * currently exist. The function does not try to create a dentry and if one
+ * currently exist or an error is there is a problem with the name.
+ * The function does not try to create a dentry and if one
* is found it doesn't try to revalidate it.
*
* Note that this routine is purely a helper for filesystem usage and should
@@ -3132,6 +3133,11 @@ static int lookup_one_common(struct mnt_idmap *idmap,
*
* No locks need be held - only a counted reference to @base is needed.
*
+ * Returns:
+ * - ref-counted dentry on success, or
+ * - %NULL if name could not be found, or
+ * - ERR_PTR(-EACCES) if name is dot or dotdot or contains a slash or nul, or
+ * - ERR_PTR() if fs provide ->d_hash, and this returned an error.
*/
struct dentry *try_lookup_noperm(struct qstr *name, struct dentry *base)
{
@@ -3208,6 +3214,11 @@ EXPORT_SYMBOL(lookup_one);
*
* Unlike lookup_one, it should be called without the parent
* i_rwsem held, and will take the i_rwsem itself if necessary.
+ *
+ * Returns:= A dentry, possibly negative, or
+ * - same errors as try_lookup_noperm() or
+ * - ERR_PTR(-ENOENT) if parent has been removed, or
+ * - ERR_PTR(-EACCES) if parent directory is not searchable.
*/
struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap, struct qstr *name,
struct dentry *base)
@@ -3244,6 +3255,10 @@ EXPORT_SYMBOL(lookup_one_unlocked);
* It should be called without the parent i_rwsem held, and will take
* the i_rwsem itself if necessary. If a fatal signal is pending or
* delivered, it will return %-EINTR if the lock is needed.
+ *
+ * Returns: A dentry, possibly negative, or
+ * - same errors as lookup_one_unlocked() or
+ * - ERR_PTR(-EINTR) is a fatal signal is pending.
*/
struct dentry *lookup_one_positive_killable(struct mnt_idmap *idmap,
struct qstr *name,
@@ -3283,6 +3298,10 @@ EXPORT_SYMBOL(lookup_one_positive_killable);
* This can be used for in-kernel filesystem clients such as file servers.
*
* The helper should be called without i_rwsem held.
+ *
+ * Returns: A positive dentry, or
+ * - ERR_PTR(-ENOENT) if the name could not be found, or
+ * - same errors as lookup_one_unlocked().
*/
struct dentry *lookup_one_positive_unlocked(struct mnt_idmap *idmap,
struct qstr *name,
@@ -3311,6 +3330,10 @@ EXPORT_SYMBOL(lookup_one_positive_unlocked);
*
* Unlike try_lookup_noperm() it *does* revalidate the dentry if it already
* existed.
+ *
+ * Returns: A dentry, possibly negative, or
+ * - ERR_PTR(-ENOENT) if parent has been removed, or
+ * - same errors as try_lookup_noperm()
*/
struct dentry *lookup_noperm_unlocked(struct qstr *name, struct dentry *base)
{
@@ -3335,6 +3358,10 @@ EXPORT_SYMBOL(lookup_noperm_unlocked);
* _can_ become positive at any time, so callers of lookup_noperm_unlocked()
* need to be very careful; pinned positives have ->d_inode stable, so
* this one avoids such problems.
+ *
+ * Returns: A positive dentry, or
+ * - ERR_PTR(-ENOENT) if name cannot be found or parent has been removed, or
+ * - same errors as try_lookup_noperm()
*/
struct dentry *lookup_noperm_positive_unlocked(struct qstr *name,
struct dentry *base)
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH v2 00/15] Further centralising of directory locking for name ops.
From: NeilBrown @ 2026-02-23 1:06 UTC (permalink / raw)
To: Christian Brauner, 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
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
This is v2 of a series I sent shortly before the merge-window opened,
and now that it has closed ....
I have added:
01/15 to improve documentation as suggested by Darrick
11/15 as discussed with Amir to simplify the following patch
various RB and AB (thanks Jeff in particular)
I'm hoping this could land in vfs/ shortly (tihs month?). I will then have another
series of patches which make a small start in changing the locking
rules, which hopefully can also land in the next merge window.
Original patch description below.
Thanks,
NeilBrown
I am working towards changing the locking rules for name-operations: locking
the name rather than the whole directory.
The current part of this process is centralising all the locking so that
it can be changed in one place.
Recently "start_creating", "start_removing", "start_renaming" and related
interaces were added which combine the locking and the lookup. At that time
many callers were changed to use the new interfaces. However there are still
an assortment of places out side of fs/namei.c where the directory is locked
explictly, whether with inode_lock() or lock_rename() or similar. These were
missed in the first pass for an assortment of uninteresting reasons.
This series addresses the remaining places where explicit locking is
used, and changes them to use the new interfaces, or otherwise removes
the explicit locking.
The biggest changes are in overlayfs. The other changes are quite
simple, though maybe the cachefiles changes is the least simple of those.
I'm running the --overlay tests in xfstests and nothing has popped yet.
I'll continue with this and run some NFS tests too.
Thanks for your review of these patches!
NeilBrown
[PATCH v2 01/15] VFS: note error returns is documentation for various
[PATCH v2 02/15] fs/proc: Don't lock root inode when creating "self"
[PATCH v2 03/15] VFS: move the start_dirop() kerndoc comment to
[PATCH v2 04/15] libfs: change simple_done_creating() to use
[PATCH v2 05/15] Apparmor: Use simple_start_creating() /
[PATCH v2 06/15] selinux: Use simple_start_creating() /
[PATCH v2 07/15] nfsd: switch purge_old() to use
[PATCH v2 08/15] VFS: make lookup_one_qstr_excl() static.
[PATCH v2 09/15] ovl: Simplify ovl_lookup_real_one()
[PATCH v2 10/15] cachefiles: change cachefiles_bury_object to use
[PATCH v2 11/15] ovl: pass name buffer to ovl_start_creating_temp()
[PATCH v2 12/15] ovl: change ovl_create_real() to get a new lock when
[PATCH v2 13/15] ovl: use is_subdir() for testing if one thing is a
[PATCH v2 14/15] ovl: remove ovl_lock_rename_workdir()
[PATCH v2 15/15] VFS: unexport lock_rename(), lock_rename_child(),
^ permalink raw reply
* Re: [PATCH 05/13] selinux: Use simple_start_creating() / simple_done_creating()
From: NeilBrown @ 2026-02-23 0:58 UTC (permalink / raw)
To: Paul Moore
Cc: Christian Brauner, Alexander Viro, David Howells, Jan Kara,
Chuck Lever, Jeff Layton, Miklos Szeredi, Amir Goldstein,
John Johansen, James Morris, Serge E. Hallyn, Stephen Smalley,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <CAHC9VhTv+K44q7+5d17jS8h9fJY_JfQVUw5NPNvPzjkHDpqp=g@mail.gmail.com>
On Mon, 23 Feb 2026, Paul Moore wrote:
> On Sat, Feb 21, 2026 at 5:28 PM NeilBrown <neilb@ownmail.net> wrote:
> > On Sat, 21 Feb 2026, Paul Moore wrote:
> > > On Wed, Feb 4, 2026 at 12:08 AM NeilBrown <neilb@ownmail.net> wrote:
> > > >
> > > > From: NeilBrown <neil@brown.name>
> > > >
> > > > Instead of explicitly locking the parent and performing a lookup in
> > > > selinux, use simple_start_creating(), and then use
> > > > simple_done_creating() to unlock.
> > > >
> > > > This extends the region that the directory is locked for, and also
> > > > performs a lookup.
> > > > The lock extension is of no real consequence.
> > > > The lookup uses simple_lookup() and so always succeeds. Thus when
> > > > d_make_persistent() is called the dentry will already be hashed.
> > > > d_make_persistent() handles this case.
> > > >
> > > > Signed-off-by: NeilBrown <neil@brown.name>
> > > > ---
> > > > security/selinux/selinuxfs.c | 15 +++++++--------
> > > > 1 file changed, 7 insertions(+), 8 deletions(-)
> > >
> > > Unless I'm missing something, there is no reason why I couldn't take
> > > just this patch into the SELinux tree once the merge window closes,
> > > yes?
> >
> > Yes - but ...
> >
> > Once this series lands (hopefully soon - I will resend after -rc1 is
> > out) I have another batch which depends on the new start_creating etc
> > API being used everywhere ...
>
> Okay, thanks for letting me know. I was curious about something like
> that based on the cover letter, but the timing wasn't clear.
>
> Acked-by: Paul Moore <paul@paul-moore.com>
Thank!
NeilBrown
^ permalink raw reply
* [PATCH] apparmor: Use sysfs_emit in param_get_{audit,mode}
From: Thorsten Blum @ 2026-02-22 21:40 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel
Replace sprintf() with sysfs_emit() in param_get_audit() and
param_get_mode(). sysfs_emit() is preferred for formatting sysfs output
because it provides safer bounds checking. Add terminating newlines as
suggested by checkpatch.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/lsm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index a87cd60ed206..1250192f7b12 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -17,6 +17,7 @@
#include <linux/ptrace.h>
#include <linux/ctype.h>
#include <linux/sysctl.h>
+#include <linux/sysfs.h>
#include <linux/audit.h>
#include <linux/user_namespace.h>
#include <linux/netfilter_ipv4.h>
@@ -2081,7 +2082,7 @@ static int param_get_audit(char *buffer, const struct kernel_param *kp)
return -EINVAL;
if (apparmor_initialized && !aa_current_policy_view_capable(NULL))
return -EPERM;
- return sprintf(buffer, "%s", audit_mode_names[aa_g_audit]);
+ return sysfs_emit(buffer, "%s\n", audit_mode_names[aa_g_audit]);
}
static int param_set_audit(const char *val, const struct kernel_param *kp)
@@ -2109,8 +2110,7 @@ static int param_get_mode(char *buffer, const struct kernel_param *kp)
return -EINVAL;
if (apparmor_initialized && !aa_current_policy_view_capable(NULL))
return -EPERM;
-
- return sprintf(buffer, "%s", aa_profile_mode_names[aa_g_profile_mode]);
+ return sysfs_emit(buffer, "%s\n", aa_profile_mode_names[aa_g_profile_mode]);
}
static int param_set_mode(const char *val, const struct kernel_param *kp)
--
Thorsten Blum <thorsten.blum@linux.dev>
GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
^ permalink raw reply related
* Re: [PATCH RESEND] apparmor: Replace memcpy + NUL termination with kmemdup_nul in do_setattr
From: Serge E. Hallyn @ 2026-02-22 20:53 UTC (permalink / raw)
To: Thorsten Blum
Cc: John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
apparmor, linux-security-module, linux-kernel
In-Reply-To: <20260222204645.285727-1-thorsten.blum@linux.dev>
On Sun, Feb 22, 2026 at 09:46:44PM +0100, Thorsten Blum wrote:
> Use kmemdup_nul() to copy 'value' instead of using memcpy() followed by
> a manual NUL termination. No functional changes.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Looks correct, thanks.
Reviewed-by: Serge Hallyn <serge@hallyn.com>
> ---
> security/apparmor/lsm.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index c1d42fc72fdb..49aa6ad68838 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -858,12 +858,9 @@ static int do_setattr(u64 attr, void *value, size_t size)
>
> /* AppArmor requires that the buffer must be null terminated atm */
> if (args[size - 1] != '\0') {
> - /* null terminate */
> - largs = args = kmalloc(size + 1, GFP_KERNEL);
> + largs = args = kmemdup_nul(value, size, GFP_KERNEL);
> if (!args)
> return -ENOMEM;
> - memcpy(args, value, size);
> - args[size] = '\0';
> }
>
> error = -EINVAL;
> --
> Thorsten Blum <thorsten.blum@linux.dev>
> GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
^ permalink raw reply
* [PATCH RESEND] apparmor: Replace memcpy + NUL termination with kmemdup_nul in do_setattr
From: Thorsten Blum @ 2026-02-22 20:46 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel
Use kmemdup_nul() to copy 'value' instead of using memcpy() followed by
a manual NUL termination. No functional changes.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/lsm.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index c1d42fc72fdb..49aa6ad68838 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -858,12 +858,9 @@ static int do_setattr(u64 attr, void *value, size_t size)
/* AppArmor requires that the buffer must be null terminated atm */
if (args[size - 1] != '\0') {
- /* null terminate */
- largs = args = kmalloc(size + 1, GFP_KERNEL);
+ largs = args = kmemdup_nul(value, size, GFP_KERNEL);
if (!args)
return -ENOMEM;
- memcpy(args, value, size);
- args[size] = '\0';
}
error = -EINVAL;
--
Thorsten Blum <thorsten.blum@linux.dev>
GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
^ permalink raw reply related
* Re: [PATCH] ucount: check for CAP_SYS_RESOURCE using ns_capable_noaudit()
From: Paul Moore @ 2026-02-22 20:44 UTC (permalink / raw)
To: Andrew Morton
Cc: Ondrej Mosnacek, Serge E. Hallyn, Eric W . Biederman,
linux-kernel, linux-security-module, selinux
In-Reply-To: <20260222115824.37ae0de1eb183997a23669ae@linux-foundation.org>
On Sun, Feb 22, 2026 at 2:58 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 27 Jan 2026 17:09:40 -0500 Paul Moore <paul@paul-moore.com> wrote:
>
> > > > > > Fixes: dbec28460a89 ("userns: Add per user namespace sysctls.")
> > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > > > > ---
> > > > > > kernel/ucount.c | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > Reviewed-by: Paul Moore <paul@paul-moore.com>
> > > >
> > > > Acked-by: Serge Hallyn <serge@hallyn.com>
> > > >
> > > > Looks good to me. What tree should this go through? Network?
> > >
> > > Andrew has already applied the two patches I posted into his
> > > mm-nonmm-unstable branch, so I assume they are set to go through his
> > > tree.
> >
> > Andrew, any chance we can get a reply to these threads when you merge
> > a patch into your tree?
>
> I already spray out soooo much email.
>
> A better approach would be for me to become more chatty. Reply
> "thanks, applied <reasons> <caveats> <etc>". Shall do this.
Appreciated, thank you.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] ucount: check for CAP_SYS_RESOURCE using ns_capable_noaudit()
From: Andrew Morton @ 2026-02-22 19:58 UTC (permalink / raw)
To: Paul Moore
Cc: Ondrej Mosnacek, Serge E. Hallyn, Eric W . Biederman,
linux-kernel, linux-security-module, selinux
In-Reply-To: <CAHC9VhQOW4BQR+UCvrcXa_yiSqADN0HC2Xp0pMKs3FtnjcVaOQ@mail.gmail.com>
On Tue, 27 Jan 2026 17:09:40 -0500 Paul Moore <paul@paul-moore.com> wrote:
> > > > > Fixes: dbec28460a89 ("userns: Add per user namespace sysctls.")
> > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > > > ---
> > > > > kernel/ucount.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > Reviewed-by: Paul Moore <paul@paul-moore.com>
> > >
> > > Acked-by: Serge Hallyn <serge@hallyn.com>
> > >
> > > Looks good to me. What tree should this go through? Network?
> >
> > Andrew has already applied the two patches I posted into his
> > mm-nonmm-unstable branch, so I assume they are set to go through his
> > tree.
>
> Andrew, any chance we can get a reply to these threads when you merge
> a patch into your tree?
I already spray out soooo much email.
A better approach would be for me to become more chatty. Reply
"thanks, applied <reasons> <caveats> <etc>". Shall do this.
^ permalink raw reply
* Re: [RFC] Landlock: mutable domains (and supervisor notification uAPI options)
From: Tingmao Wang @ 2026-02-22 18:04 UTC (permalink / raw)
To: Justin Suess, Günther Noack, Mickaël Salaün
Cc: amir73il, jack, jannh, linux-security-module, penguin-kernel,
song
In-Reply-To: <20260216212753.4159224-1-utilityemal77@gmail.com>
On 2/16/26 21:27, Justin Suess wrote:
> On Sun, Feb 15, 2026 at 02:54:08AM +0000, Tingmao Wang wrote:
>> [...]
>> The next stage is to introduce "mutable domains". The motivation for this
>> is two fold:
>>
>> 1. This allows the supervisor to allow access to (large) file hierarchies
>> without needing to be woken up again for each access.
>> 2. Because we cannot block within security_path_mknod and other
>> directory-modification related hooks [6], the proposal was to return
>> immediately from those hooks after queuing the supervisor notification,
>> then wait in a separate task_work. This however means that we cannot
>> directly "allow" access (and even if we can, it may introduce TOCTOU
>> problems). In order to allow access to requested files, the supervisor
>> has to add additional rules to the (now mutable) domain which will
>> allow the required access.
>
> Is blocking during connect(2) allowed either if the socket is non-blocking?
>
> This may be another example case that needs to be handled differently than calls
> we can block in safely.
I think the non-blocking socket case is worth considering but is
orthogonal to the above discussion - even when the socket is marked
non-blocking, we can still "safely" block in the connect hook (as long as
there isn't another reason, like locks, that prevents us from doing so),
because it only affects the calling process. Although if we're doing the
-ERESTARTNOINTR approach for fs, we might as well do the same here and
return -ERESTARTNOINTR from the connect hook (but that would still "block"
the syscall). Also, even for the file create/delete case, while we don't
block in the hook, from the user-space perspective, we're still blocking
(i.e. in the task_work), since the syscall doesn't immediately return (but
instead waits for a response from the supervisor).
However we could consider implementing some special case for
non-blocking-capable operations (like socket connect) such that we return
from the syscall without waiting for a supervisor response (and we return
a value that a "try again later" response would normally return), but then
we have to figure out how to cause a "you can retry now" notification that
would normally be sent for the non-blocking operation.
Related, there is the question of how we want to handle "non-blocking
open()s". Technically there isn't a non-blocking open() for regular files
from the user-space perspective, but ideally, for example, a wait on a
Landlock supervisor would not prevent an io_uring_enter() processing
multiple openat()s from continuing on to other requests. But I suspect
this would be very hard to implement.
> [...]
>> (Disallowing) self-supervision
>> ------------------------------
>>
>> We should figure out a way to ensure that a process cannot call
>> landlock_restrict_self() with a ruleset that has a supervisor for which it
>> has access to (i.e. via a supervisor ruleset fd). This prevents
>> accidental misuse, and also prevents deadlocks as discussed in [11]. I'm
>> not sure if this will be easy to implement, however.
>
> This seems like a graph acyclicity problem.
>
> Here are a couple cases to consider:
>
> 1. LANDLOCK_RESTRICT_SELF_TSYNC misuse:
>
> In the case where a user wants to use this supervisor to supervise other
> threads within the same process, a user could naively call
> LANDLOCK_RESTRICT_SELF_TSYNC (merged into 7.0) when enforcing the
> supervisee_fd. This would enforce the same policy on the thread running
> the supervisor and the supervisee.
Yes, but if the intention is to supervise threads within the same process
(which IMO is already a slightly questionable use case, maybe it can be
used as a tracing method, but certainly not for security), calling
restrict with LANDLOCK_RESTRICT_SELF_TSYNC is itself a mistake. There is
a case for us to try to detect and prevent this, but I'm thinking it would
more be to prevent "useless sandboxing" rather than to prevent deadlocks
(after all, it might not deadlock if someone else also has a fd for that
supervisor).
>
> 2. Transfer of the supervisee_fd (SCM_RIGHTS)
>
> It's possible to transfer file descriptors over unix domain sockets. If
> we had a supervisor daemon that used this form of IPC to send precooked
> supervisee_fds to other threads, and one of those ended up in a parent
> process of the supervisor, we could inadvertently end up with problems.
Note that being a parent process of the supervisor itself is not a
problem, what matters here is the Landlock domain relationships. It will
only be a problem if the fd is then used to make the supervisor be
supervised by itself (but if it's an unrelated parent process that is
supervised, it should still be fine).
>
> 3. Blocking in other LSMs (pointed out in your source [11])
>
> The hardest case to deal with, other LSMs like TOMOYO can also block and
> cause dependency cycles.
>
> ---
>
> This gets tricky, and I don't know if just checking parent / child
> relationships would work. Because the supervisor and supervisee rulesets
> are just file descriptors, and there are potentially unlimited number of
> ways these FDs could be transfered or instantiated.
>
> I think the best way to deal with this is constraining the problem space:
>
> An idea (binding supervisors/supervisees to domains on first use)
>
> Whenever landlock_restrict_self(supervisee_fd,...) is called, check the
> current domain credentials and verify that the domain is a *proper
> subset* of the supervisors domain. Then permanently close the
> supervisee_fd and never allow reenforcement. Similarly, once a
> supervisor_fd is created, never allow commiting from a context with
> "current landlock domain != original landlock domain at creation"
>
> This prevents post-enforcement usage of the supervisee_fd by a parent
> domain, and post-commit usage of a supervisee_fd by any subdomain.
It's not the supervisee fd that matters right? For example, a supervisor
can still call landlock_restrict_self() with the supervisee_fd it acquired
from the ioctl() (after all, this call itself is fine, it's only the fact
that there is no other process holding the supervisor fd that means this
gets us into a deadlock situation). I would think that if we
automatically close any fds, it would be the supervisor fd that we would
close. In this particular case this then means that there are no longer
any process holding the supervisor fd, which is a case we can detect, and
just deny any access request by the supervisee from that point forward
(preventing deadlock).
>
> I'm not sure if it's possible to check whether one domain is a
> proper subset of another (ie supervisor domain includes but *doesn't equal*
> supervisee domain), but I think that's one way do do it.
>
> This idea would help, but doesn't address case 3 above.
>
>>
>> [11]: https://lore.kernel.org/all/cc3e131f-f9a3-417b-9267-907b45083dc3@maowtm.org/
>>
>>
>> Supervisor notification
>> -----------------------
>>
>> The above RFC only covers mutable domains. The natural next stage of this
>> work is to send notification to the supervisor on access denials, so that
>> it can decide whether to allow the access or not. For that, there are
>> also lots of questions at this stage:
>>
>>
>> - Should we in fact implement that first, before mutable domains? This
>> means that the supervisor would only be able to find out about denials,
>> but not allow them without a sandbox restart. We still eventually want
>> the mutable domains, since that makes this a lot more useful, but I can
>> see some use cases for just the notification part (e.g. island denial
>> log), and I can't see a likely use case for just mutable domains, aside
>> from live reload of landlock-config (maybe that _is_ useful on its own,
>> considering that you can also find out about denials from the kernel
>> audit log, and add missing rules based on that).
>>
>>
>> - Earlier when implementing the Landlock supervise v1 RFC, I basically
>> came up with an ad-hoc uAPI for the notification [12], and the PoC code
>> linked to above also uses this uAPI. There are of course many problems
>> with this as it stands, e.g. it only having one destname, which means
>> that for rename, the fd1 needs to be the child being moved, which does
>> not align with the vfs semantic and how Landlock treat it (i.e. the
>> thing being updated here is the parent directory, not the child itself).
>> Same for delete, which currently sends the child as fd1.
>>
>> But also, in discussion with Mickaël last year, he mentioned that we
>> could reuse the fsnotify infrastructure, and perhaps additionally, use
>> fanotify to deliver these notifications. I do think there is some
>> potential here, as fanotify already implements an event header, a
>> mechanism for receiving and replying to events, etc. We could possibly
>> extend it to send Landlock specific notifications via a new kind of mark
>> (FAN_MARK_LANDLOCK_DOMAIN ??) and add one or more new corresponding
>> event types. Mickaël mentioned mount notifications [13] as an example
>> of using fanotify to send notifications other than file/dir
>> modifications.
>>
>> I'm not sure if directly extending the fanotify uAPI is a good idea tho,
>> considering that Landlock is not a feature specific to the filesystem -
>> we will also have denial events for net_port rules, and perhaps more in
>> the future. However, Mickaël mentioned that there might be some
>> internal infrastructure which we can re-use (even if we have our own
>> notification uAPI).
> I think that a new FAN_MARK would be required to use fanotify uAPI.
>
> There are a couple questions I have with this: (if we extend fanotify)
>
> 1. What FAN_CLASS_* would notifications use?
>
> FAN_CLASS_* specifies the type of notification, when the notification is
> triggered.
>
> See [1] for the current classes.
>
> If we want interactive, pre-access blocking, that would correspond to
> FAN_CLASS_PRE_CONTENT or FAN_CLASS_CONTENT. Both of which currently
> require CAP_SYS_ADMIN regardless of FAN_MARK. Which requiring that
> would require that supervisors have CAP_SYS_ADMIN, if the current
> CAP_SYS_ADMIN requirements remain in place.
>
> (If we don't have interactive blocking denials, we could just use
> FAN_CLASS_NOTIF)
I'm not sure, using fanotify is just a vague idea at this point and I
don't have any concrete design to offer, and this is indeed a good point -
we definitely don't want to require CAP_SYS_ADMIN (since being
unprivileged is one advantage of Landlock)
>
> 2. How would fanotify events be encoded?
>
> Events in fanotify use this structure for event data (one or more of the
> following must be recieved in a notification) [2]
>
> struct fanotify_event_metadata {
> __u32 event_len;
> __u8 vers;
> __u8 reserved;
> __u16 metadata_len;
> __aligned_u64 mask;
> __s32 fd;
> __s32 pid;
> };
>
> There are access classes landlock restricts that might not have an fd at
> all, like abstract unix sockets, tcp ports, signals etc.
This was also one of my concerns, I guess we might still need a cookie to
identify such events. This is one of the reasons why I think extending
seccomp-unotify makes slightly more sense (if we have to choose one of
these two, but of course we can still create a standalone uAPI).
>
> Good news is fanotify supports multiple types of additional information
> records, and we could potentially extend fanotify to support new ones as
> you alluded to.
>
> For examples of this, see struct fanotify_event_info_mnt,
> fanotify_event_info_pidfd.
>
> These records get attached to the event so they could be used to pass
> landlock access data.
>
> 3. If we support interactive permission decisions (even for a
> subset of landlock access rights only), do we use the response code?
> (question might be moot if we don't do blocking/responses at all)
Regardless of uAPI choice, I would really like to support blocking (even
though it makes this whole thing much more difficult).
>
> From [2]:
>
> For permission events, the application must write(2) a structure
> of the following form to the fanotify file descriptor:
>
> struct fanotify_response {
> __s32 fd;
> __u32 response;
> };
>
>
> response is a FAN_ALLOW or FAN_DENY. This is used by fanotify as a
> one-time access decision. Would this be used to do one-off exceptions to
> policy, or would we require policy decisions to go through the
> supervisor_fd and ignore the response code?
I don't think we would be able to do one-off allow decisions (but we
should still be able to do deny) regardless of the uAPI we use, due to the
problem with inode locks.
>
> 4. How would we reconcile the disparity between fanotify access rights
> and landlock access rights?
>
> There's no clean 1:1 mapping between fanotify access rights and landlock
> access rights as Mickaël pointed out. [2] [3]
>
> Many fs rights (creation, deletion, rename, linking) are not handled or
> implemented, (not even considering network/unix/signal scoping), so we'd
> be adding all these landlock specific rights.
>
> We could make a "catch-all" FAN_LANDLOCK_ACCESS or similar and ignore
> all the existing rights, and put the actual access data in the event
> record. It's awkward either way.
> ---
>
> In conclusion, I think extending fanotify is more viable than seccomp,
> from a purely technical standpoint. because it seems extensible,
> and because it runs post-lsm hooks.
>
> That being said, it's awkward, requires large extensions to the API, and
> definition of permissions that are specific to landlock.
>
> Whether or not landlock makes sense in fanotify from a semantic point of
> view is an entirely different question. There's no precedent for
> non-filesystem access controls in fanotify, so it's a little... out-of-place
> for an LSM to expose features on a filesystem access notification api?
>
> Curious on what people think.
>
> [1]: https://man7.org/linux/man-pages/man2/fanotify_init.2.html
> [2]: https://man7.org/linux/man-pages/man7/fanotify.7.html
> [3]: https://lore.kernel.org/all/20250304.Choo7foe2eoj@digikod.net/
>>
>>
>> - The other uAPI alternative which I have been thinking of is to extend
>> seccomp-unotify. For example, a Landlock denial could result in the
>> syscall being trapped and a `struct seccomp_notif` being sent to the
>> seccomp supervisor (via the existing mechanism), with additional
>> information (mostly, the file(s) / net ports being accessed and access
>> rights requested) attached to the notification _somehow_. Then the
>> supervisor can use the same kind of responses one would use for
>> seccomp-unotify to cause the syscall to either be retried (possibly via
>> `SECCOMP_USER_NOTIF_FLAG_CONTINUE`) or return with an error code of its
>> choice (or alternatively, carry out the operation on behalf of the
>> child, and pretend that the syscall succeed, which might be useful to
>> implement an "allow file creation but only this file" / "allow `mktemp
>> -d` but not arbitrary create on anything under /tmp").
>>
>> Looking at `struct seccomp_notif` and `struct seccomp_data` however, I'm
>> not sure how feasible / doable this extension would be. Also,
>> seccomp-unotify is supposed to trigger before a syscall is actually
>> executed, whereas if we use it this way, we will want it to trigger
>> after we're already midway through the syscall (in the LSM hook). This
>> might make it hard to implement (and also twists a bit the uAPI
>> semantics of seccomp-unotify).
>>
>
> (Some of the stuff discussed with seccomp below is derived from a side
> conversation with Tingmao over this proposal)
>
> There are some problems with extending seccomp unotify. Passing the
> full context needed through this api to the supervisor is problematic.
> seccomp unotify notifications look like this [4]:
>
> struct seccomp_notif {
> __u64 id; /* Cookie */
> __u32 pid; /* TID of target thread */
> __u32 flags; /* Currently unused (0) */
> struct seccomp_data data; /* See seccomp(2) */
> };
>
> And struct seccomp_data [5]:
>
> struct seccomp_data {
> int nr; /* System call number */
> __u32 arch; /* AUDIT_ARCH_* value
> (see <linux/audit.h>) */
> __u64 instruction_pointer; /* CPU instruction pointer */
> __u64 args[6]; /* Up to 6 system call arguments */
> };
>
> Even if we pass the syscall data, for the userspace to actually decode
> the arguments to figure out what the access is doing we have two
> critical problems (1,2) and one annoyance (3):
>
> 1. The syscall itself doesn't necessarily contain the full context of the access.
>
> 2. We cannot decode the pointer-based arguments from userspace for a syscall
> in seccomp without TOCTOU. It also requires reaching into userspace
> memory. [6]
>
> 3. Decoding the syscall number is an arch-specific operation that we now have
> to expect userspace to deal with.
We would probably need to attach additional (potentially variable length,
like file names for mknod / link / rename requests) Landlock-specific
information. The supervisor should not have to do any syscall decoding
(otherwise this partially defeats the point of Landlock supervise - they
can just use seccomp-unotify instead, and handling all "monitored"
syscalls on behalf of the supervisee to prevent TOCTOU).
Also, if we do not allow one-off allows (but require the supervisor to
modify the ruleset), this cannot lead to TOCTOU because if the supervised
process tries to change the argument, it will cause another Landlock
denial and notification.
Thanks for the review!
Tingmao
^ permalink raw reply
* Re: [RFC] Landlock: mutable domains (and supervisor notification uAPI options)
From: Tingmao Wang @ 2026-02-22 18:04 UTC (permalink / raw)
To: Justin Suess, Günther Noack, Mickaël Salaün
Cc: amir73il, jack, jannh, linux-security-module, penguin-kernel,
song
In-Reply-To: <20260215212353.3549464-1-utilityemal77@gmail.com>
On 2/15/26 21:23, Justin Suess wrote:
> On Sun, Feb 15, 2026 at 02:54:08AM +0000, Tingmao Wang wrote:
> [...]
>> Discussion on LANDLOCK_ADD_RULE_INTERSECT
>> -----------------------------------------
>>
>> This was initially proposed by Mickaël, although now after writing some
>> example code against it [7], I'm not 100% sure that it is the most useful
>> uAPI. For a supervisor based on some sort of config file, it already has
>> to track which rules are added to know what to remove, and thus I feel
>> that it would be easier (both to use and to implement) to have an API that
>> simply "replaces" a rule, rather than do a bitwise AND on the access.
>>
> Instead of intersection being done at the rule level via
> landlock_add_rule, would it be better for intersection to be done at the
> ruleset_fd/ruleset level?
>
> So instead of intersecting individual rules, you can intersect entire
> rulesets, with the added benefit of being able to intersect handled
> accesses as well. (so you could handle an access initially, and not
> handle it later).
Personally I don't think making the list of handled accesses mutable would
add a lot of value (after all, a sandbox would usually handle all accesses
that it knows of), and I would like to avoid the complexity of making the
list of handled accesses mutable. The semantic of "intersection" and
"union" of handled accesses is also not trivial: if ruleset A handles
read, and ruleset B handles read+write, their "intersection", if
interpreted as "only allow accesses allowed by both rulesets", would in
fact handle read+write (and their "union" would handle read only).
In the second (union) case, there is also the problem of what to do if
ruleset B has write access rules - these rules would technically become
invalid (although to no negative effect) in a ruleset that doesn't handle
write.
I do see the benefit of modifying scope bits (and maybe also
quiet_access_* bits), but I'm still worried about the extra complexity
(and thus also testing / docs needed etc)
>
> Intersecting at the ruleset level allows for grouping the intersection rules
> together, so you could create an unenforced ruleset for the sole purpose
> of intersecting with rulesets, and intersect all the rule(s) at once.
>
> That way, the ruleset fd can be reused for this purpose later with other
> supervisees, instead of creating ruleset, intersecting individual rules,
> repeat.
>
> I think also the semantics of having a function called
> "landlock_add_rule" actually removing accesses (when the intersect flag
> is added) is also confusing, because we're not really *add*-ing
> anything, we're removing.
>
> ALTERNATIVE #1
>
> Maybe the best way to do it is instead continue treating rulesets as
> immutable, but allow composition of them at ruleset creation time.
>
> This would look something like:
>
> Ruleset C = Ruleset A & Ruleset B
>
> Ruleset A and B are never modified, but instead a new Ruleset C is
> created that is the intersection of A and B. This could be done in a
> variety of ways (LANDLOCK_CREATE_RULESET_INTERSECT? new IOCTL?)
>
> An example API for what this might look like:
>
> struct landlock_ruleset_attr ruleset_attr = {
> // other fields for handled accesses must be blank.
> .left_fd = existing_fd,
> .right_fd = other_existing_fd,
> };
> int new_ruleset_fd = syscall(SYS_landlock_create_ruleset, &ruleset_attr,
> sizeof(ruleset_attr), LANDLOCK_CREATE_RULESET_INTERSECT);
>
> And then the resulting ruleset which is the intersection of existing_fd
> and other_existing_fd could be returned.
>
> Similarly, we could:
>
> int new_ruleset_fd = syscall(SYS_landlock_create_ruleset, &ruleset_attr,
> sizeof(ruleset_attr), LANDLOCK_CREATE_RULESET_UNION);
If we do keep with the "intersect" way of removing rules (instead of
replace / clear all), this does seem like an interesting idea. However,
it is more complex to implement (it will probably require traversing two
rbtrees at once to be implemented efficiently), and I'm not sure how much
utility this would add compared to just LANDLOCK_ADD_RULE_INTERSECT. See
below for more reasoning.
>
> Which would be convienent for creating unions of rulesets.
>
> Then instead mutating rulesets, we commit/replace an entirely new ruleset.
>
> ioctl(supervisee_fd, LANDLOCK_IOCTL_COMMIT_RULESET, &new_ruleset_fd);
Using a dedicated ioctl to commit is also a potentially better idea - I
find that having the commit be a side effect of landlock_add_rule() via a
flag a bit unwieldy, as it would either require the supervisor to track
when it adds the last rule, or to add an "empty" rule just to commit.
Mickaël, you initially suggested the LANDLOCK_ADD_RULE_COMMIT_SUPERVISOR
flag, but do you think this is better?
>
> This has the following benefits:
>
> 1. Clearer semantics: "landlock_add_rule" is just for adding rules, not
> removing.
>
> 2. Intersection of all ruleset attributes, not just individual rule
> attributes.
>
> 3. Better logical grouping of rules for the purpose of intersection, and
> better composition.
>
> It does have drawbacks:
>
> 1. Intersecting individual rules requires making an entire ruleset for
> that one rule.
>
> 2. Users must be responsible for closing the unused/old rulesets that
> they might not longer need.
>
> ALTERNATIVE #2
>
> A middle ground is to keep the ruleset mutation via landlock_add_rule,
> but have it be done at the ruleset_fd level.
>
> Something like this:
>
> struct landlock_ruleset_operand intersection = {
> .operand = other_ruleset_fd
> };
> landlock_add_rule(ruleset_fd, LANDLOCK_RULE_INTERSECT_RULESET, &intersection, 0))
>
> I think this is also a valid way to do things, and increases the
> reusibility of rulesets.
>
> 1. Again, having landlock_add_rule being used to actually remove rules
> is confusing.
In this case, wouldn't we also be removing rules via landlock_add_rule()?
Personally I feel like this inconsistency is tolerable (it's easy enough
to explain), but I guess we could also change this to an ioctl if this is
a problem.
>
> 2. I'm unsure if we can change handled accesses after ruleset creation,
> so we might not be able to intersect the handled accesses like we can in
> the ALTERNATIVE #1.
>
>> Another alternative is to simply have a "clear all rules in this ruleset"
>> flag. This allows the supervisor to not have to track what is already
>> allowed - if it reloads the config file, it can simply clear the ruleset,
>> re-add all rules based on the config, then commit it. Although I worry
>> that this might make implementing some other use cases more difficult.
>
> At a minimum, it is cumbersome, and I worry about file descriptors
> becoming inaccessible (due to bind mounts / namespace changes in the
> supervisor's environment).
>
> Of course they can just hold those file descriptors open for the purposes
> of future intersections, but this is annoying and error prone.
If a supervisor doesn't care about potential renames / mount / namespace
changes making the sandboxed application lose access to previously
accessible files, the "clear all rules" approach would not force it to
keep any fds open in order to remove rules (i.e. it can clear everything,
then re-open the fds to add the rules back). On the other hand, with the
"intersect" approach, it would have to keep the fds open in all cases to
correctly remove previously added rules, so I think this "clear all" is
not more cumbersome.
There is a general consideration here about how much we want to design the
API to advantage / disadvantage particular ways of using it. For example,
having ruleset-ruleset intersection / union operations would (in theory,
setting aside the fact that to remain compatible to older kernels it
cannot do this for constructions of existing static rulesets) work very
well for something like island [1] / landlock-config, where we compose
rulesets by intersecting / unioning different landlock configuration files
together. However, it will introduce more complexity to someone who just
wants to allow access as they come up (e.g. something like a "permissive /
learning mode"), as they now have to, every time when an access is denied,
create a new ruleset, add one single rule, and do a union.
IMO, there is also the general preference of having complicated logic
being in user-space rather than implemented by the kernel. In this case,
one can argue that for someone that wants to compose rulesets via logical
operations, this should really be handled by a Landlock library, which
could, for static rulesets, do it completely internally in-memory, and do
one landlock_create_ruleset() + n landlock_add_rule()s in the end. For
live modifications, it could then use the more low-level "intersect" /
"add rule" / "clear all" uAPI. Compared to intersecting single rules,
having the kernel do the logical operations on entire rulesets also
doesn't reduce the number of syscalls needed (you still need one
landlock_add_rule() for each rule to be modified), so there is not a
performance argument either
(although for the use case where the supervisor wants to incrementally add
and remove rules, there is a performance benefit to intersect vs "clear
all". But I do wonder how often in practice this would be implemented for
a supervisor that can remove rules - because it needs to keep an in-memory
table for the fds it has open anyway, in order to correctly remove rules,
a simpler approach would be to simply clear all, then re-add whatever it
wants based on what it still has in that table).
[1]: https://github.com/landlock-lsm/island
^ permalink raw reply
* Re: [RFC] Landlock: mutable domains (and supervisor notification uAPI options)
From: Tingmao Wang @ 2026-02-22 18:04 UTC (permalink / raw)
To: Günther Noack, Mickaël Salaün, Justin Suess
Cc: linux-security-module
In-Reply-To: <6981cdab-dcff-47fc-a236-1890f9bc9c55@maowtm.org>
On 2/15/26 02:54, Tingmao Wang wrote:
> PoC code used in the above videos are largely generated, somewhat buggy,
> and unreviewed, but they are available:
>
> - mutable domains:
> https://github.com/micromaomao/linux-dev/pull/26/changes
> - supervisor notification:
> https://github.com/micromaomao/linux-dev/pull/27/changes
btw, on second thought I should have clarified that I don't expect anyone
to review any of the code here. Those were done purely to ensure that the
design I'm asking for review here actually works, and I'm open to any
changes.
^ permalink raw reply
* Re: [PATCH 05/13] selinux: Use simple_start_creating() / simple_done_creating()
From: Paul Moore @ 2026-02-22 14:19 UTC (permalink / raw)
To: NeilBrown
Cc: Christian Brauner, Alexander Viro, David Howells, Jan Kara,
Chuck Lever, Jeff Layton, Miklos Szeredi, Amir Goldstein,
John Johansen, James Morris, Serge E. Hallyn, Stephen Smalley,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <177171292163.8396.10671162503209732019@noble.neil.brown.name>
On Sat, Feb 21, 2026 at 5:28 PM NeilBrown <neilb@ownmail.net> wrote:
> On Sat, 21 Feb 2026, Paul Moore wrote:
> > On Wed, Feb 4, 2026 at 12:08 AM NeilBrown <neilb@ownmail.net> wrote:
> > >
> > > From: NeilBrown <neil@brown.name>
> > >
> > > Instead of explicitly locking the parent and performing a lookup in
> > > selinux, use simple_start_creating(), and then use
> > > simple_done_creating() to unlock.
> > >
> > > This extends the region that the directory is locked for, and also
> > > performs a lookup.
> > > The lock extension is of no real consequence.
> > > The lookup uses simple_lookup() and so always succeeds. Thus when
> > > d_make_persistent() is called the dentry will already be hashed.
> > > d_make_persistent() handles this case.
> > >
> > > Signed-off-by: NeilBrown <neil@brown.name>
> > > ---
> > > security/selinux/selinuxfs.c | 15 +++++++--------
> > > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > Unless I'm missing something, there is no reason why I couldn't take
> > just this patch into the SELinux tree once the merge window closes,
> > yes?
>
> Yes - but ...
>
> Once this series lands (hopefully soon - I will resend after -rc1 is
> out) I have another batch which depends on the new start_creating etc
> API being used everywhere ...
Okay, thanks for letting me know. I was curious about something like
that based on the cover letter, but the timing wasn't clear.
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 05/13] selinux: Use simple_start_creating() / simple_done_creating()
From: NeilBrown @ 2026-02-21 22:28 UTC (permalink / raw)
To: Paul Moore
Cc: Christian Brauner, Alexander Viro, David Howells, Jan Kara,
Chuck Lever, Jeff Layton, Miklos Szeredi, Amir Goldstein,
John Johansen, James Morris, Serge E. Hallyn, Stephen Smalley,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <CAHC9VhThChVk1Dk+f-KANGj7Tu7zzHCiA==taeQ+=nQaH6a7sg@mail.gmail.com>
On Sat, 21 Feb 2026, Paul Moore wrote:
> On Wed, Feb 4, 2026 at 12:08 AM NeilBrown <neilb@ownmail.net> wrote:
> >
> > From: NeilBrown <neil@brown.name>
> >
> > Instead of explicitly locking the parent and performing a lookup in
> > selinux, use simple_start_creating(), and then use
> > simple_done_creating() to unlock.
> >
> > This extends the region that the directory is locked for, and also
> > performs a lookup.
> > The lock extension is of no real consequence.
> > The lookup uses simple_lookup() and so always succeeds. Thus when
> > d_make_persistent() is called the dentry will already be hashed.
> > d_make_persistent() handles this case.
> >
> > Signed-off-by: NeilBrown <neil@brown.name>
> > ---
> > security/selinux/selinuxfs.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
>
> Unless I'm missing something, there is no reason why I couldn't take
> just this patch into the SELinux tree once the merge window closes,
> yes?
Yes - but ...
Once this series lands (hopefully soon - I will resend after -rc1 is
out) I have another batch which depends on the new start_creating etc
API being used everywhere. So for Christian to be able to apply that,
he will need to pull in this patch from the SELinux tree.
So if you could apply just this patch to some branch and merge that
branch with your other work however works best for you, and make the
branch available, then I think Christian will be happy to merge that
with whatever vfs branch he includes my work in, and all should be good.
Thanks
NeilBrown
>
> --
> paul-moore.com
>
^ permalink raw reply
* [PATCH v2] landlock: Expand restrict flags example for ABI version 8
From: Panagiotis "Ivory" Vasilopoulos @ 2026-02-21 22:12 UTC (permalink / raw)
To: Mickaël Salaün, Günther Noack, Jonathan Corbet,
Shuah Khan
Cc: linux-security-module, linux-doc, linux-kernel
Add LANDLOCK_RESTRICT_SELF_TSYNC to the backwards compatibility example
for restrict flags. This introduces completeness, similar to that of
the ruleset attributes example.
Additionally, I modified the two comments of the example to make them
more consistent with the ruleset attributes example's.
Signed-off-by: Panagiotis 'Ivory' Vasilopoulos <git@n0toose.net>
---
Changes in v2:
- Fix formatting error.
- Link to v1: https://lore.kernel.org/r/20260221-landlock-docs-add-tsync-example-v1-1-f89383809eb4@n0toose.net
---
Documentation/userspace-api/landlock.rst | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 13134bccdd39d78ddce3daf454f32dda162ce91b..0affe1c953d61a4b32aca700cd262c49cee6304a 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -197,12 +197,18 @@ similar backwards compatibility check is needed for the restrict flags
.. code-block:: c
- __u32 restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
- if (abi < 7) {
- /* Clear logging flags unsupported before ABI 7. */
+ __u32 restrict_flags =
+ LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
+ LANDLOCK_RESTRICT_SELF_TSYNC;
+ switch (abi) {
+ case 1 ... 6:
+ /* Clear logging flags unsupported for ABI < 7 */
restrict_flags &= ~(LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF);
+ case 7:
+ /* Removes multithread flag unsupported for ABI < 8 */
+ restrict_flags &= ~LANDLOCK_RESTRICT_SELF_TSYNC;
}
The next step is to restrict the current thread from gaining more privileges
---
base-commit: ceb977bfe9e8715e6cd3a4785c7aab8ea5cd2b77
change-id: 20260221-landlock-docs-add-tsync-example-e8fd5c64a366
Best regards,
--
Panagiotis "Ivory" Vasilopoulos <git@n0toose.net>
^ permalink raw reply related
* [PATCH] landlock: Expand restrict flags example for ABI version 8
From: Panagiotis "Ivory" Vasilopoulos @ 2026-02-21 22:05 UTC (permalink / raw)
To: Mickaël Salaün, Günther Noack, Jonathan Corbet,
Shuah Khan
Cc: linux-security-module, linux-doc, linux-kernel
Add LANDLOCK_RESTRICT_SELF_TSYNC to the backwards compatibility example
for restrict flags. This introduces completeness, similar to that of
the ruleset attributes example.
Additionally, I modified the two comments of the example to make them
more consistent with the ruleset attributes example's.
Signed-off-by: Panagiotis 'Ivory' Vasilopoulos <git@n0toose.net>
---
Documentation/userspace-api/landlock.rst | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 13134bccdd39d78ddce3daf454f32dda162ce91b..d972af20791a722d4665bd2ac287b05c5e97248c 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -197,12 +197,18 @@ similar backwards compatibility check is needed for the restrict flags
.. code-block:: c
- __u32 restrict_flags = LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
- if (abi < 7) {
- /* Clear logging flags unsupported before ABI 7. */
+ __u32 restrict_flags =
+ LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
+ LANDLOCK_RESTRICT_SELF_TSYNC;
+ switch (abi) {
+ case 1 ... 6:
+ /* Clear logging flags unsupported for ABI < 7 */
restrict_flags &= ~(LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF);
+ case 7:
+ /* Removes multithread flag unsupported for ABI < 8 */
+ restrict_flags &= ~LANDLOCK_RESTRICT_SELF_TSYNC;
}
The next step is to restrict the current thread from gaining more privileges
---
base-commit: ceb977bfe9e8715e6cd3a4785c7aab8ea5cd2b77
change-id: 20260221-landlock-docs-add-tsync-example-e8fd5c64a366
Best regards,
--
Panagiotis "Ivory" Vasilopoulos <git@n0toose.net>
^ permalink raw reply related
* Re: [PATCH v4 15/17] module: Introduce hash-based integrity checking
From: Nicolas Schier @ 2026-02-21 21:38 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Daniel Gomez,
Aaron Tomlin, Christophe Leroy (CS GROUP), Nicolas Bouchinet,
Xiu Jianfeng, Fabian Grünbichler, Arnout Engelen,
Mattia Rizzolo, kpcyrd, Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260113-module-hashes-v4-15-0b932db9b56b@weissschuh.net>
Hi Thomas,
On Tue, Jan 13, 2026 at 01:28:59PM +0100, Thomas Weißschuh wrote:
> The current signature-based module integrity checking has some drawbacks
> in combination with reproducible builds. Either the module signing key
> is generated at build time, which makes the build unreproducible, or a
> static signing key is used, which precludes rebuilds by third parties
> and makes the whole build and packaging process much more complicated.
>
> The goal is to reach bit-for-bit reproducibility. Excluding certain
> parts of the build output from the reproducibility analysis would be
> error-prone and force each downstream consumer to introduce new tooling.
>
> Introduce a new mechanism to ensure only well-known modules are loaded
> by embedding a merkle tree root of all modules built as part of the full
> kernel build into vmlinux.
>
> Non-builtin modules can be validated as before through signatures.
>
> Normally the .ko module files depend on a fully built vmlinux to be
> available for modpost validation and BTF generation. With
> CONFIG_MODULE_HASHES, vmlinux now depends on the modules
> to build a merkle tree. This introduces a dependency cycle which is
> impossible to satisfy. Work around this by building the modules during
> link-vmlinux.sh, after vmlinux is complete enough for modpost and BTF
> but before the final module hashes are
>
> The PKCS7 format which is used for regular module signatures can not
> represent Merkle proofs, so a new kind of module signature is
> introduced. As this signature type is only ever used for builtin
> modules, no compatibility issues can arise.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> .gitignore | 1 +
> Documentation/kbuild/reproducible-builds.rst | 5 +-
> Makefile | 8 +-
> include/asm-generic/vmlinux.lds.h | 11 +
> include/linux/module_hashes.h | 25 ++
> include/linux/module_signature.h | 1 +
> kernel/module/Kconfig | 21 +-
> kernel/module/Makefile | 1 +
> kernel/module/hashes.c | 92 ++++++
> kernel/module/hashes_root.c | 6 +
> kernel/module/internal.h | 1 +
> kernel/module/main.c | 4 +-
> scripts/.gitignore | 1 +
> scripts/Makefile | 3 +
> scripts/Makefile.modfinal | 11 +
> scripts/Makefile.modinst | 13 +
> scripts/Makefile.vmlinux | 5 +
> scripts/link-vmlinux.sh | 14 +-
> scripts/modules-merkle-tree.c | 467 +++++++++++++++++++++++++++
> security/lockdown/Kconfig | 2 +-
> 20 files changed, 685 insertions(+), 7 deletions(-)
>
[...]
> diff --git a/kernel/module/hashes_root.c b/kernel/module/hashes_root.c
> new file mode 100644
> index 000000000000..1abfcd3aa679
> --- /dev/null
> +++ b/kernel/module/hashes_root.c
> @@ -0,0 +1,6 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/module_hashes.h>
> +
> +/* Blank dummy data. Will be overridden by link-vmlinux.sh */
> +const struct module_hashes_root module_hashes_root __module_hashes_section = {};
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index e2d49122c2a1..e22837d3ac76 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -338,6 +338,7 @@ void module_mark_ro_after_init(const Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
> const char *secstrings);
>
> int module_sig_check(struct load_info *info, const u8 *sig, size_t sig_len);
> +int module_hash_check(struct load_info *info, const u8 *sig, size_t sig_len);
>
> #ifdef CONFIG_DEBUG_KMEMLEAK
> void kmemleak_load_module(const struct module *mod, const struct load_info *info);
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 2a28a0ece809..fa30b6387936 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3362,8 +3362,10 @@ static int module_integrity_check(struct load_info *info, int flags)
>
> if (IS_ENABLED(CONFIG_MODULE_SIG) && sig_type == PKEY_ID_PKCS7) {
> err = module_sig_check(info, sig, sig_len);
> + } else if (IS_ENABLED(CONFIG_MODULE_HASHES) && sig_type == PKEY_ID_MERKLE) {
> + err = module_hash_check(info, sig, sig_len);
> } else {
> - pr_err("module: not signed with expected PKCS#7 message\n");
> + pr_err("module: not signed with signature mechanism\n");
> err = -ENOPKG;
To prevent others from running into the same issue:
My first test got stuck here, as I tested with virtme-ng, which symlinks
modules from build tree to /lib/modules/$(uname -r)/..., resulting in
[ 15.956855] module: not signed with signature mechanism
modprobe: ERROR: could not insert 'efivarfs': Package not installed
As the modules_install step was missing, modules were not being signed.
[...]
> diff --git a/scripts/modules-merkle-tree.c b/scripts/modules-merkle-tree.c
> new file mode 100644
> index 000000000000..a6ec0e21213b
> --- /dev/null
> +++ b/scripts/modules-merkle-tree.c
> @@ -0,0 +1,467 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Compute hashes for modules files and build a merkle tree.
> + *
> + * Copyright (C) 2025 Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> + * Copyright (C) 2025 Thomas Weißschuh <linux@weissschuh.net>
> + *
> + */
> +#define _GNU_SOURCE 1
> +#include <arpa/inet.h>
> +#include <err.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <stdarg.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include <stdlib.h>
> +
> +#include <sys/stat.h>
> +#include <sys/mman.h>
> +
> +#include <openssl/evp.h>
> +#include <openssl/err.h>
> +
> +#include "ssl-common.h"
> +
> +static int hash_size;
> +static EVP_MD_CTX *ctx;
> +
> +struct module_signature {
> + uint8_t algo; /* Public-key crypto algorithm [0] */
> + uint8_t hash; /* Digest algorithm [0] */
> + uint8_t id_type; /* Key identifier type [PKEY_ID_PKCS7] */
> + uint8_t signer_len; /* Length of signer's name [0] */
> + uint8_t key_id_len; /* Length of key identifier [0] */
> + uint8_t __pad[3];
> + uint32_t sig_len; /* Length of signature data */
> +};
> +
> +#define PKEY_ID_MERKLE 3
> +
> +static const char magic_number[] = "~Module signature appended~\n";
This here will be the forth definition of struct module_signature,
increasing the risk of unwanted diversion. I second Petr's suggestion
to reuse a _common_ definition instead.
(Here, even include/linux/module_signature.h could be included itself.)
> +
> +struct file_entry {
> + char *name;
> + unsigned int pos;
> + unsigned char hash[EVP_MAX_MD_SIZE];
> +};
> +
> +static struct file_entry *fh_list;
> +static size_t num_files;
> +
> +struct leaf_hash {
> + unsigned char hash[EVP_MAX_MD_SIZE];
> +};
> +
> +struct mtree {
> + struct leaf_hash **l;
> + unsigned int *entries;
> + unsigned int levels;
> +};
> +
> +static inline void *xcalloc(size_t n, size_t size)
> +{
> + void *p;
> +
> + p = calloc(n, size);
> + if (!p)
> + errx(1, "Memory allocation failed");
> +
> + return p;
> +}
> +
> +static void *xmalloc(size_t size)
> +{
> + void *p;
> +
> + p = malloc(size);
> + if (!p)
> + errx(1, "Memory allocation failed");
> +
> + return p;
> +}
> +
> +static inline void *xreallocarray(void *oldp, size_t n, size_t size)
> +{
> + void *p;
> +
> + p = reallocarray(oldp, n, size);
> + if (!p)
> + errx(1, "Memory allocation failed");
> +
> + return p;
> +}
> +
> +static inline char *xasprintf(const char *fmt, ...)
> +{
> + va_list ap;
> + char *strp;
> + int ret;
> +
> + va_start(ap, fmt);
> + ret = vasprintf(&strp, fmt, ap);
> + va_end(ap);
> + if (ret == -1)
> + err(1, "Memory allocation failed");
> +
> + return strp;
> +}
Please consider moving these x* functions into scripts/include/xalloc.h
for reuse. (I am sure someone else wrote this already, but I can't find
it...)
>
>
thanks for all your efforts for reproducibility!
As I have no clue about that: Is the patent for merkle trees [1] a
problem when integrating that here?
Can you verify if I get the mechanics roughly correct?
* Modules are merkle tree leaves. Modules are built and logically
paired by the order from modules.order; a single left-over module is
paired with itself.
* Hashes of paired modules are hashed again (branch node hash);
hashes of pairs of branch nodes' hashes are hashed again;
repeat until we reach the single merkle tree root hash
* The final merkle tree root hash (and the count of tree levels) is
included in vmlinux
'make && find . -name '*.ko' -exec rm {} \; && make' does not rebuild
the in-tree modules. Shifting the module-hashes support from
scripts/link-vmlinux.sh to scripts/Makefile.vmlinux might (make it
easier) to fix this again.
Kind regards,
Nicolas
[1]: https://worldwide.espacenet.com/patent/search/family/022107098/publication/US4309569A?q=pn%3DUS4309569
--
Nicolas
^ permalink raw reply
* Re: [PATCH v6] lsm: Add LSM hook security_unix_find
From: Justin Suess @ 2026-02-21 13:22 UTC (permalink / raw)
To: Günther Noack
Cc: brauner, demiobenour, fahimitahera, hi, horms, ivanov.mikhail1,
jannh, jmorris, john.johansen, konstantin.meskhidze,
linux-security-module, m, matthieu, mic, netdev, paul,
samasth.norway.ananda, serge, viro
In-Reply-To: <20260220.70d0460d1309@gnoack.org>
On Fri, Feb 20, 2026 at 04:49:34PM +0100, Günther Noack wrote:
> Hello!
>
> On Thu, Feb 19, 2026 at 03:04:59PM -0500, Justin Suess wrote:
> > diff --git a/security/security.c b/security/security.c
> > index 67af9228c4e9..c73196b8db4b 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
> >
> > #endif /* CONFIG_SECURITY_NETWORK */
> >
> > +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> > +/**
> > + * security_unix_find() - Check if a named AF_UNIX socket can connect
> > + * @path: path of the socket being connected to
> > + * @other: peer sock
> > + * @flags: flags associated with the socket
> > + *
> > + * This hook is called to check permissions before connecting to a named
> > + * AF_UNIX socket.
>
> Nit: Could we please insert a sentence about locking here?
>
> Something like:
>
> The caller holds no locks on @other.
>
> (Originally brought up by Mickaël in
> https://lore.kernel.org/all/20260217.lievaS8eeng8@digikod.net/)
>
> Thanks,
> –Günther
Sounds good. Would a "Link:" to the mentioned thread be appropriate in the commit
message?
I feel like the reasoning for this is subtle but important for hook
consumers.
Justin
^ permalink raw reply
* Re: [syzbot] [kernel?] INFO: task hung in restrict_one_thread_callback
From: Günther Noack @ 2026-02-21 13:19 UTC (permalink / raw)
To: Ding Yihan
Cc: syzbot, Mickaël Salaün, linux-security-module,
Jann Horn
In-Reply-To: <20260221.5d8a306bcaf1@gnoack.org>
On Sat, Feb 21, 2026 at 01:00:03PM +0100, Günther Noack wrote:
> (Very) tentative investigation:
>
> In the Syzkaller report [2], it seems that the reproducer [2.1] is
> creating two rulesets and then enforcing them in parallel, a scenario
> which we are exercising in the TEST(competing_enablement) in
> tools/testing/selftests/landlock/tsync_test.c already, but which has
> not failed in my own selftest runs.
>
> In the crash report, there are four threads in total:
>
> * Two are stuck in the line
> wait_for_completion(&ctx->ready_to_commit);
> in the per-thread task work (line 128 [4.1])
> * Two are stuck in the line
> wait_for_completion(&shared_ctx.all_prepared)
> in the calling thread's coordination logic (line 539 [4.2])
>
> In line 539, we are already on the code path where we detected that we
> are getting interrupted by another thread and where we are attempting
> to deal with the scenario where two landlock_restrict_self() calls
> compete. This is detected on line 523 when
> wait_for_completion_interruptible() is true. The approach to handle
> this is to set the overall -ERESTARTNOINTR error and cancel the work
> that has been ongoing so far, by canceling the task works that did not
> start running yet and waiting for the ones that did start running
> (that is the step where we are blocked!). The reasoning there was
> that these task works will all hit the "all_prepared" stage now, but
> as we can see in the stack trace, the task works that are actively
> running are already on line 128 and have passed the "all_prepared"
> stage).
>
> Differences I can see between syzkaller and our own test:
>
> * The reproducer also calls openat() and then twice socketpair().
> These syscalls should be unrelated, but it's possible that the
> "async" invocation of socketpair() contributes to adding more
> threads. (Assuming that "async" means "in new thread" in syzkaller)
> * Syzkaller gives it more attempts. ([2.2])
>
> I do not understand yet what went wrong in our scheme and need to look
> deeper.
OK, I think I understand now. Our existing recovery code for this
conflict is this:
/*
* Decrement num_preparing for current, to undo that we initialized it
* to 1 a few lines above.
*/
if (atomic_dec_return(&shared_ctx.num_preparing) > 0) {
if (wait_for_completion_interruptible(
&shared_ctx.all_prepared)) {
/* In case of interruption, we need to retry the system call. */
atomic_set(&shared_ctx.preparation_error,
-ERESTARTNOINTR);
/*
* Cancel task works for tasks that did not start running yet,
* and decrement all_prepared and num_unfinished accordingly.
*/
cancel_tsync_works(&works, &shared_ctx);
/*
* The remaining task works have started running, so waiting for
* their completion will finish.
*/
wait_for_completion(&shared_ctx.all_prepared);
}
}
When I wrote this, I assumed, as the last comment states, that the
task works which we could not cancel, are already running.
I was wrong there, because I had misunderstood task_work_run(). When
the task works get run there, it first *atomically dequeues the entire
queue of scheduled task works*, and then runs them sequentially.
That is why, if we have one task work that belongs to the first
landlock_restrict_self() call and one which belongs to the other, the
task work which is scheduled later can (a) not be dequeued with
cancel_tsync_works() any more, and (b) also has not started running
yet.
Now the only thing that is necessary to produce the deadlock is that
we have a pair of threads where the task works for the restriction
calls have been scheduled in different order. When the two
landlock_restrict_self() calls end up in the recovery path quoted
above, they will wait for one of their task works to run which is
blocked from running by another task work that is scheduled before and
does not finish either.
(Just pasting a brain dump here to save you some time hunting for the
root cause. I don't know the best solution yet either.)
–Günther
^ permalink raw reply
* Re: [syzbot] [kernel?] INFO: task hung in restrict_one_thread_callback
From: Günther Noack @ 2026-02-21 12:00 UTC (permalink / raw)
To: Ding Yihan
Cc: syzbot, Mickaël Salaün, linux-security-module,
Jann Horn
In-Reply-To: <00A9E53EDC82309F+7b1dfc69-95f8-4ffc-a67c-967de0e2dfee@uniontech.com>
Hello Ding!
On Sat, Feb 21, 2026 at 03:28:47PM +0800, Ding Yihan wrote:
> Since I am relatively new to the inner workings of this specific subsystem,
> I would like to take a few days to thoroughly study the root cause
> (the task_work and mutex interaction) and prepare a detailed and proper commit message.
>
> I will send out the formal patch (v1) to the mailing list later.
Thank you very much for preparing a patch, and especially also for
forwarding this to us. (The original syzkaller report was somehow not
addressed to Landlock or the LSM list. We should fix that.)
Timing wise, the feature was picked up for the 7.0 release, so we
still have some time to fix it before this is stable.
As an early review for the patch:
Background:
We had previously convinced ourselves that grabbing the
cred_guard_mutex was not necessary. To quote the comment in
landlock_restrict_sibling_threads():
Unlike seccomp, which modifies sibling tasks directly, we do not need to
acquire the cred_guard_mutex and sighand->siglock:
- As in our case, all threads are themselves exchanging their own struct
cred through the credentials API, no locks are needed for that.
- Our for_each_thread() loops are protected by RCU.
- We do not acquire a lock to keep the list of sibling threads stable
between our for_each_thread loops. If the list of available sibling
threads changes between these for_each_thread loops, we make up for
that by continuing to look for threads until they are all discovered
and have entered their task_work, where they are unable to spawn new
threads.
The question of locking cred_guard_mutex came up in the patch
discussion multiple times as well, the most recent discussion was:
https://lore.kernel.org/all/20251020.fohbo6Iecahz@digikod.net/
If it helps, I keep some of my own notes for this particular feature
on https://wiki.gnoack.org/LandlockMultithreadedEnforcement.
(Very) tentative investigation:
In the Syzkaller report [2], it seems that the reproducer [2.1] is
creating two rulesets and then enforcing them in parallel, a scenario
which we are exercising in the TEST(competing_enablement) in
tools/testing/selftests/landlock/tsync_test.c already, but which has
not failed in my own selftest runs.
In the crash report, there are four threads in total:
* Two are stuck in the line
wait_for_completion(&ctx->ready_to_commit);
in the per-thread task work (line 128 [4.1])
* Two are stuck in the line
wait_for_completion(&shared_ctx.all_prepared)
in the calling thread's coordination logic (line 539 [4.2])
In line 539, we are already on the code path where we detected that we
are getting interrupted by another thread and where we are attempting
to deal with the scenario where two landlock_restrict_self() calls
compete. This is detected on line 523 when
wait_for_completion_interruptible() is true. The approach to handle
this is to set the overall -ERESTARTNOINTR error and cancel the work
that has been ongoing so far, by canceling the task works that did not
start running yet and waiting for the ones that did start running
(that is the step where we are blocked!). The reasoning there was
that these task works will all hit the "all_prepared" stage now, but
as we can see in the stack trace, the task works that are actively
running are already on line 128 and have passed the "all_prepared"
stage).
Differences I can see between syzkaller and our own test:
* The reproducer also calls openat() and then twice socketpair().
These syscalls should be unrelated, but it's possible that the
"async" invocation of socketpair() contributes to adding more
threads. (Assuming that "async" means "in new thread" in syzkaller)
* Syzkaller gives it more attempts. ([2.2])
I do not understand yet what went wrong in our scheme and need to look
deeper.
Ding, do you have more insights into it from your debugging?
Thanks,
–Günther
For reference:
[1] Report Mail: https://lore.kernel.org/all/69984159.050a0220.21cd75.01bb.GAE@google.com/
[2] Report: https://syzkaller.appspot.com/bug?extid=7ea2f5e9dfd468201817
[2.1] Reproducer: https://syzkaller.appspot.com/text?tag=ReproSyz&x=16e41c02580000
[2.2] Reproducer (C): https://syzkaller.appspot.com/text?tag=ReproC&x=15813652580000
[3] Patch: https://lore.kernel.org/all/6999504d.a70a0220.2c38d7.0154.GAE@google.com/
[4.1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/security/landlock/tsync.c?id=635c467cc14ebdffab3f77610217c1dacaf88e8c#n128
[4.2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/security/landlock/tsync.c?id=635c467cc14ebdffab3f77610217c1dacaf88e8c#n539
^ permalink raw reply
* Re: [syzbot] [kernel?] INFO: task hung in restrict_one_thread_callback
From: Ding Yihan @ 2026-02-21 7:28 UTC (permalink / raw)
To: syzbot, Mickaël Salaün
Cc: linux-security-module, Günther Noack
In-Reply-To: <69995a88.050a0220.340abe.0d25.GAE@google.com>
Hi all,
Thanks to syzbot for the testing and confirmation.
Since I am relatively new to the inner workings of this specific subsystem,
I would like to take a few days to thoroughly study the root cause
(the task_work and mutex interaction) and prepare a detailed and proper commit message.
I will send out the formal patch (v1) to the mailing list later.
Best regards,
Yihan Ding
在 2026/2/21 15:11, syzbot 写道:
> Hello,
>
> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
>
> Reported-by: syzbot+7ea2f5e9dfd468201817@syzkaller.appspotmail.com
> Tested-by: syzbot+7ea2f5e9dfd468201817@syzkaller.appspotmail.com
>
> Tested on:
>
> commit: d4906ae1 Add linux-next specific files for 20260220
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=13ea89e6580000
> kernel config: https://syzkaller.appspot.com/x/.config?x=51f859f3211496bc
> dashboard link: https://syzkaller.appspot.com/bug?extid=7ea2f5e9dfd468201817
> compiler: Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
> patch: https://syzkaller.appspot.com/x/patch.diff?x=15f0595a580000
>
> Note: testing is done by a robot and is best-effort only.
>
^ permalink raw reply
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