* [PATCH v2 2/7] fs/9p: add option for path-based inodes
From: Tingmao Wang @ 2025-09-04 0:04 UTC (permalink / raw)
To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, Mickaël Salaün
Cc: Tingmao Wang, v9fs, Günther Noack, linux-security-module,
Jan Kara, Amir Goldstein, Matthew Bobrowski, Al Viro,
linux-fsdevel
In-Reply-To: <cover.1756935780.git.m@maowtm.org>
By this point we have two ways to test for inode reuse - qid and qid+path.
By default, uncached mode uses qid+path and cached mode uses qid (and in
fact does not support qid+path). This patch adds the option to control
the behaviour for uncached mode.
In a future version, if we can negotiate with the server and be sure that
it won't give us duplicate qid.path, the default for those cases can be
qid-based.
Signed-off-by: Tingmao Wang <m@maowtm.org>
Cc: "Mickaël Salaün" <mic@digikod.net>
Cc: "Günther Noack" <gnoack@google.com>
---
Changes since v1:
- Removed inodeident=none and instead supports inodeident=qid. This means
that there is no longer an option to not re-use inodes at all.
- No longer supports inodeident=path on cached mode, checks added at
option init time.
- Added explicit bits for both V9FS_INODE_IDENT_PATH and
V9FS_INODE_IDENT_QID, in order to set a default based on cache bits when
neither are set explicitly by the user.
fs/9p/v9fs.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++-
fs/9p/v9fs.h | 3 +++
2 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index 77e9c4387c1d..f87d6680b85a 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -36,7 +36,7 @@ enum {
/* Options that take integer arguments */
Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid,
/* String options */
- Opt_uname, Opt_remotename, Opt_cache, Opt_cachetag,
+ Opt_uname, Opt_remotename, Opt_cache, Opt_cachetag, Opt_inodeident,
/* Options that take no arguments */
Opt_nodevmap, Opt_noxattr, Opt_directio, Opt_ignoreqv,
/* Access options */
@@ -63,6 +63,7 @@ static const match_table_t tokens = {
{Opt_access, "access=%s"},
{Opt_posixacl, "posixacl"},
{Opt_locktimeout, "locktimeout=%u"},
+ {Opt_inodeident, "inodeident=%s"},
{Opt_err, NULL}
};
@@ -149,6 +150,21 @@ int v9fs_show_options(struct seq_file *m, struct dentry *root)
if (v9ses->flags & V9FS_NO_XATTR)
seq_puts(m, ",noxattr");
+ switch (v9ses->flags & V9FS_INODE_IDENT_MASK) {
+ case V9FS_INODE_IDENT_QID:
+ seq_puts(m, ",inodeident=qid");
+ break;
+ case V9FS_INODE_IDENT_PATH:
+ seq_puts(m, ",inodeident=path");
+ break;
+ default:
+ /*
+ * Unspecified, will be set later in v9fs_session_init depending on
+ * cache setting
+ */
+ break;
+ }
+
return p9_show_client_options(m, v9ses->clnt);
}
@@ -369,6 +385,26 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
v9ses->session_lock_timeout = (long)option * HZ;
break;
+ case Opt_inodeident:
+ s = match_strdup(&args[0]);
+ if (!s) {
+ ret = -ENOMEM;
+ p9_debug(P9_DEBUG_ERROR,
+ "problem allocating copy of inodeident arg\n");
+ goto free_and_return;
+ }
+ v9ses->flags &= ~V9FS_INODE_IDENT_MASK;
+ if (strcmp(s, "qid") == 0) {
+ v9ses->flags |= V9FS_INODE_IDENT_QID;
+ } else if (strcmp(s, "path") == 0) {
+ v9ses->flags |= V9FS_INODE_IDENT_PATH;
+ } else {
+ ret = -EINVAL;
+ p9_debug(P9_DEBUG_ERROR, "Unknown inodeident argument %s\n", s);
+ }
+ kfree(s);
+ break;
+
default:
continue;
}
@@ -393,6 +429,7 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
{
struct p9_fid *fid;
int rc = -ENOMEM;
+ bool cached;
v9ses->uname = kstrdup(V9FS_DEFUSER, GFP_KERNEL);
if (!v9ses->uname)
@@ -427,6 +464,26 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
if (rc < 0)
goto err_clnt;
+ cached = v9ses->cache & (CACHE_META | CACHE_LOOSE);
+
+ if (cached && v9ses->flags & V9FS_INODE_IDENT_PATH) {
+ rc = -EINVAL;
+ p9_debug(P9_DEBUG_ERROR,
+ "inodeident=path not supported in cached mode\n");
+ goto err_clnt;
+ }
+
+ if (!(v9ses->flags & V9FS_INODE_IDENT_MASK)) {
+ /* Unspecified - use default */
+ if (cached) {
+ /* which is qid in cached mode (path not supported) */
+ v9ses->flags |= V9FS_INODE_IDENT_QID;
+ } else {
+ /* ...or path in uncached mode */
+ v9ses->flags |= V9FS_INODE_IDENT_PATH;
+ }
+ }
+
v9ses->maxdata = v9ses->clnt->msize - P9_IOHDRSZ;
if (!v9fs_proto_dotl(v9ses) &&
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index 134b55a605be..b4e738c1bba5 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -43,8 +43,11 @@ enum p9_session_flags {
V9FS_DIRECT_IO = 0x100,
V9FS_SYNC = 0x200,
V9FS_INODE_IDENT_PATH = 0x400,
+ V9FS_INODE_IDENT_QID = 0x800,
};
+#define V9FS_INODE_IDENT_MASK (V9FS_INODE_IDENT_PATH | V9FS_INODE_IDENT_QID)
+
/**
* enum p9_cache_shortcuts - human readable cache preferences
* @CACHE_SC_NONE: disable all caches
--
2.51.0
^ permalink raw reply related
* [PATCH v2 3/7] fs/9p: Add ability to identify inode by path for non-.L in uncached mode
From: Tingmao Wang @ 2025-09-04 0:04 UTC (permalink / raw)
To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, Mickaël Salaün
Cc: Tingmao Wang, v9fs, Günther Noack, linux-security-module,
Jan Kara, Amir Goldstein, Matthew Bobrowski, Al Viro,
linux-fsdevel
In-Reply-To: <cover.1756935780.git.m@maowtm.org>
This replicates the earlier .L patch for non-.L, and removing some
previously inserted conditionals in shared code.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v1:
- Reflect v2 changes to the .L counterpart of this.
fs/9p/v9fs.h | 7 ++-
fs/9p/vfs_inode.c | 150 ++++++++++++++++++++++++++++++++++++++--------
2 files changed, 130 insertions(+), 27 deletions(-)
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index b4e738c1bba5..bacd0052e22c 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -202,7 +202,8 @@ extern int v9fs_vfs_rename(struct mnt_idmap *idmap,
unsigned int flags);
extern struct inode *v9fs_inode_from_fid(struct v9fs_session_info *v9ses,
struct p9_fid *fid,
- struct super_block *sb, int new);
+ struct super_block *sb,
+ struct dentry *dentry, int new);
extern const struct inode_operations v9fs_dir_inode_operations_dotl;
extern const struct inode_operations v9fs_file_inode_operations_dotl;
extern const struct inode_operations v9fs_symlink_inode_operations_dotl;
@@ -267,7 +268,7 @@ v9fs_get_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
if (v9fs_proto_dotl(v9ses))
return v9fs_inode_from_fid_dotl(v9ses, fid, sb, dentry, 0);
else
- return v9fs_inode_from_fid(v9ses, fid, sb, 0);
+ return v9fs_inode_from_fid(v9ses, fid, sb, dentry, 0);
}
/**
@@ -291,7 +292,7 @@ v9fs_get_new_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
if (v9fs_proto_dotl(v9ses))
return v9fs_inode_from_fid_dotl(v9ses, fid, sb, dentry, 1);
else
- return v9fs_inode_from_fid(v9ses, fid, sb, 1);
+ return v9fs_inode_from_fid(v9ses, fid, sb, dentry, 1);
}
#endif
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 5e56c13da733..606760f966fd 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -364,29 +364,76 @@ void v9fs_evict_inode(struct inode *inode)
clear_inode(inode);
}
+struct iget_data {
+ struct p9_wstat *st;
+
+ /* May be NULL */
+ struct dentry *dentry;
+
+ bool need_double_check;
+};
+
static int v9fs_test_inode(struct inode *inode, void *data)
{
int umode;
dev_t rdev;
struct v9fs_inode *v9inode = V9FS_I(inode);
- struct p9_wstat *st = (struct p9_wstat *)data;
+ struct p9_wstat *st = ((struct iget_data *)data)->st;
+ struct dentry *dentry = ((struct iget_data *)data)->dentry;
struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);
+ bool cached = v9ses->cache & (CACHE_META | CACHE_LOOSE);
umode = p9mode2unixmode(v9ses, st, &rdev);
- /* don't match inode of different type */
+ /*
+ * Don't reuse inode of different type, even if path matches.
+ */
if (inode_wrong_type(inode, umode))
return 0;
- /* compare qid details */
- if (memcmp(&v9inode->qid.version,
- &st->qid.version, sizeof(v9inode->qid.version)))
- return 0;
-
if (v9inode->qid.type != st->qid.type)
return 0;
if (v9inode->qid.path != st->qid.path)
return 0;
+
+ if (cached) {
+ /*
+ * Server side changes are not supposed to happen in cached mode.
+ * If we fail this version comparison on the inode, we don't reuse
+ * it.
+ */
+ if (memcmp(&v9inode->qid.version,
+ &st->qid.version, sizeof(v9inode->qid.version)))
+ return 0;
+ }
+
+ if (v9fs_inode_ident_path(v9ses) && dentry) {
+ if (v9inode->path) {
+ if (!ino_path_compare(v9inode->path, dentry)) {
+ p9_debug(
+ P9_DEBUG_VFS,
+ "Refusing to reuse inode %p based on path mismatch",
+ inode);
+ return 0;
+ }
+ } else if (inode->i_state & I_NEW) {
+ /*
+ * iget5_locked may call this function with a still
+ * initializing (I_NEW) inode, so we're now racing with the
+ * code in v9fs_qid_iget that prepares v9inode->path.
+ * Returning from this test function now with positive result
+ * will cause us to wait for this inode to be ready, and we
+ * can then re-check in v9fs_qid_iget.
+ */
+ ((struct iget_data *)data)->need_double_check = true;
+ } else {
+ WARN_ONCE(
+ 1,
+ "Inode %p (ino %lu) does not have v9inode->path even though fs has path-based inode identification enabled?",
+ inode, inode->i_ino);
+ }
+ }
+
return 1;
}
@@ -395,33 +442,74 @@ static int v9fs_test_new_inode(struct inode *inode, void *data)
return 0;
}
-static int v9fs_set_inode(struct inode *inode, void *data)
+static int v9fs_set_inode(struct inode *inode, void *data)
{
struct v9fs_inode *v9inode = V9FS_I(inode);
- struct p9_wstat *st = (struct p9_wstat *)data;
+ struct iget_data *idata = data;
+ struct p9_wstat *st = idata->st;
memcpy(&v9inode->qid, &st->qid, sizeof(st->qid));
+ /*
+ * We can't fill v9inode->path here, because allocating an ino_path
+ * means that we might sleep, and we can't sleep here.
+ */
+ v9inode->path = NULL;
return 0;
}
-static struct inode *v9fs_qid_iget(struct super_block *sb,
- struct p9_qid *qid,
- struct p9_wstat *st,
+static struct inode *v9fs_qid_iget(struct super_block *sb, struct p9_qid *qid,
+ struct p9_wstat *st, struct dentry *dentry,
int new)
{
dev_t rdev;
int retval;
umode_t umode;
struct inode *inode;
+ struct v9fs_inode *v9inode;
struct v9fs_session_info *v9ses = sb->s_fs_info;
int (*test)(struct inode *inode, void *data);
+ struct iget_data data = {
+ .st = st,
+ .dentry = dentry,
+ .need_double_check = false,
+ };
if (new)
test = v9fs_test_new_inode;
else
test = v9fs_test_inode;
- inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode, st);
+ if (dentry) {
+ /*
+ * If we need to compare paths to find the inode to reuse, we need
+ * to take the rename_sem for this FS. We need to take it here,
+ * instead of inside ino_path_compare, as iget5_locked has
+ * spinlock in it (inode_hash_lock)
+ */
+ down_read(&v9ses->rename_sem);
+ }
+ while (true) {
+ data.need_double_check = false;
+ inode = iget5_locked(sb, QID2INO(qid), test, v9fs_set_inode, &data);
+ if (!data.need_double_check)
+ break;
+ /*
+ * Need to double check path as it wasn't initialized yet when we
+ * tested it
+ */
+ if (!inode || (inode->i_state & I_NEW)) {
+ WARN_ONCE(
+ 1,
+ "Expected iget5_locked to return an existing inode");
+ break;
+ }
+ if (ino_path_compare(V9FS_I(inode)->path, dentry))
+ break;
+ iput(inode);
+ }
+ if (dentry)
+ up_read(&v9ses->rename_sem);
+
if (!inode)
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
@@ -437,6 +525,16 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
if (retval)
goto error;
+ v9inode = V9FS_I(inode);
+ if (dentry) {
+ down_read(&v9ses->rename_sem);
+ v9inode->path = make_ino_path(dentry);
+ up_read(&v9ses->rename_sem);
+ if (!v9inode->path) {
+ retval = -ENOMEM;
+ goto error;
+ }
+ }
v9fs_stat2inode(st, inode, sb, 0);
v9fs_set_netfs_context(inode);
v9fs_cache_inode_get_cookie(inode);
@@ -448,9 +546,18 @@ static struct inode *v9fs_qid_iget(struct super_block *sb,
}
-struct inode *
-v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
- struct super_block *sb, int new)
+/**
+ * Issues a getattr request and use the result to look up the inode for
+ * the target pointed to by @fid.
+ * @v9ses: session information
+ * @fid: fid to issue attribute request for
+ * @sb: superblock on which to create inode
+ * @dentry: if not NULL, the path of the provided dentry is compared
+ * against the path stored in the inode, to determine reuse eligibility.
+ */
+struct inode *v9fs_inode_from_fid(struct v9fs_session_info *v9ses,
+ struct p9_fid *fid, struct super_block *sb,
+ struct dentry *dentry, int new)
{
struct p9_wstat *st;
struct inode *inode = NULL;
@@ -459,7 +566,7 @@ v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
if (IS_ERR(st))
return ERR_CAST(st);
- inode = v9fs_qid_iget(sb, &st->qid, st, new);
+ inode = v9fs_qid_iget(sb, &st->qid, st, dentry, new);
p9stat_free(st);
kfree(st);
return inode;
@@ -608,18 +715,14 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir,
"p9_client_walk failed %d\n", err);
goto error;
}
- /*
- * Instantiate inode. On .L fs, pass in dentry for inodeident=path.
- */
- inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb,
- v9fs_proto_dotl(v9ses) ? dentry : NULL);
+ /* instantiate inode and assign the unopened fid to the dentry */
+ inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb, dentry);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS,
"inode creation failed %d\n", err);
goto error;
}
- /* Assign the unopened fid to the dentry */
v9fs_fid_add(dentry, &fid);
d_instantiate(dentry, inode);
}
@@ -1415,4 +1518,3 @@ static const struct inode_operations v9fs_symlink_inode_operations = {
.getattr = v9fs_vfs_getattr,
.setattr = v9fs_vfs_setattr,
};
-
--
2.51.0
^ permalink raw reply related
* [PATCH v2 4/7] fs/9p: .L: Refresh stale inodes on reuse
From: Tingmao Wang @ 2025-09-04 0:04 UTC (permalink / raw)
To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, Mickaël Salaün
Cc: Tingmao Wang, v9fs, Günther Noack, linux-security-module,
Jan Kara, Amir Goldstein, Matthew Bobrowski, Al Viro,
linux-fsdevel
In-Reply-To: <cover.1756935780.git.m@maowtm.org>
This uses the stat struct we already got as part of lookup process to
refresh the inode "for free".
Only for uncached mode for now. We will need to revisit this for cached
mode once we sort out reusing an old inode with changed qid.version.
(Currently this is not done in this series, which would be fine unless
server side change happens, which is not supposed to happen in cached mode
anyway)
Note that v9fs_test_inode_dotl already makes sure we don't reuse
inodes of the wrong type or different qid.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v1:
- Check cache bits instead of using `new` - uncached mode now also have
new=0.
fs/9p/vfs_inode_dotl.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 86adaf5bcc0e..d008e82256ac 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -164,6 +164,7 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
.dentry = dentry,
.need_double_check = false,
};
+ bool cached = v9ses->cache & (CACHE_META | CACHE_LOOSE);
if (new)
test = v9fs_test_new_inode_dotl;
@@ -203,8 +204,21 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
if (!inode)
return ERR_PTR(-ENOMEM);
- if (!(inode->i_state & I_NEW))
+ if (!(inode->i_state & I_NEW)) {
+ /*
+ * If we're returning an existing inode, we might as well refresh
+ * it with the metadata we just got. Refreshing the i_size also
+ * prevents read errors.
+ *
+ * We only do this for uncached mode, since in cached move, any
+ * change on the inode will bump qid.version, which will result in
+ * us getting a new inode in the first place. If we got an old
+ * inode, let's not touch it for now.
+ */
+ if (!cached)
+ v9fs_stat2inode_dotl(st, inode, 0);
return inode;
+ }
/*
* initialize the inode with the stat info
* FIXME!! we may need support for stale inodes
--
2.51.0
^ permalink raw reply related
* [PATCH v2 5/7] fs/9p: non-.L: Refresh stale inodes on reuse
From: Tingmao Wang @ 2025-09-04 0:04 UTC (permalink / raw)
To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, Mickaël Salaün
Cc: Tingmao Wang, v9fs, Günther Noack, linux-security-module,
Jan Kara, Amir Goldstein, Matthew Bobrowski, Al Viro,
linux-fsdevel
In-Reply-To: <cover.1756935780.git.m@maowtm.org>
This replicates the previous .L commit for non-.L
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
Changes since v1:
- Check cache bits instead of using `new` - uncached mode now also have
new=0.
fs/9p/vfs_inode.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 606760f966fd..4b4712eafe4d 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -473,6 +473,7 @@ static struct inode *v9fs_qid_iget(struct super_block *sb, struct p9_qid *qid,
.dentry = dentry,
.need_double_check = false,
};
+ bool cached = v9ses->cache & (CACHE_META | CACHE_LOOSE);
if (new)
test = v9fs_test_new_inode;
@@ -512,8 +513,12 @@ static struct inode *v9fs_qid_iget(struct super_block *sb, struct p9_qid *qid,
if (!inode)
return ERR_PTR(-ENOMEM);
- if (!(inode->i_state & I_NEW))
+ if (!(inode->i_state & I_NEW)) {
+ /* See explanation in v9fs_qid_iget_dotl */
+ if (!cached)
+ v9fs_stat2inode(st, inode, sb, 0);
return inode;
+ }
/*
* initialize the inode with the stat info
* FIXME!! we may need support for stale inodes
--
2.51.0
^ permalink raw reply related
* [PATCH v2 6/7] fs/9p: update the target's ino_path on rename
From: Tingmao Wang @ 2025-09-04 0:04 UTC (permalink / raw)
To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, Mickaël Salaün
Cc: Tingmao Wang, v9fs, Günther Noack, linux-security-module,
Jan Kara, Amir Goldstein, Matthew Bobrowski, Al Viro,
linux-fsdevel
In-Reply-To: <cover.1756935780.git.m@maowtm.org>
This makes it possible for the inode to "move along" to the new location
when a file under a inodeident=path 9pfs is moved, and it will be reused
on next access to the new location.
Modifying the ino_path of children when renaming a directory is currently
not handled. Renaming non-empty directories still work, but the children
won't have their the inodes be reused after renaming.
Inodes will also not be reused on server-side rename, since there is no
way for us to know about it. From our perspective this is
indistinguishable from a new file being created in the destination that
just happened to have the same qid, and the original file being deleted.
Signed-off-by: Tingmao Wang <m@maowtm.org>
Cc: "Mickaël Salaün" <mic@digikod.net>
Cc: "Günther Noack" <gnoack@google.com>
---
New patch in v2
fs/9p/ino_path.c | 3 ++-
fs/9p/v9fs.h | 3 +++
fs/9p/vfs_inode.c | 30 ++++++++++++++++++++++++++++++
fs/9p/vfs_inode_dotl.c | 6 ++++++
4 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/fs/9p/ino_path.c b/fs/9p/ino_path.c
index a03145e08a9d..ee4752b9f796 100644
--- a/fs/9p/ino_path.c
+++ b/fs/9p/ino_path.c
@@ -27,7 +27,8 @@ struct v9fs_ino_path *make_ino_path(struct dentry *dentry)
struct dentry *curr = dentry;
ssize_t i;
- lockdep_assert_held_read(&v9fs_dentry2v9ses(dentry)->rename_sem);
+ /* Either read or write lock held is ok */
+ lockdep_assert_held(&v9fs_dentry2v9ses(dentry)->rename_sem);
might_sleep(); /* Allocation below might block */
rcu_read_lock();
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index bacd0052e22c..c441fa8e757b 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -157,6 +157,9 @@ struct v9fs_inode {
/*
* Stores the path of the file this inode is for, only for filesystems
* with inode_ident=path. Lifetime is the same as this inode.
+ * Read/write to this pointer should be under the target v9fs's
+ * rename_sem to protect against races (except when initializing or
+ * freeing an inode, at which point nobody else has reference to us)
*/
struct v9fs_ino_path *path;
};
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 4b4712eafe4d..68a1837ff3dc 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -532,6 +532,12 @@ static struct inode *v9fs_qid_iget(struct super_block *sb, struct p9_qid *qid,
v9inode = V9FS_I(inode);
if (dentry) {
+ /*
+ * In order to make_ino_path, we need at least a read lock on the
+ * rename_sem. Since we re initializing a new inode, there is no
+ * risk of races with another task trying to write to
+ * v9inode->path, so we do not need an actual down_write.
+ */
down_read(&v9ses->rename_sem);
v9inode->path = make_ino_path(dentry);
up_read(&v9ses->rename_sem);
@@ -983,18 +989,21 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
{
int retval;
struct inode *old_inode;
+ struct v9fs_inode *old_v9inode;
struct inode *new_inode;
struct v9fs_session_info *v9ses;
struct p9_fid *oldfid = NULL, *dfid = NULL;
struct p9_fid *olddirfid = NULL;
struct p9_fid *newdirfid = NULL;
struct p9_wstat wstat;
+ struct v9fs_ino_path *new_ino_path = NULL;
if (flags)
return -EINVAL;
p9_debug(P9_DEBUG_VFS, "\n");
old_inode = d_inode(old_dentry);
+ old_v9inode = V9FS_I(old_inode);
new_inode = d_inode(new_dentry);
v9ses = v9fs_inode2v9ses(old_inode);
oldfid = v9fs_fid_lookup(old_dentry);
@@ -1022,6 +1031,17 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
}
down_write(&v9ses->rename_sem);
+ if (v9fs_inode_ident_path(v9ses)) {
+ /*
+ * Try to allocate this first, and don't actually do rename if
+ * allocation fails.
+ */
+ new_ino_path = make_ino_path(new_dentry);
+ if (!new_ino_path) {
+ retval = -ENOMEM;
+ goto error_locked;
+ }
+ }
if (v9fs_proto_dotl(v9ses)) {
retval = p9_client_renameat(olddirfid, old_dentry->d_name.name,
newdirfid, new_dentry->d_name.name);
@@ -1061,6 +1081,15 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
v9fs_invalidate_inode_attr(old_inode);
v9fs_invalidate_inode_attr(old_dir);
v9fs_invalidate_inode_attr(new_dir);
+ if (v9fs_inode_ident_path(v9ses)) {
+ /*
+ * We currently have rename_sem write lock, which protects all
+ * v9inode->path in this fs.
+ */
+ free_ino_path(old_v9inode->path);
+ old_v9inode->path = new_ino_path;
+ new_ino_path = NULL;
+ }
/* successful rename */
d_move(old_dentry, new_dentry);
@@ -1068,6 +1097,7 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
up_write(&v9ses->rename_sem);
error:
+ free_ino_path(new_ino_path);
p9_fid_put(newdirfid);
p9_fid_put(olddirfid);
p9_fid_put(oldfid);
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index d008e82256ac..a3f70dd422fb 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -232,6 +232,12 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
v9inode = V9FS_I(inode);
if (dentry) {
+ /*
+ * In order to make_ino_path, we need at least a read lock on the
+ * rename_sem. Since we re initializing a new inode, there is no
+ * risk of races with another task trying to write to
+ * v9inode->path, so we do not need an actual down_write.
+ */
down_read(&v9ses->rename_sem);
v9inode->path = make_ino_path(dentry);
up_read(&v9ses->rename_sem);
--
2.51.0
^ permalink raw reply related
* [PATCH v2 7/7] docs: fs/9p: Document the "inodeident" option
From: Tingmao Wang @ 2025-09-04 0:04 UTC (permalink / raw)
To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov,
Christian Schoenebeck, Mickaël Salaün
Cc: Tingmao Wang, v9fs, Günther Noack, linux-security-module,
Jan Kara, Amir Goldstein, Matthew Bobrowski, Al Viro,
linux-fsdevel
In-Reply-To: <cover.1756935780.git.m@maowtm.org>
Add a row for this option in the Options table.
Signed-off-by: Tingmao Wang <m@maowtm.org>
---
New patch in v2
Documentation/filesystems/9p.rst | 42 ++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/Documentation/filesystems/9p.rst b/Documentation/filesystems/9p.rst
index be3504ca034a..8b570a7ae698 100644
--- a/Documentation/filesystems/9p.rst
+++ b/Documentation/filesystems/9p.rst
@@ -238,6 +238,48 @@ Options
cachetag cache tag to use the specified persistent cache.
cache tags for existing cache sessions can be listed at
/sys/fs/9p/caches. (applies only to cache=fscache)
+
+ inodeident this setting controls how inodes work on this filesystem.
+ More specifically, how they are "reused". This is most
+ relevant when used with features like Landlock and
+ fanotify (in inode mark mode). These features rely on
+ holding a specific inode and identifying further access to
+ the same file (as identified by that inode).
+
+ There are 2 possible values:
+ qid
+ This is the default and the only possible
+ option if loose or metadata cache is
+ enabled. In this mode, 9pfs assumes that
+ the server will not present different
+ files with the same inode number, and will
+ use the presented inode number to lookup
+ inodes. For QEMU users, this can be
+ ensured by setting multidevs=remap. If
+ the server does present inode number
+ collisions, this may lead to unpredictable
+ behaviour when both files are accessed.
+ path
+ This is the default if neither loose nor
+ metadata cache bits are enabled. This
+ option causes 9pfs to internally track the
+ file path that an inode originated from,
+ and will only use an existing inode
+ (instead of allocating a new one) if the
+ path matches, even if the file's inode
+ number matches that of an existing inode.
+
+ .. note::
+ For inodeident=path, when a directory is renamed
+ or moved, inodeident=path mode currently does not
+ update its children's inodes to point to the new
+ path, and thus further access to them via the new
+ location will use newly allocated inodes, and
+ existing inode marks placed by Landlock and
+ fanotify on them will no longer work.
+
+ The inode path for the target being renamed itself
+ (not its children) *is* updated, however.
============= ===============================================================
Behavior
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v3 29/34] apparmor: move initcalls to the LSM framework
From: Paul Moore @ 2025-09-04 1:28 UTC (permalink / raw)
To: John Johansen
Cc: selinux, linux-integrity, linux-security-module, Mimi Zohar,
Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <1932305e-c6df-42c6-906b-d57364652242@canonical.com>
On Wed, Sep 3, 2025 at 7:15 PM John Johansen
<john.johansen@canonical.com> wrote:
> On 9/3/25 13:34, Paul Moore wrote:
> > On Thu, Aug 14, 2025 at 6:54 PM Paul Moore <paul@paul-moore.com> wrote:
> >>
> >> Reviewed-by: Kees Cook <kees@kernel.org>
> >> Signed-off-by: Paul Moore <paul@paul-moore.com>
> >> ---
> >> security/apparmor/apparmorfs.c | 4 +---
> >> security/apparmor/crypto.c | 3 +--
> >> security/apparmor/include/apparmorfs.h | 2 ++
> >> security/apparmor/include/crypto.h | 1 +
> >> security/apparmor/lsm.c | 9 ++++++++-
> >> 5 files changed, 13 insertions(+), 6 deletions(-)
> >
> > Thanks for reviewing all the other patches John. Assuming you are
> > okay with this patch, can I get an ACK?
>
> I'm working on it. I managed to get down to I think 3 patches remaining to review/ack, and I wanted to get some testing on this one before acking. Hopefully will get that done today
No worries, I just wanted to make sure it wasn't an oversight. Thanks
again for reviewing everything.
--
paul-moore.com
^ permalink raw reply
* Re: LSM namespacing API
From: Dr. Greg @ 2025-09-04 2:16 UTC (permalink / raw)
To: Casey Schaufler
Cc: John Johansen, Serge E. Hallyn, Stephen Smalley, Paul Moore,
linux-security-module, selinux
In-Reply-To: <3826d6c2-164b-415f-8bf4-63060ce428df@schaufler-ca.com>
On Mon, Sep 01, 2025 at 10:31:43AM -0700, Casey Schaufler wrote:
Hi, I hope mid-week has gone well for everyone.
> On 9/1/2025 9:01 AM, Dr. Greg wrote:
> > On Thu, Aug 21, 2025 at 07:57:11AM -0700, John Johansen wrote:
> >
> > Good morning, I hope the week is starting well for everyone.
> >
> > Now that everyone is getting past the summer holiday season, it would
> > seem useful to specifically clarify some of the LSM namespace
> > implementation details.
> >
> >> On 8/21/25 07:26, Serge E. Hallyn wrote:
> >>> On Thu, Aug 21, 2025 at 12:46:10AM -0700, John Johansen wrote:
> >>>> On 8/19/25 10:47, Stephen Smalley wrote:
> >>>>> On Tue, Aug 19, 2025 at 10:56???AM Paul Moore <paul@paul-moore.com>
> >>>>> wrote:
> >>>>>> Hello all,
> >>>>>>
> >>>>>> As most of you are likely aware, Stephen Smalley has been working on
> >>>>>> adding namespace support to SELinux, and the work has now progressed
> >>>>>> to the point where a serious discussion on the API is warranted. For
> >>>>>> those of you are unfamiliar with the details or Stephen's patchset, or
> >>>>>> simply need a refresher, he has some excellent documentation in his
> >>>>>> work-in-progress repo:
> >>>>>>
> >>>>>> * https://github.com/stephensmalley/selinuxns
> >>>>>>
> >>>>>> Stephen also gave a (pre-recorded) presentation at LSS-NA this year
> >>>>>> about SELinux namespacing, you can watch the presentation here:
> >>>>>>
> >>>>>> * https://www.youtube.com/watch?v=AwzGCOwxLoM
> >>>>>>
> >>>>>> In the past you've heard me state, rather firmly at times, that I
> >>>>>> believe namespacing at the LSM framework layer to be a mistake,
> >>>>>> although if there is something that can be done to help facilitate the
> >>>>>> namespacing of individual LSMs at the framework layer, I would be
> >>>>>> supportive of that. I think that a single LSM namespace API, similar
> >>>>>> to our recently added LSM syscalls, may be such a thing, so I'd like
> >>>>>> us to have a discussion to see if we all agree on that, and if so,
> >>>>>> what such an API might look like.
> >>>>>>
> >>>>>> At LSS-NA this year, John Johansen and I had a brief discussion where
> >>>>>> he suggested a single LSM wide clone*(2) flag that individual LSM's
> >>>>>> could opt into via callbacks. John is directly CC'd on this mail, so
> >>>>>> I'll let him expand on this idea.
> >>>>>>
> >>>>>> While I agree with John that a fs based API is problematic (see all of
> >>>>>> our discussions around the LSM syscalls), I'm concerned that a single
> >>>>>> clone*(2) flag will significantly limit our flexibility around how
> >>>>>> individual LSMs are namespaced, something I don't want to see happen.
> >>>>>> This makes me wonder about the potential for expanding
> >>>>>> lsm_set_self_attr(2) to support a new LSM attribute that would support
> >>>>>> a namespace "unshare" operation, e.g. LSM_ATTR_UNSHARE. This would
> >>>>>> provide a single LSM framework API for an unshare operation while also
> >>>>>> providing a mechanism to pass LSM specific via the lsm_ctx struct if
> >>>>>> needed. Just as we do with the other LSM_ATTR_* flags today,
> >>>>>> individual LSMs can opt-in to the API fairly easily by providing a
> >>>>>> setselfattr() LSM callback.
> >>>>>>
> >>>>>> Thoughts?
> >>>>> I think we want to be able to unshare a specific security module
> >>>>> namespace without unsharing the others, i.e. just SELinux or just
> >>>>> AppArmor.
> >>>> yes which is part of the problem with the single flag. That choice
> >>>> would be entirely at the policy level, without any input from userspace.
> >>> AIUI Paul's suggestion is the user can pre-set the details of which
> >>> lsms to unshare and how with the lsm_set_self_attr(), and then a
> >>> single CLONE_LSM effects that.
> >> yes, I was specifically addressing the conversation I had with Paul at
> >> LSS that Paul brought up. That is
> >>
> >> At LSS-NA this year, John Johansen and I had a brief discussion where
> >> he suggested a single LSM wide clone*(2) flag that individual LSM's
> >> could opt into via callbacks.
> >>
> >> the idea there isn't all that different than what Paul proposed. You
> >> could have a single flag, if you can provide ancillary information. But
> >> a single flag on its own isn't sufficient.
> > If one thing has come out of this thread, it would seem to be the fact
> > that there is going to be little commonality in the requirements that
> > various LSM's will have for the creation of a namespace.
> >
> > Given that, the most infrastructure that the LSM should provide would
> > be a common API for a resource orchestrator to request namespace
> > separation and to provide a framework for configuring the namespace
> > prior to when execution begins in the context of the namespace.
> >
> > The first issue to resolve would seem to be what namespace separation
> > implies.
> >
> > John, if I interpret your comments in this discussion correctly, your
> > contention is that when namespace separation is requested, all of the
> > LSM's that implement namespaces will create a subordinate namespace,
> > is that a correct assumption?
> >
> > It would seem, consistent with the 'stacking' concept, that any LSM
> > with namespace capability that chooses not to separate, will result in
> > denial of the separation request. That in turn will imply the need to
> > unwind or delete any namespace context that other LSM's may have
> > allocated before the refusal occurred.
> Were it true that 'stacking' rated the status of a 'concept'.
If 'concept' doesn't work as a term, we can call it an agreement on
the co-existence of multiple security models.
> An LSM that is capable of namespacing (the definition of which is
> elusive at this time) should be allowed to decline participation in
> a namespace creation.
Given the above, a full stop may be in order.
Perhaps, in pursuit of wisdom, we should call for a general consensus
among the group as to whether or not we have any clue as to what we
are doing?
> That, or there needs to be a convention for "null" namespaces, by
> which an LSM can pretend that it isn't involved in the new
> namespace. I think the latter smells funny and would invite
> "security people don't understand performance" remarks. No LSM
> should be allowed to prevent another from using namespaces.
Unfortunately that would seem to collide with the general consensus
that has evolved around 'stacking', as the means by which Linux
supports multiple LSM based security models/architectures.
The kernel security architecture admits to the notion that all of
the active LSM's have to agree that a specific security event be
allowed. If any LSM elects to deny a hook call, permission is denied
for the event.
John responded to our e-mail in this thread and clarified that he
doesn't believe that a POSIX 1e style capability for namespace
separation is required. However, our understanding from his reply is
that he felt that LSM namespace creation itself should have its own
LSM hook/event.
If this is the case, to be consistent with the stacking architecture,
any LSM should have the ability to deny security namespace creation
through its interpretation of the LSM namespace creation hook.
For example, it would certainly seem to be a valid concept for
something like an enhanced 'lockdown' mode to deny the ability for any
processes to escape into an LSM policy domain other than what was
configured when the platform was placed in a locked down status.
If we don't adhere to this model, we will have a 'snowflake' to
contend with in the LSM security model.
> > This model also implies that the orchestrator requesting the
> > separation will need to pass a set of parameters describing the
> > characteristics of each namespace, described by the LSM identifier
> > that they pertain to. Since there may be a need to configure multiple
> > namespaces there would be a requirement to pass an array or list of
> > these parameter sets.
> Just like lsm_set_self_attr(2).
That provides basic infrastructure, however, with concession to the
general acknowledgement that every LSM is different, the requirement
for every attribute to have a unique descriptive identity value may
prove restrictive, particularly in model based LSM's.
What may be needed is an agnostic attribute identifier that
orchestration software could use, in combination with the 'flags'
variable to specify exactly what type of attribute is being delivered
by the system call to an LSM. In other words, the attribute would
tell an LSM to interpret the flags value as an indicator of the
payload being delivered.
> > There will also be a need to inject, possibly substantial amounts of
> > policy or model information into the namespace, before execution in
> > the context of the namespace begins.
> Yup. A major downside of loadable policy.
Irregardless of merit, it will be reality, see below.
> > There will also be a need to decide whether namespace separation
> > should occur at the request of the orchestrator or at the next fork,
> > the latter model being what the other resource namespaces use. We
> > believe the argument for direct separation can be made by looking at
> > the gymnastics that orchestrators need to jump through with the
> > 'change-on-fork' model.
> >
> > Case in point, it would seem realistic that a process with sufficient
> > privilege, may desire to place itself in a new LSM namespace context
> > in a manner that does not require re-execution of itself.
> >
> > With respect to separation, the remaining issue is if a new security
> > capability bit needs to be implemented to gate namespace separation.
> > John, based on your comments, I believe you would support this need?
> I don't like the notion of a new capability for this. But then, I
> object to almost every new capability proposed. Existing namespaces
> don't need their own capabilities. I don't see this case as special.
It appears that John is thinking that an LSM hook is what will be
needed, so no new capability bit would be required.
That concept seems consistent with the precedence that was established
by using this type of scheme to control the creation of user
namespaces.
> >> You can do a subset with a single flag and only policy directing things,
> >> but that would cut container managers out of the decision. Without a
> >> universal container identifier that really limits what you can do. In
> >> another email I likend it to the MCS label approach to the container
> >> where you have a single security policy for the container and each
> >> container gets to be a unique instance of that policy. Its not a perfect
> >> analogy as with namespace policy can be loaded into the namespace making
> >> it unique. I don't think the approach is right because not all namespaces
> >> implement a loadable policy, and even when they do I think we can do a
> >> better job if the container manager is allowed to provide additional
> >> context with the namespacing request.
> > In order to be relevant, the configuration of LSM namespaces need to
> > be under control of a resource orchestrator or container manager.
> I do not approve of kernel features that are pointless without
> specific user space support. If it can't be used in ways other than
> those defined by a particular user space component they really don't
> belong in the kernel.
It appears you have already created the necessary infrastructure with
lsm_set_self_attr(2).
Given the apparent consensus that an LSM is free to implement
namespaces in whatever manner it pleases, an LSM can offer
configuration of an instance of its security namespace with an LSM
specific pseudo-filesystem interface.
If a centralized namespace separation is pursued, what will be
required is a method for loading policy/configuration before execution
starts in the context of the namespace.
> > What we hear from people doing Kubernetes, at scale, is a desire to be
> > able to request that a container be run somewhere in the hardware
> > resource pool and for that container to implement a security model
> > specific to the needs of the workload running in that container. In a
> > manner that is orthogonal from other security policies that may be in
> > effect for other workloads, on the host or in other containers.
> That sounds to me like they want per-container security policy. That
> would require that the kernel have the 'concept' of a
> container. That's not something I expect to see in my lifetime.
Per-container security policy is the expectation that will be raised
by the creation of LSM namespaces. We can speak very directly to that
fact, from conversations with groups that are running fleets of
thousands of virtual machines supporting tens of thousands of
container instances.
A 'container' is a set of kernel resource domains applied to an
execution workload. An LSM namespace will be another resource domain
that is placed around the workload by an orchestration system.
Speaking from personal implementation experience. If the LSM
namespace is entered and configured before the container runtime
engine is started, you have in effect, created a per container
security policy for that workload.
There are a plethora of issues surrounding this but it may be best to
leave those to further evolution of this discussion.
Have a good remainder of the week.
As always,
Dr. Greg
The Quixote Project - Flailing at the Travails of Cybersecurity
https://github.com/Quixote-Project
^ permalink raw reply
* Re: [PATCH v3 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Roberto Sassu @ 2025-09-04 8:12 UTC (permalink / raw)
To: John Johansen, Paul Moore, linux-security-module, linux-integrity,
selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <06a68323-b297-4be7-92eb-c2091207b9f0@canonical.com>
On Tue, 2025-09-02 at 10:20 -0700, John Johansen wrote:
> On 8/14/25 15:50, Paul Moore wrote:
> > The LSM currently has a lot of code to maintain a list of the currently
> > active LSMs in a human readable string, with the only user being the
> > "/sys/kernel/security/lsm" code. Let's drop all of that code and
> > generate the string on first use and then cache it for subsequent use.
> >
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > ---
> > include/linux/lsm_hooks.h | 1 -
> > security/inode.c | 59 +++++++++++++++++++++++++++++++++++++--
> > security/lsm_init.c | 49 --------------------------------
> > 3 files changed, 57 insertions(+), 52 deletions(-)
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 7343dd60b1d5..65a8227bece7 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -172,7 +172,6 @@ struct lsm_info {
> >
> >
> > /* DO NOT tamper with these variables outside of the LSM framework */
> > -extern char *lsm_names;
> > extern struct lsm_static_calls_table static_calls_table __ro_after_init;
> >
> > /**
> > diff --git a/security/inode.c b/security/inode.c
> > index 43382ef8896e..a5e7a073e672 100644
> > --- a/security/inode.c
> > +++ b/security/inode.c
> > @@ -22,6 +22,8 @@
> > #include <linux/lsm_hooks.h>
> > #include <linux/magic.h>
> >
> > +#include "lsm.h"
> > +
> > static struct vfsmount *mount;
> > static int mount_count;
> >
> > @@ -315,12 +317,65 @@ void securityfs_remove(struct dentry *dentry)
> > EXPORT_SYMBOL_GPL(securityfs_remove);
> >
> > #ifdef CONFIG_SECURITY
> > +#include <linux/spinlock.h>
> > +
> > static struct dentry *lsm_dentry;
> > +
> > +/* NOTE: we never free the string below once it is set. */
> > +static DEFINE_SPINLOCK(lsm_read_lock);
>
> nit, this is only used on the write side, so not the best name
>
> > +static char *lsm_read_str = NULL;
> > +static ssize_t lsm_read_len = 0;
> > +
> > static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
> > loff_t *ppos)
> > {
> > - return simple_read_from_buffer(buf, count, ppos, lsm_names,
> > - strlen(lsm_names));
> > + int i;
> > + char *str;
> > + ssize_t len;
> > +
> > +restart:
> > +
> > + rcu_read_lock();
Uhm, it seems we cannot use plain RCU here, simple_read_from_buffer()
can sleep.
Roberto
> > + if (!lsm_read_str) {
> should probably be
> if (!rcu_access_pointer(lsm_read_str)) {
>
> > + /* we need to generate the string and try again */
> > + rcu_read_unlock();
> > + goto generate_string;
> > + }
> > + len = simple_read_from_buffer(buf, count, ppos,
> > + rcu_dereference(lsm_read_str),
> > + lsm_read_len);
> > + rcu_read_unlock();
> > + return len;
> > +
> > +generate_string:
> > +
> > + for (i = 0; i < lsm_active_cnt; i++)
> > + /* the '+ 1' accounts for either a comma or a NUL */
> > + len += strlen(lsm_idlist[i]->name) + 1;
> > +
> > + str = kmalloc(len, GFP_KERNEL);
> > + if (!str)
> > + return -ENOMEM;
> > + str[0] = '\0';
> > +
> > + for (i = 0; i < lsm_active_cnt; i++) {
> > + if (i > 0)
> > + strcat(str, ",");
> > + strcat(str, lsm_idlist[i]->name);
> > + }
> > +
> > + spin_lock(&lsm_read_lock);
> > + if (lsm_read_str) {
> > + /* we raced and lost */
> > + spin_unlock(&lsm_read_lock);
> > + kfree(str);
> > + goto restart;
> > + }
> > + lsm_read_str = str;
> > + lsm_read_len = len - 1;
> > + spin_unlock(&lsm_read_lock);
> > +
> > + goto restart;
> > }
> >
> > static const struct file_operations lsm_ops = {
> > diff --git a/security/lsm_init.c b/security/lsm_init.c
> > index 9e495a36a332..87e2147016b3 100644
> > --- a/security/lsm_init.c
> > +++ b/security/lsm_init.c
> > @@ -10,8 +10,6 @@
> >
> > #include "lsm.h"
> >
> > -char *lsm_names;
> > -
> > /* Pointers to LSM sections defined in include/asm-generic/vmlinux.lds.h */
> > extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
> > extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
> > @@ -371,42 +369,6 @@ static void __init lsm_init_ordered(void)
> > }
> > }
> >
> > -static bool match_last_lsm(const char *list, const char *lsm)
> > -{
> > - const char *last;
> > -
> > - if (WARN_ON(!list || !lsm))
> > - return false;
> > - last = strrchr(list, ',');
> > - if (last)
> > - /* Pass the comma, strcmp() will check for '\0' */
> > - last++;
> > - else
> > - last = list;
> > - return !strcmp(last, lsm);
> > -}
> > -
> > -static int lsm_append(const char *new, char **result)
> > -{
> > - char *cp;
> > -
> > - if (*result == NULL) {
> > - *result = kstrdup(new, GFP_KERNEL);
> > - if (*result == NULL)
> > - return -ENOMEM;
> > - } else {
> > - /* Check if it is the last registered name */
> > - if (match_last_lsm(*result, new))
> > - return 0;
> > - cp = kasprintf(GFP_KERNEL, "%s,%s", *result, new);
> > - if (cp == NULL)
> > - return -ENOMEM;
> > - kfree(*result);
> > - *result = cp;
> > - }
> > - return 0;
> > -}
> > -
> > static void __init lsm_static_call_init(struct security_hook_list *hl)
> > {
> > struct lsm_static_call *scall = hl->scalls;
> > @@ -443,15 +405,6 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> > hooks[i].lsmid = lsmid;
> > lsm_static_call_init(&hooks[i]);
> > }
> > -
> > - /*
> > - * Don't try to append during early_security_init(), we'll come back
> > - * and fix this up afterwards.
> > - */
> > - if (slab_is_available()) {
> > - if (lsm_append(lsmid->name, &lsm_names) < 0)
> > - panic("%s - Cannot get early memory.\n", __func__);
> > - }
> > }
> >
> > int __init early_security_init(void)
> > @@ -488,8 +441,6 @@ int __init security_init(void)
> > lsm_early_for_each_raw(lsm) {
> > init_debug(" early started: %s (%s)\n", lsm->id->name,
> > is_enabled(lsm) ? "enabled" : "disabled");
> > - if (lsm->enabled)
> > - lsm_append(lsm->id->name, &lsm_names);
> > }
> >
> > /* Load LSMs in specified order. */
>
^ permalink raw reply
* Re: [PATCH v3 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: John Johansen @ 2025-09-04 8:48 UTC (permalink / raw)
To: Roberto Sassu, Paul Moore, linux-security-module, linux-integrity,
selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <dd03266930a7b219c590c54bb2c210366f8d89a1.camel@huaweicloud.com>
On 9/4/25 01:12, Roberto Sassu wrote:
> On Tue, 2025-09-02 at 10:20 -0700, John Johansen wrote:
>> On 8/14/25 15:50, Paul Moore wrote:
>>> The LSM currently has a lot of code to maintain a list of the currently
>>> active LSMs in a human readable string, with the only user being the
>>> "/sys/kernel/security/lsm" code. Let's drop all of that code and
>>> generate the string on first use and then cache it for subsequent use.
>>>
>>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>>> ---
>>> include/linux/lsm_hooks.h | 1 -
>>> security/inode.c | 59 +++++++++++++++++++++++++++++++++++++--
>>> security/lsm_init.c | 49 --------------------------------
>>> 3 files changed, 57 insertions(+), 52 deletions(-)
>>>
>>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>>> index 7343dd60b1d5..65a8227bece7 100644
>>> --- a/include/linux/lsm_hooks.h
>>> +++ b/include/linux/lsm_hooks.h
>>> @@ -172,7 +172,6 @@ struct lsm_info {
>>>
>>>
>>> /* DO NOT tamper with these variables outside of the LSM framework */
>>> -extern char *lsm_names;
>>> extern struct lsm_static_calls_table static_calls_table __ro_after_init;
>>>
>>> /**
>>> diff --git a/security/inode.c b/security/inode.c
>>> index 43382ef8896e..a5e7a073e672 100644
>>> --- a/security/inode.c
>>> +++ b/security/inode.c
>>> @@ -22,6 +22,8 @@
>>> #include <linux/lsm_hooks.h>
>>> #include <linux/magic.h>
>>>
>>> +#include "lsm.h"
>>> +
>>> static struct vfsmount *mount;
>>> static int mount_count;
>>>
>>> @@ -315,12 +317,65 @@ void securityfs_remove(struct dentry *dentry)
>>> EXPORT_SYMBOL_GPL(securityfs_remove);
>>>
>>> #ifdef CONFIG_SECURITY
>>> +#include <linux/spinlock.h>
>>> +
>>> static struct dentry *lsm_dentry;
>>> +
>>> +/* NOTE: we never free the string below once it is set. */
>>> +static DEFINE_SPINLOCK(lsm_read_lock);
>>
>> nit, this is only used on the write side, so not the best name
>>
>>> +static char *lsm_read_str = NULL;
>>> +static ssize_t lsm_read_len = 0;
>>> +
>>> static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
>>> loff_t *ppos)
>>> {
>>> - return simple_read_from_buffer(buf, count, ppos, lsm_names,
>>> - strlen(lsm_names));
>>> + int i;
>>> + char *str;
>>> + ssize_t len;
>>> +
>>> +restart:
>>> +
>>> + rcu_read_lock();
>
> Uhm, it seems we cannot use plain RCU here, simple_read_from_buffer()
> can sleep.
>
doh, yes.
But we shouldn't need RCU here either. This is a write once, never update
again situation. Instead we can get away with just a lock on the write
side to ensure exclusion on setting the value.
We don't even need the read side memory barrier, if the assumption that
the pointer is read as a single value holds, and the the null read will
go to the lock, and end up rereading.
> Roberto
>
>>> + if (!lsm_read_str) {
>> should probably be
>> if (!rcu_access_pointer(lsm_read_str)) {
>>
>>> + /* we need to generate the string and try again */
>>> + rcu_read_unlock();
>>> + goto generate_string;
>>> + }
>>> + len = simple_read_from_buffer(buf, count, ppos,
>>> + rcu_dereference(lsm_read_str),
>>> + lsm_read_len);
>>> + rcu_read_unlock();
>>> + return len;
>>> +
>>> +generate_string:
>>> +
>>> + for (i = 0; i < lsm_active_cnt; i++)
>>> + /* the '+ 1' accounts for either a comma or a NUL */
>>> + len += strlen(lsm_idlist[i]->name) + 1;
>>> +
>>> + str = kmalloc(len, GFP_KERNEL);
>>> + if (!str)
>>> + return -ENOMEM;
>>> + str[0] = '\0';
>>> +
>>> + for (i = 0; i < lsm_active_cnt; i++) {
>>> + if (i > 0)
>>> + strcat(str, ",");
>>> + strcat(str, lsm_idlist[i]->name);
>>> + }
>>> +
>>> + spin_lock(&lsm_read_lock);
>>> + if (lsm_read_str) {
>>> + /* we raced and lost */
>>> + spin_unlock(&lsm_read_lock);
>>> + kfree(str);
>>> + goto restart;
>>> + }
>>> + lsm_read_str = str;
>>> + lsm_read_len = len - 1;
>>> + spin_unlock(&lsm_read_lock);
>>> +
>>> + goto restart;
>>> }
>>>
>>> static const struct file_operations lsm_ops = {
>>> diff --git a/security/lsm_init.c b/security/lsm_init.c
>>> index 9e495a36a332..87e2147016b3 100644
>>> --- a/security/lsm_init.c
>>> +++ b/security/lsm_init.c
>>> @@ -10,8 +10,6 @@
>>>
>>> #include "lsm.h"
>>>
>>> -char *lsm_names;
>>> -
>>> /* Pointers to LSM sections defined in include/asm-generic/vmlinux.lds.h */
>>> extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
>>> extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
>>> @@ -371,42 +369,6 @@ static void __init lsm_init_ordered(void)
>>> }
>>> }
>>>
>>> -static bool match_last_lsm(const char *list, const char *lsm)
>>> -{
>>> - const char *last;
>>> -
>>> - if (WARN_ON(!list || !lsm))
>>> - return false;
>>> - last = strrchr(list, ',');
>>> - if (last)
>>> - /* Pass the comma, strcmp() will check for '\0' */
>>> - last++;
>>> - else
>>> - last = list;
>>> - return !strcmp(last, lsm);
>>> -}
>>> -
>>> -static int lsm_append(const char *new, char **result)
>>> -{
>>> - char *cp;
>>> -
>>> - if (*result == NULL) {
>>> - *result = kstrdup(new, GFP_KERNEL);
>>> - if (*result == NULL)
>>> - return -ENOMEM;
>>> - } else {
>>> - /* Check if it is the last registered name */
>>> - if (match_last_lsm(*result, new))
>>> - return 0;
>>> - cp = kasprintf(GFP_KERNEL, "%s,%s", *result, new);
>>> - if (cp == NULL)
>>> - return -ENOMEM;
>>> - kfree(*result);
>>> - *result = cp;
>>> - }
>>> - return 0;
>>> -}
>>> -
>>> static void __init lsm_static_call_init(struct security_hook_list *hl)
>>> {
>>> struct lsm_static_call *scall = hl->scalls;
>>> @@ -443,15 +405,6 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>>> hooks[i].lsmid = lsmid;
>>> lsm_static_call_init(&hooks[i]);
>>> }
>>> -
>>> - /*
>>> - * Don't try to append during early_security_init(), we'll come back
>>> - * and fix this up afterwards.
>>> - */
>>> - if (slab_is_available()) {
>>> - if (lsm_append(lsmid->name, &lsm_names) < 0)
>>> - panic("%s - Cannot get early memory.\n", __func__);
>>> - }
>>> }
>>>
>>> int __init early_security_init(void)
>>> @@ -488,8 +441,6 @@ int __init security_init(void)
>>> lsm_early_for_each_raw(lsm) {
>>> init_debug(" early started: %s (%s)\n", lsm->id->name,
>>> is_enabled(lsm) ? "enabled" : "disabled");
>>> - if (lsm->enabled)
>>> - lsm_append(lsm->id->name, &lsm_names);
>>> }
>>>
>>> /* Load LSMs in specified order. */
>>
>
^ permalink raw reply
* subscribe
From: Muneendra Kumar M @ 2025-09-04 8:57 UTC (permalink / raw)
To: linux-security-module
[-- Attachment #1.1: Type: text/plain, Size: 20 bytes --]
regards,
Muneendra.
[-- Attachment #1.2: Type: text/html, Size: 51 bytes --]
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4207 bytes --]
^ permalink raw reply
* Re: [PATCH v3 27/34] tomoyo: move initcalls to the LSM framework
From: Tetsuo Handa @ 2025-09-04 9:52 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, John Johansen, Mimi Zohar, Roberto Sassu,
Fan Wu, Mickaël Salaün, Günther Noack, Kees Cook,
Micah Morton, Casey Schaufler, Nicolas Bouchinet, Xiu Jianfeng,
linux-integrity, selinux
In-Reply-To: <CAHC9VhSf=zo0BkTc_=Qqq59OMVHsMUs_OqcmSUK501LYpAdMYg@mail.gmail.com>
On 2025/09/04 5:32, Paul Moore wrote:
> On Thu, Aug 14, 2025 at 6:54 PM Paul Moore <paul@paul-moore.com> wrote:
>>
>> Reviewed-by: Kees Cook <kees@kernel.org>
>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>> ---
>> security/tomoyo/common.h | 2 ++
>> security/tomoyo/securityfs_if.c | 4 +---
>> security/tomoyo/tomoyo.c | 1 +
>> 3 files changed, 4 insertions(+), 3 deletions(-)
>
> Tetsuo, does this look okay to you?
>
Yes.
^ permalink raw reply
* Re: [RFC PATCH] lsm,selinux: introduce LSM_ATTR_UNSHARE and wire it up for SELinux
From: Stephen Smalley @ 2025-09-04 14:41 UTC (permalink / raw)
To: selinux, linux-security-module
Cc: paul, omosnace, john.johansen, serge, casey
In-Reply-To: <20250903192426.215857-2-stephen.smalley.work@gmail.com>
On Wed, Sep 3, 2025 at 3:28 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> In the hopes of nudging the conversation in [1] in a more focused
> direction with a goal of getting SELinux namespaces upstreamed, this
> RFC provides a starting point for a concrete discussion. This is RFC
> only and has only been build-tested thus far.
>
> This defines a new LSM_ATTR_UNSHARE attribute for the
> lsm_set_self_attr(2) system call and wires it up for SELinux to invoke
> the underlying function for unsharing the SELinux namespace. As with
> the selinuxfs interface, this immediately unshares the SELinux
> namespace of the current process just like an unshare(2) system call
> would do for other namespaces. I have not yet explored the
> alternatives of deferring the unshare to the next unshare(2),
> clone(2), or execve(2) call and would want to first confirm that doing
> so does not introduce any issues in the kernel or make it harder to
> integrate with existing container runtimes.
>
> Differences between this syscall interface and the selinuxfs interface
> that need discussion before moving forward:
>
> 1. The syscall interface does not currently check any Linux capability
> or DAC permissions, whereas the selinuxfs interface can only be set by
> uid-0 or CAP_DAC_OVERRIDE processes. We need to decide what if any
> capability or DAC check should apply to this syscall interface and if
> any, add the checks to either the LSM framework code or to the SELinux
> hook function.
>
> Pros: Checking a capability or DAC permissions prevents misuse of this
> interface by unprivileged processes, particularly on systems with
> policies that do not yet define any of the new SELinux permissions
> introduced for controlling this operation. This is a potential concern
> on Linux distributions that do not tightly coordinate kernel updates
> with policy updates (or where users may choose to deploy upstream
> kernels on their own), but not on Android.
>
> Cons: Checking a capability or DAC permissions requires any process
> that uses this facility to have the corresponding capability or
> permissions, which might otherwise be unnecessary and create
> additional risks. This is less likely if we use a capability already
> required by container runtimes and similar components that might
> leverage this facility for unsharing SELinux namespaces.
>
> 2. The syscall interface checks a new SELinux unshare_selinuxns
> permission in the process2 class between the task SID and itself,
> similar to other checks for setting process attributes. This means
> that:
> allow domain self:process2 *; -or-
> allow domain self:process2 ~anything-other-than-unshare_selinuxns; -or-
> allow domain self:process2 unshare_selinuxns;
> would allow a process to unshare its SELinux namespace.
>
> The selinuxfs interface checks a new unshare permission in the
> security class between the task SID and the security initial SID,
> likewise similar to other checks for setting selinuxfs attributes.
> This means that:
> allow domain security_t:security *; -or-
> allow domain security_t:security ~anything-other-than-unshare; -or-
> allow domain security_t:security unshare;
> would allow a process to unshare its SELinux namespace.
>
> Technically, the selinuxfs interface also currently requires open and
> write access to the selinuxfs node; hence:
> allow domain security_t:file { open write };
> is also required for the selinuxfs interface.
>
> We need to decide what we want the SELinux check(s) to be for the
> syscall and whether it should be more like the former (process
> attributes) or more like the latter (security policy settings). Note
> that the permission name itself is unimportant here and only differs
> because it seemed less evident in the process2 class that we are
> talking about a SELinux namespace otherwise.
>
> Regardless, either form of allow rule can be prohibited in policies
> via neverallow rules on systems that enforce their usage
> (e.g. Android, not necessarily on Linux distributions).
>
> 3. The selinuxfs interface currently offers more functionality than I
> have implemented here for the sycall interface, including:
>
> a) the ability to read the selinuxfs node to see if your namespace has
> been unshared, which should be easily implementable via
> lsm_get_self_attr(2). However, questions remain as to when that
> should return 1 versus 0 (currently returns 1 whenever your namespace
> is NOT the initial SELinux namespace, useful for the testsuite to
> detect it is in a child, but could instead be reset to 0 by a
> subsequent policy load to indicate completion of the setup of the
> namespace, thus hiding from child processes that they are in a child
> namespace once its policy has been loaded).
>
> b) the abilities to get and set the maximum number of SELinux
> namespaces (via a /sys/fs/selinux/maxns node) and to get and set the
> maximum depth for SELinux namespaces (via a /sys/fs/selinux/maxnsdepth
> node). These could be left in selinuxfs or migrated to some other LSM
> management APIs since they are global in scope, not per-process
> attributes.
>
> Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> include/uapi/linux/lsm.h | 1 +
> security/selinux/hooks.c | 8 ++++++++
> security/selinux/include/classmap.h | 4 +++-
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> index 938593dfd5da..fb1b4a8aa639 100644
> --- a/include/uapi/linux/lsm.h
> +++ b/include/uapi/linux/lsm.h
> @@ -83,6 +83,7 @@ struct lsm_ctx {
> #define LSM_ATTR_KEYCREATE 103
> #define LSM_ATTR_PREV 104
> #define LSM_ATTR_SOCKCREATE 105
> +#define LSM_ATTR_UNSHARE 106
>
> /*
> * LSM_FLAG_XXX definitions identify special handling instructions
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c544b3e2fd5c..11b0b3c5b74a 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6812,6 +6812,10 @@ static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
> error = avc_has_perm(state, mysid, mysid, SECCLASS_PROCESS,
> PROCESS__SETCURRENT, NULL);
> break;
> + case LSM_ATTR_UNSHARE:
> + error = avc_has_perm(state, mysid, mysid, SECCLASS_PROCESS,
Ignore the obvious typo here (should be SECCLASS_PROCESS2 above). Will
be fixed before submission. Otherwise, the RFC remains the same.
> + PROCESS2__UNSHARE_SELINUXNS, NULL);
> + break;
> default:
> error = -EOPNOTSUPP;
> break;
> @@ -6923,6 +6927,10 @@ static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
> }
>
> tsec->sid = sid;
> + } else if (attr == LSM_ATTR_UNSHARE) {
> + error = selinux_state_create(new);
> + if (error)
> + goto abort_change;
> } else {
> error = -EINVAL;
> goto abort_change;
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index be52ebb6b94a..07fe316308cd 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -60,7 +60,9 @@ const struct security_class_mapping secclass_map[] = {
> "siginh", "setrlimit", "rlimitinh", "dyntransition",
> "setcurrent", "execmem", "execstack", "execheap",
> "setkeycreate", "setsockcreate", "getrlimit", NULL } },
> - { "process2", { "nnp_transition", "nosuid_transition", NULL } },
> + { "process2",
> + { "nnp_transition", "nosuid_transition", "unshare_selinuxns",
> + NULL } },
> { "system",
> { "ipc_info", "syslog_read", "syslog_mod", "syslog_console",
> "module_request", "module_load", "firmware_load",
> --
> 2.51.0
>
^ permalink raw reply
* Re: [PATCH v3 27/34] tomoyo: move initcalls to the LSM framework
From: Paul Moore @ 2025-09-04 15:02 UTC (permalink / raw)
To: Tetsuo Handa
Cc: linux-security-module, John Johansen, Mimi Zohar, Roberto Sassu,
Fan Wu, Mickaël Salaün, Günther Noack, Kees Cook,
Micah Morton, Casey Schaufler, Nicolas Bouchinet, Xiu Jianfeng,
linux-integrity, selinux
In-Reply-To: <3be8c5b7-a5d1-497d-8fbd-c74c1e22034f@I-love.SAKURA.ne.jp>
On Thu, Sep 4, 2025 at 5:53 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> On 2025/09/04 5:32, Paul Moore wrote:
> > On Thu, Aug 14, 2025 at 6:54 PM Paul Moore <paul@paul-moore.com> wrote:
> >>
> >> Reviewed-by: Kees Cook <kees@kernel.org>
> >> Signed-off-by: Paul Moore <paul@paul-moore.com>
> >> ---
> >> security/tomoyo/common.h | 2 ++
> >> security/tomoyo/securityfs_if.c | 4 +---
> >> security/tomoyo/tomoyo.c | 1 +
> >> 3 files changed, 4 insertions(+), 3 deletions(-)
> >
> > Tetsuo, does this look okay to you?
> >
>
> Yes.
Thanks for reviewing, may I add your ACK?
--
paul-moore.com
^ permalink raw reply
* Re: [syzbot] [kernel?] INFO: trying to register non-static key in skb_dequeue (4)
From: syzbot @ 2025-09-04 15:07 UTC (permalink / raw)
To: apparmor, audit, casey, davem, edumazet, eparis, eric.dumazet,
horms, jmorris, john.johansen, kuba, linux-kernel,
linux-security-module, luto, netdev, omosnace, pabeni, paul,
peterz, selinux, serge, stephen.smalley.work, syzkaller-bugs,
tglx
In-Reply-To: <68b93e3c.a00a0220.eb3d.0000.GAE@google.com>
syzbot has bisected this issue to:
commit eb59d494eebd4c5414728a35cdea6a0ba78ff26e
Author: Casey Schaufler <casey@schaufler-ca.com>
Date: Sat Aug 16 17:28:58 2025 +0000
audit: add record for multiple task security contexts
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1199fe62580000
start commit: 5d50cf9f7cf2 Add linux-next specific files for 20250903
git tree: linux-next
final oops: https://syzkaller.appspot.com/x/report.txt?x=1399fe62580000
console output: https://syzkaller.appspot.com/x/log.txt?x=1599fe62580000
kernel config: https://syzkaller.appspot.com/x/.config?x=7d2429dff5531d80
dashboard link: https://syzkaller.appspot.com/bug?extid=bb185b018a51f8d91fd2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15b9a312580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16819e62580000
Reported-by: syzbot+bb185b018a51f8d91fd2@syzkaller.appspotmail.com
Fixes: eb59d494eebd ("audit: add record for multiple task security contexts")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [syzbot] [kernel?] INFO: trying to register non-static key in skb_dequeue (4)
From: Paul Moore @ 2025-09-04 15:12 UTC (permalink / raw)
To: syzbot
Cc: apparmor, audit, casey, davem, edumazet, eparis, eric.dumazet,
horms, jmorris, john.johansen, kuba, linux-kernel,
linux-security-module, luto, netdev, omosnace, pabeni, peterz,
selinux, serge, stephen.smalley.work, syzkaller-bugs, tglx
In-Reply-To: <68b9ab18.050a0220.192772.0008.GAE@google.com>
On Thu, Sep 4, 2025 at 11:07 AM syzbot
<syzbot+bb185b018a51f8d91fd2@syzkaller.appspotmail.com> wrote:
>
> syzbot has bisected this issue to:
>
> commit eb59d494eebd4c5414728a35cdea6a0ba78ff26e
> Author: Casey Schaufler <casey@schaufler-ca.com>
> Date: Sat Aug 16 17:28:58 2025 +0000
>
> audit: add record for multiple task security contexts
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1199fe62580000
> start commit: 5d50cf9f7cf2 Add linux-next specific files for 20250903
> git tree: linux-next
> final oops: https://syzkaller.appspot.com/x/report.txt?x=1399fe62580000
> console output: https://syzkaller.appspot.com/x/log.txt?x=1599fe62580000
> kernel config: https://syzkaller.appspot.com/x/.config?x=7d2429dff5531d80
> dashboard link: https://syzkaller.appspot.com/bug?extid=bb185b018a51f8d91fd2
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15b9a312580000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16819e62580000
>
> Reported-by: syzbot+bb185b018a51f8d91fd2@syzkaller.appspotmail.com
> Fixes: eb59d494eebd ("audit: add record for multiple task security contexts")
>
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
The timing on this is amusing, I got the sysbot report just as I
merged a fix for this provided by Eric Dumazet :)
https://lore.kernel.org/audit/20250904072537.2278210-1-edumazet@google.com
The commit has the appropriate syzbot tags so this should close out
automatically.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Paul Moore @ 2025-09-04 15:18 UTC (permalink / raw)
To: John Johansen
Cc: Roberto Sassu, linux-security-module, linux-integrity, selinux,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <e92064a4-06c5-4913-917c-f9aca02378f3@canonical.com>
On Thu, Sep 4, 2025 at 4:48 AM John Johansen
<john.johansen@canonical.com> wrote:
> On 9/4/25 01:12, Roberto Sassu wrote:
> > On Tue, 2025-09-02 at 10:20 -0700, John Johansen wrote:
> >> On 8/14/25 15:50, Paul Moore wrote:
> >>> The LSM currently has a lot of code to maintain a list of the currently
> >>> active LSMs in a human readable string, with the only user being the
> >>> "/sys/kernel/security/lsm" code. Let's drop all of that code and
> >>> generate the string on first use and then cache it for subsequent use.
> >>>
> >>> Signed-off-by: Paul Moore <paul@paul-moore.com>
> >>> ---
> >>> include/linux/lsm_hooks.h | 1 -
> >>> security/inode.c | 59 +++++++++++++++++++++++++++++++++++++--
> >>> security/lsm_init.c | 49 --------------------------------
> >>> 3 files changed, 57 insertions(+), 52 deletions(-)
> >>>
> >>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> >>> index 7343dd60b1d5..65a8227bece7 100644
> >>> --- a/include/linux/lsm_hooks.h
> >>> +++ b/include/linux/lsm_hooks.h
> >>> @@ -172,7 +172,6 @@ struct lsm_info {
> >>>
> >>>
> >>> /* DO NOT tamper with these variables outside of the LSM framework */
> >>> -extern char *lsm_names;
> >>> extern struct lsm_static_calls_table static_calls_table __ro_after_init;
> >>>
> >>> /**
> >>> diff --git a/security/inode.c b/security/inode.c
> >>> index 43382ef8896e..a5e7a073e672 100644
> >>> --- a/security/inode.c
> >>> +++ b/security/inode.c
> >>> @@ -22,6 +22,8 @@
> >>> #include <linux/lsm_hooks.h>
> >>> #include <linux/magic.h>
> >>>
> >>> +#include "lsm.h"
> >>> +
> >>> static struct vfsmount *mount;
> >>> static int mount_count;
> >>>
> >>> @@ -315,12 +317,65 @@ void securityfs_remove(struct dentry *dentry)
> >>> EXPORT_SYMBOL_GPL(securityfs_remove);
> >>>
> >>> #ifdef CONFIG_SECURITY
> >>> +#include <linux/spinlock.h>
> >>> +
> >>> static struct dentry *lsm_dentry;
> >>> +
> >>> +/* NOTE: we never free the string below once it is set. */
> >>> +static DEFINE_SPINLOCK(lsm_read_lock);
> >>
> >> nit, this is only used on the write side, so not the best name
> >>
> >>> +static char *lsm_read_str = NULL;
> >>> +static ssize_t lsm_read_len = 0;
> >>> +
> >>> static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
> >>> loff_t *ppos)
> >>> {
> >>> - return simple_read_from_buffer(buf, count, ppos, lsm_names,
> >>> - strlen(lsm_names));
> >>> + int i;
> >>> + char *str;
> >>> + ssize_t len;
> >>> +
> >>> +restart:
> >>> +
> >>> + rcu_read_lock();
> >
> > Uhm, it seems we cannot use plain RCU here, simple_read_from_buffer()
> > can sleep.
>
> doh, yes.
D'oh indeed! Thanks for catching this.
> But we shouldn't need RCU here either. This is a write once, never update
> again situation. Instead we can get away with just a lock on the write
> side to ensure exclusion on setting the value.
>
> We don't even need the read side memory barrier, if the assumption that
> the pointer is read as a single value holds, and the the null read will
> go to the lock, and end up rereading.
It's funny you bring this up, my first draft of this function
(unposted) did just that, although I figured I'd add the RCU read side
protection ... "just in case".
I'll rework this function, but I'll hold off on posting another
revision until I hear back on some of the reviews that are still
pending in case additional edits are needed.
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH] selftests/landlock: Support running the full fs test suite on another FS
From: Mickaël Salaün @ 2025-09-04 15:11 UTC (permalink / raw)
To: Tingmao Wang
Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
Jann Horn
In-Reply-To: <20250830034753.186551-1-m@maowtm.org>
On Sat, Aug 30, 2025 at 11:47:51AM +0800, Tingmao Wang wrote:
> Adds a TMP_BIND_TO environment variable which the test binary will check,
> and if present, instead of mounting a tmpfs on ./tmp, it will bind mount
> that path to ./tmp instead.
>
> Currently there is the layout3_fs tests which runs a few tests (but not
> the full set of Landlock tests) in separate filesystems, notably no file
> creation/write/rename etc. This is necessary for certain special fs such
> as proc or sysfs, as the tests can only read a specific path. However,
> for a more typical fs like v9fs, this is limitting.
>
> This test makes it possible to run the full set on any filesystem (even
> though this is still not automated). Note that there are some expected
> failures, such as v9fs not supporting RENAME_EXCHANGE, as well as the
> known issue of ephemeral inodes, which may be fixed by a later revision of
> [1].
>
> Suggestions for alternatives welcome. Maybe we need to also detect the
> fs, and disable known-unsupported tests like RENAME_EXCHANGE?
Extending layout3_fs tests with new ones that make sense (i.e. those you
listed) would make be more consistent whithout scaling issue. We can
extend the fixture variants to skip some tests.
This patch should help:
https://lore.kernel.org/all/20250704171345.1393451-1-mic@digikod.net/
Feel free to take it with your 9pfs patch series and add a new patch
with extended checks.
>
> Link: https://lore.kernel.org/v9fs/cover.1743971855.git.m@maowtm.org/ [1]
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
> tools/testing/selftests/landlock/fs_test.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index fa0f18ec62c4..847be67fff9e 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -285,6 +285,22 @@ static const struct mnt_opt mnt_tmp = {
> .data = MNT_TMP_DATA,
> };
>
> +static struct mnt_opt get_tmp_mnt_opt(void)
> +{
> + const char *const tmp_bind_to = getenv("TMP_BIND_TO");
> +
> + if (tmp_bind_to) {
> + struct mnt_opt mnt = {
> + .flags = MS_BIND,
> + .source = tmp_bind_to
> + };
> +
> + return mnt;
> + }
> +
> + return mnt_tmp;
> +}
> +
> static int mount_opt(const struct mnt_opt *const mnt, const char *const target)
> {
> return mount(mnt->source ?: mnt->type, target, mnt->type, mnt->flags,
> @@ -322,7 +338,9 @@ static void prepare_layout_opt(struct __test_metadata *const _metadata,
>
> static void prepare_layout(struct __test_metadata *const _metadata)
> {
> - prepare_layout_opt(_metadata, &mnt_tmp);
> + struct mnt_opt mnt = get_tmp_mnt_opt();
> +
> + prepare_layout_opt(_metadata, &mnt);
> }
>
> static void cleanup_layout(struct __test_metadata *const _metadata)
>
> base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
> --
> 2.51.0
>
>
^ permalink raw reply
* Re: LSM namespacing API
From: Casey Schaufler @ 2025-09-04 17:40 UTC (permalink / raw)
To: Dr. Greg
Cc: John Johansen, Serge E. Hallyn, Stephen Smalley, Paul Moore,
linux-security-module, selinux, Casey Schaufler
In-Reply-To: <20250904021650.GA7191@wind.enjellic.com>
On 9/3/2025 7:16 PM, Dr. Greg wrote:
> On Mon, Sep 01, 2025 at 10:31:43AM -0700, Casey Schaufler wrote:
>
> Hi, I hope mid-week has gone well for everyone.
>
>> On 9/1/2025 9:01 AM, Dr. Greg wrote:
>>> On Thu, Aug 21, 2025 at 07:57:11AM -0700, John Johansen wrote:
>>>
>>> Good morning, I hope the week is starting well for everyone.
>>>
>>> Now that everyone is getting past the summer holiday season, it would
>>> seem useful to specifically clarify some of the LSM namespace
>>> implementation details.
>>>
>>>> On 8/21/25 07:26, Serge E. Hallyn wrote:
>>>>> On Thu, Aug 21, 2025 at 12:46:10AM -0700, John Johansen wrote:
>>>>>> On 8/19/25 10:47, Stephen Smalley wrote:
>>>>>>> On Tue, Aug 19, 2025 at 10:56???AM Paul Moore <paul@paul-moore.com>
>>>>>>> wrote:
>>>>>>>> Hello all,
>>>>>>>>
>>>>>>>> As most of you are likely aware, Stephen Smalley has been working on
>>>>>>>> adding namespace support to SELinux, and the work has now progressed
>>>>>>>> to the point where a serious discussion on the API is warranted. For
>>>>>>>> those of you are unfamiliar with the details or Stephen's patchset, or
>>>>>>>> simply need a refresher, he has some excellent documentation in his
>>>>>>>> work-in-progress repo:
>>>>>>>>
>>>>>>>> * https://github.com/stephensmalley/selinuxns
>>>>>>>>
>>>>>>>> Stephen also gave a (pre-recorded) presentation at LSS-NA this year
>>>>>>>> about SELinux namespacing, you can watch the presentation here:
>>>>>>>>
>>>>>>>> * https://www.youtube.com/watch?v=AwzGCOwxLoM
>>>>>>>>
>>>>>>>> In the past you've heard me state, rather firmly at times, that I
>>>>>>>> believe namespacing at the LSM framework layer to be a mistake,
>>>>>>>> although if there is something that can be done to help facilitate the
>>>>>>>> namespacing of individual LSMs at the framework layer, I would be
>>>>>>>> supportive of that. I think that a single LSM namespace API, similar
>>>>>>>> to our recently added LSM syscalls, may be such a thing, so I'd like
>>>>>>>> us to have a discussion to see if we all agree on that, and if so,
>>>>>>>> what such an API might look like.
>>>>>>>>
>>>>>>>> At LSS-NA this year, John Johansen and I had a brief discussion where
>>>>>>>> he suggested a single LSM wide clone*(2) flag that individual LSM's
>>>>>>>> could opt into via callbacks. John is directly CC'd on this mail, so
>>>>>>>> I'll let him expand on this idea.
>>>>>>>>
>>>>>>>> While I agree with John that a fs based API is problematic (see all of
>>>>>>>> our discussions around the LSM syscalls), I'm concerned that a single
>>>>>>>> clone*(2) flag will significantly limit our flexibility around how
>>>>>>>> individual LSMs are namespaced, something I don't want to see happen.
>>>>>>>> This makes me wonder about the potential for expanding
>>>>>>>> lsm_set_self_attr(2) to support a new LSM attribute that would support
>>>>>>>> a namespace "unshare" operation, e.g. LSM_ATTR_UNSHARE. This would
>>>>>>>> provide a single LSM framework API for an unshare operation while also
>>>>>>>> providing a mechanism to pass LSM specific via the lsm_ctx struct if
>>>>>>>> needed. Just as we do with the other LSM_ATTR_* flags today,
>>>>>>>> individual LSMs can opt-in to the API fairly easily by providing a
>>>>>>>> setselfattr() LSM callback.
>>>>>>>>
>>>>>>>> Thoughts?
>>>>>>> I think we want to be able to unshare a specific security module
>>>>>>> namespace without unsharing the others, i.e. just SELinux or just
>>>>>>> AppArmor.
>>>>>> yes which is part of the problem with the single flag. That choice
>>>>>> would be entirely at the policy level, without any input from userspace.
>>>>> AIUI Paul's suggestion is the user can pre-set the details of which
>>>>> lsms to unshare and how with the lsm_set_self_attr(), and then a
>>>>> single CLONE_LSM effects that.
>>>> yes, I was specifically addressing the conversation I had with Paul at
>>>> LSS that Paul brought up. That is
>>>>
>>>> At LSS-NA this year, John Johansen and I had a brief discussion where
>>>> he suggested a single LSM wide clone*(2) flag that individual LSM's
>>>> could opt into via callbacks.
>>>>
>>>> the idea there isn't all that different than what Paul proposed. You
>>>> could have a single flag, if you can provide ancillary information. But
>>>> a single flag on its own isn't sufficient.
>>> If one thing has come out of this thread, it would seem to be the fact
>>> that there is going to be little commonality in the requirements that
>>> various LSM's will have for the creation of a namespace.
>>>
>>> Given that, the most infrastructure that the LSM should provide would
>>> be a common API for a resource orchestrator to request namespace
>>> separation and to provide a framework for configuring the namespace
>>> prior to when execution begins in the context of the namespace.
>>>
>>> The first issue to resolve would seem to be what namespace separation
>>> implies.
>>>
>>> John, if I interpret your comments in this discussion correctly, your
>>> contention is that when namespace separation is requested, all of the
>>> LSM's that implement namespaces will create a subordinate namespace,
>>> is that a correct assumption?
>>>
>>> It would seem, consistent with the 'stacking' concept, that any LSM
>>> with namespace capability that chooses not to separate, will result in
>>> denial of the separation request. That in turn will imply the need to
>>> unwind or delete any namespace context that other LSM's may have
>>> allocated before the refusal occurred.
>> Were it true that 'stacking' rated the status of a 'concept'.
> If 'concept' doesn't work as a term, we can call it an agreement on
> the co-existence of multiple security models.
Sure.
>> An LSM that is capable of namespacing (the definition of which is
>> elusive at this time) should be allowed to decline participation in
>> a namespace creation.
> Given the above, a full stop may be in order.
>
> Perhaps, in pursuit of wisdom, we should call for a general consensus
> among the group as to whether or not we have any clue as to what we
> are doing?
That's the purpose of this thread, I believe. Now, whether we'll ever
get to true consensus seems unlikely, but I expect to see something
close enough that the wailing of those opposed will fail to prevent
acceptance.
>> That, or there needs to be a convention for "null" namespaces, by
>> which an LSM can pretend that it isn't involved in the new
>> namespace. I think the latter smells funny and would invite
>> "security people don't understand performance" remarks. No LSM
>> should be allowed to prevent another from using namespaces.
> Unfortunately that would seem to collide with the general consensus
> that has evolved around 'stacking', as the means by which Linux
> supports multiple LSM based security models/architectures.
I don't see that at all. For whatever reason, the developers of
namespaces chose to ignore the LSM infrastructure and the implications
their scheme has upon it. Managing the combination of differing
philosophies is often complex, and this is no exception.
> The kernel security architecture admits to the notion that all of
> the active LSM's have to agree that a specific security event be
> allowed. If any LSM elects to deny a hook call, permission is denied
> for the event.
call_void_hook()
> John responded to our e-mail in this thread and clarified that he
> doesn't believe that a POSIX 1e style capability for namespace
> separation is required. However, our understanding from his reply is
> that he felt that LSM namespace creation itself should have its own
> LSM hook/event.
>
> If this is the case, to be consistent with the stacking architecture,
> any LSM should have the ability to deny security namespace creation
> through its interpretation of the LSM namespace creation hook.
>
> For example, it would certainly seem to be a valid concept for
> something like an enhanced 'lockdown' mode to deny the ability for any
> processes to escape into an LSM policy domain other than what was
> configured when the platform was placed in a locked down status.
>
> If we don't adhere to this model, we will have a 'snowflake' to
> contend with in the LSM security model.
Again, call_void_hook()
>>> This model also implies that the orchestrator requesting the
>>> separation will need to pass a set of parameters describing the
>>> characteristics of each namespace, described by the LSM identifier
>>> that they pertain to. Since there may be a need to configure multiple
>>> namespaces there would be a requirement to pass an array or list of
>>> these parameter sets.
>> Just like lsm_set_self_attr(2).
> That provides basic infrastructure, however, with concession to the
> general acknowledgement that every LSM is different, the requirement
> for every attribute to have a unique descriptive identity value may
> prove restrictive, particularly in model based LSM's.
That was an argument made against the lsm_set_self_attr() interface
in the beginning. Even if lsm_set_self_attr() isn't the answer, it
provides a clue on how to formulate one.
> What may be needed is an agnostic attribute identifier that
> orchestration software could use, in combination with the 'flags'
> variable to specify exactly what type of attribute is being delivered
> by the system call to an LSM. In other words, the attribute would
> tell an LSM to interpret the flags value as an indicator of the
> payload being delivered.
That's what flags are for. Or did I miss something?
>>> There will also be a need to inject, possibly substantial amounts of
>>> policy or model information into the namespace, before execution in
>>> the context of the namespace begins.
>> Yup. A major downside of loadable policy.
> Irregardless of merit, it will be reality, see below.
s/Irregardless/Regardless/
"Irregardless" is not a word.
>>> There will also be a need to decide whether namespace separation
>>> should occur at the request of the orchestrator or at the next fork,
>>> the latter model being what the other resource namespaces use. We
>>> believe the argument for direct separation can be made by looking at
>>> the gymnastics that orchestrators need to jump through with the
>>> 'change-on-fork' model.
>>>
>>> Case in point, it would seem realistic that a process with sufficient
>>> privilege, may desire to place itself in a new LSM namespace context
>>> in a manner that does not require re-execution of itself.
>>>
>>> With respect to separation, the remaining issue is if a new security
>>> capability bit needs to be implemented to gate namespace separation.
>>> John, based on your comments, I believe you would support this need?
>> I don't like the notion of a new capability for this. But then, I
>> object to almost every new capability proposed. Existing namespaces
>> don't need their own capabilities. I don't see this case as special.
> It appears that John is thinking that an LSM hook is what will be
> needed, so no new capability bit would be required.
>
> That concept seems consistent with the precedence that was established
> by using this type of scheme to control the creation of user
> namespaces.
>
>>>> You can do a subset with a single flag and only policy directing things,
>>>> but that would cut container managers out of the decision. Without a
>>>> universal container identifier that really limits what you can do. In
>>>> another email I likend it to the MCS label approach to the container
>>>> where you have a single security policy for the container and each
>>>> container gets to be a unique instance of that policy. Its not a perfect
>>>> analogy as with namespace policy can be loaded into the namespace making
>>>> it unique. I don't think the approach is right because not all namespaces
>>>> implement a loadable policy, and even when they do I think we can do a
>>>> better job if the container manager is allowed to provide additional
>>>> context with the namespacing request.
>>> In order to be relevant, the configuration of LSM namespaces need to
>>> be under control of a resource orchestrator or container manager.
>> I do not approve of kernel features that are pointless without
>> specific user space support. If it can't be used in ways other than
>> those defined by a particular user space component they really don't
>> belong in the kernel.
> It appears you have already created the necessary infrastructure with
> lsm_set_self_attr(2).
>
> Given the apparent consensus that an LSM is free to implement
> namespaces in whatever manner it pleases, an LSM can offer
> configuration of an instance of its security namespace with an LSM
> specific pseudo-filesystem interface.
>
> If a centralized namespace separation is pursued, what will be
> required is a method for loading policy/configuration before execution
> starts in the context of the namespace.
Just so.
>>> What we hear from people doing Kubernetes, at scale, is a desire to be
>>> able to request that a container be run somewhere in the hardware
>>> resource pool and for that container to implement a security model
>>> specific to the needs of the workload running in that container. In a
>>> manner that is orthogonal from other security policies that may be in
>>> effect for other workloads, on the host or in other containers.
>> That sounds to me like they want per-container security policy. That
>> would require that the kernel have the 'concept' of a
>> container. That's not something I expect to see in my lifetime.
> Per-container security policy is the expectation that will be raised
> by the creation of LSM namespaces. We can speak very directly to that
> fact, from conversations with groups that are running fleets of
> thousands of virtual machines supporting tens of thousands of
> container instances.
All the more reason not to implement them at the LSM level. If
you can't meet expectations, the effort is futile.
> A 'container' is a set of kernel resource domains applied to an
> execution workload. An LSM namespace will be another resource domain
> that is placed around the workload by an orchestration system.
A 'container' is whatever the snake oil sales rep says it is.
Kata containers use virtual machines. Containers may be implemented
without an "orchestration system".
> Speaking from personal implementation experience. If the LSM
> namespace is entered and configured before the container runtime
> engine is started, you have in effect, created a per container
> security policy for that workload.
The base system policy will still be enforced. Having multiple policies
in place is tricky. What we can't have is a system where the base
policy is replaced rather than supplemented. It that not obvious?
> There are a plethora of issues surrounding this but it may be best to
> leave those to further evolution of this discussion.
>
> Have a good remainder of the week.
>
> As always,
> Dr. Greg
>
> The Quixote Project - Flailing at the Travails of Cybersecurity
> https://github.com/Quixote-Project
>
^ permalink raw reply
* Re: [PATCH v3 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Paul Moore @ 2025-09-04 17:52 UTC (permalink / raw)
To: John Johansen
Cc: Roberto Sassu, linux-security-module, linux-integrity, selinux,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <CAHC9VhQPmF-RCSUjZo-pe1+sWyw5ZGdnD7P0CWb7yXQQoo+92g@mail.gmail.com>
On Thu, Sep 4, 2025 at 11:18 AM Paul Moore <paul@paul-moore.com> wrote:
>
> I'll rework this function, but I'll hold off on posting another
> revision until I hear back on some of the reviews that are still
> pending in case additional edits are needed.
For the curious, here is what it looks like now:
diff --git a/security/inode.c b/security/inode.c
index 43382ef8896e..4813d116fd7c 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -22,6 +22,8 @@
#include <linux/lsm_hooks.h>
#include <linux/magic.h>
+#include "lsm.h"
+
static struct vfsmount *mount;
static int mount_count;
@@ -315,12 +317,49 @@ void securityfs_remove(struct dentry *dentry)
EXPORT_SYMBOL_GPL(securityfs_remove);
#ifdef CONFIG_SECURITY
+#include <linux/spinlock.h>
+
static struct dentry *lsm_dentry;
+
static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
loff_t *ppos)
{
- return simple_read_from_buffer(buf, count, ppos, lsm_names,
- strlen(lsm_names));
+ int i;
+ static char *str;
+ static size_t len;
+ static DEFINE_SPINLOCK(lock);
+
+ /* NOTE: we never free or modify the string once it is set */
+
+ if (unlikely(!str)) {
+ char *str_tmp;
+ size_t len_tmp = 0;
+
+ for (i = 0; i < lsm_active_cnt; i++)
+ /* the '+ 1' accounts for either a comma or a NUL */
+ len_tmp += strlen(lsm_idlist[i]->name) + 1;
+
+ str_tmp = kmalloc(len_tmp, GFP_KERNEL);
+ if (!str_tmp)
+ return -ENOMEM;
+ str_tmp[0] = '\0';
+
+ for (i = 0; i < lsm_active_cnt; i++) {
+ if (i > 0)
+ strcat(str_tmp, ",");
+ strcat(str_tmp, lsm_idlist[i]->name);
+ }
+
+ spin_lock(&lock);
+ if (!str) {
+ str = str_tmp;
+ len = len_tmp - 1;
+ } else
+ kfree(str_tmp);
+ spin_unlock(&lock);
+ }
+
+ return simple_read_from_buffer(buf, count, ppos, str, len);
}
--
paul-moore.com
^ permalink raw reply related
* Re: [PATCH net-next 1/8] ipv4: cipso: Simplify IP options handling in cipso_v4_error()
From: Paul Moore @ 2025-09-04 21:46 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, kuba, pabeni, edumazet, horms, dsahern, petrm,
linux-security-module
In-Reply-To: <20250901083027.183468-2-idosch@nvidia.com>
On Mon, Sep 1, 2025 at 4:32 AM Ido Schimmel <idosch@nvidia.com> wrote:
>
> When __ip_options_compile() is called with an skb, the IP options are
> parsed from the skb data into the provided IP option argument. This is
> in contrast to the case where the skb argument is NULL and the options
> are parsed from opt->__data.
>
> Given that cipso_v4_error() always passes an skb to
> __ip_options_compile(), there is no need to allocate an extra 40 bytes
> (maximum IP options size).
>
> Therefore, simplify the function by removing these extra bytes and make
> the function similar to ipv4_send_dest_unreach() which also calls both
> __ip_options_compile() and __icmp_send().
>
> This is a preparation for changing the arguments being passed to
> __icmp_send().
>
> No functional changes intended.
>
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
> net/ipv4/cipso_ipv4.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] ima: don't clear IMA_DIGSIG flag when setting non-IMA xattr
From: Mimi Zohar @ 2025-09-05 2:41 UTC (permalink / raw)
To: Coiby Xu, linux-integrity
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM,
open list
In-Reply-To: <20250902042515.759750-1-coxu@redhat.com>
On Tue, 2025-09-02 at 12:25 +0800, Coiby Xu wrote:
> Currently when both IMA and EVM are in fix mode, the IMA signature will
> be reset to IMA hash if a program first stores IMA signature in
> security.ima and then sets security.selinux for a file. For example, on
> Fedora, after booting the kernel with "ima_appraise=fix evm=fix
> ima_policy=appraise_tcb" and installing rpm-plugin-ima, reinstalling a
> package will not make good reference IMA signature generated. Instead
> IMA hash is generated,
> # getfattr -m - -d -e hex /usr/bin/bash
> # file: usr/bin/bash
> security.ima=0x0404...
>
> This happens because when setting selinux.selinux, the IMA_DIGSIG flag
> that had been set early was cleared. As a result, IMA hash is generated
> when the file is closed.
>
> Here's a minimal C reproducer,
>
> #include <stdio.h>
> #include <sys/xattr.h>
> #include <fcntl.h>
> #include <unistd.h>
> #include <string.h>
> #include <stdlib.h>
>
> int main() {
> const char* file_path = "/usr/sbin/test_binary";
> const char* hex_string = "030204d33204490066306402304";
> int length = strlen(hex_string);
> char* ima_attr_value;
> int fd;
>
> fd = open(file_path, O_WRONLY|O_CREAT|O_EXCL, 0644);
> if (fd == -1) {
> perror("Error opening file");
> return 1;
> }
>
> ima_attr_value = (char*)malloc(length / 2 );
> for (int i = 0, j = 0; i < length; i += 2, j++) {
> sscanf(hex_string + i, "%2hhx", &ima_attr_value[j]);
> }
>
> if (fsetxattr(fd, "security.ima", ima_attr_value, length/2, 0) == -1) {
> perror("Error setting extended attribute");
> close(fd);
> return 1;
> }
>
> const char* selinux_value= "system_u:object_r:bin_t:s0";
> if (fsetxattr(fd, "security.selinux", selinux_value, strlen(selinux_value), 0) == -1) {
> perror("Error setting extended attribute");
> close(fd);
> return 1;
> }
>
> close(fd);
>
> return 0;
> }
>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
Thanks, Coiby. Agreed, the ability to clear the IMA_DIGSIG flag should be
limited to security.ima xattr and probably security.evm xattr. Writing other
security xattrs should not affect the IMA_DIGSIG flag.
Even without an IMA appraise policy, the security xattrs are written out to the
filesystem, but the IMA_DIGSIG flag is not cached.
Please document the tristate values:
0: clear IMA_DIGSIG
1: set IMA_DIGSIG
-1: don't change IMA_DIGSIG
> ---
> security/integrity/ima/ima_appraise.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index f435eff4667f..fc82161f8b30 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -708,7 +708,7 @@ static void ima_reset_appraise_flags(struct inode *inode, int digsig)
> set_bit(IMA_CHANGE_XATTR, &iint->atomic_flags);
> if (digsig)
> set_bit(IMA_DIGSIG, &iint->atomic_flags);
This matches both -1 and 1. Test "digsig == 1" here.
> - else
> + else if (digsig != -1)
and test "digsig == 0" here.
> clear_bit(IMA_DIGSIG, &iint->atomic_flags);
> }
>
> @@ -794,6 +794,8 @@ static int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
> digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
> } else if (!strcmp(xattr_name, XATTR_NAME_EVM) && xattr_value_len > 0) {
> digsig = (xvalue->type == EVM_XATTR_PORTABLE_DIGSIG);
> + } else if (result != 1) {
The "if (result != 1)" test is redundant.
thanks,
Mimi
> + digsig = -1;
> }
> if (result == 1 || evm_revalidate_status(xattr_name)) {
> ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
^ permalink raw reply
* Re: LSM namespacing API
From: Dr. Greg @ 2025-09-05 22:14 UTC (permalink / raw)
To: John Johansen
Cc: Serge E. Hallyn, Stephen Smalley, Paul Moore,
linux-security-module, selinux
In-Reply-To: <fc3aadf1-9598-4fc2-bdb9-290df425b5d8@canonical.com>
On Tue, Sep 02, 2025 at 03:55:39AM -0700, John Johansen wrote:
Hi, I hope the week has gone well for everyone.
> On 9/1/25 09:01, Dr. Greg wrote:
> >On Thu, Aug 21, 2025 at 07:57:11AM -0700, John Johansen wrote:
> >
> >Good morning, I hope the week is starting well for everyone.
> >
> >Now that everyone is getting past the summer holiday season, it would
> >seem useful to specifically clarify some of the LSM namespace
> >implementation details.
> >
> >>On 8/21/25 07:26, Serge E. Hallyn wrote:
> >>>On Thu, Aug 21, 2025 at 12:46:10AM -0700, John Johansen wrote:
> >>>>On 8/19/25 10:47, Stephen Smalley wrote:
> >>>>>On Tue, Aug 19, 2025 at 10:56???AM Paul Moore <paul@paul-moore.com>
> >>>>>wrote:
> >>>>>>
> >>>>>>Hello all,
> >>>>>>
> >>>>>>As most of you are likely aware, Stephen Smalley has been working on
> >>>>>>adding namespace support to SELinux, and the work has now progressed
> >>>>>>to the point where a serious discussion on the API is warranted. For
> >>>>>>those of you are unfamiliar with the details or Stephen's patchset, or
> >>>>>>simply need a refresher, he has some excellent documentation in his
> >>>>>>work-in-progress repo:
> >>>>>>
> >>>>>>* https://github.com/stephensmalley/selinuxns
> >>>>>>
> >>>>>>Stephen also gave a (pre-recorded) presentation at LSS-NA this year
> >>>>>>about SELinux namespacing, you can watch the presentation here:
> >>>>>>
> >>>>>>* https://www.youtube.com/watch?v=AwzGCOwxLoM
> >>>>>>
> >>>>>>In the past you've heard me state, rather firmly at times, that I
> >>>>>>believe namespacing at the LSM framework layer to be a mistake,
> >>>>>>although if there is something that can be done to help facilitate the
> >>>>>>namespacing of individual LSMs at the framework layer, I would be
> >>>>>>supportive of that. I think that a single LSM namespace API, similar
> >>>>>>to our recently added LSM syscalls, may be such a thing, so I'd like
> >>>>>>us to have a discussion to see if we all agree on that, and if so,
> >>>>>>what such an API might look like.
> >>>>>>
> >>>>>>At LSS-NA this year, John Johansen and I had a brief discussion where
> >>>>>>he suggested a single LSM wide clone*(2) flag that individual LSM's
> >>>>>>could opt into via callbacks. John is directly CC'd on this mail, so
> >>>>>>I'll let him expand on this idea.
> >>>>>>
> >>>>>>While I agree with John that a fs based API is problematic (see all of
> >>>>>>our discussions around the LSM syscalls), I'm concerned that a single
> >>>>>>clone*(2) flag will significantly limit our flexibility around how
> >>>>>>individual LSMs are namespaced, something I don't want to see happen.
> >>>>>>This makes me wonder about the potential for expanding
> >>>>>>lsm_set_self_attr(2) to support a new LSM attribute that would support
> >>>>>>a namespace "unshare" operation, e.g. LSM_ATTR_UNSHARE. This would
> >>>>>>provide a single LSM framework API for an unshare operation while also
> >>>>>>providing a mechanism to pass LSM specific via the lsm_ctx struct if
> >>>>>>needed. Just as we do with the other LSM_ATTR_* flags today,
> >>>>>>individual LSMs can opt-in to the API fairly easily by providing a
> >>>>>>setselfattr() LSM callback.
> >>>>>>
> >>>>>>Thoughts?
> >>>>>
> >>>>>I think we want to be able to unshare a specific security module
> >>>>>namespace without unsharing the others, i.e. just SELinux or just
> >>>>>AppArmor.
> >>>>
> >>>>yes which is part of the problem with the single flag. That choice
> >>>>would be entirely at the policy level, without any input from userspace.
> >>>
> >>>AIUI Paul's suggestion is the user can pre-set the details of which
> >>>lsms to unshare and how with the lsm_set_self_attr(), and then a
> >>>single CLONE_LSM effects that.
> >
> >>yes, I was specifically addressing the conversation I had with Paul at
> >>LSS that Paul brought up. That is
> >>
> >> At LSS-NA this year, John Johansen and I had a brief discussion where
> >> he suggested a single LSM wide clone*(2) flag that individual LSM's
> >> could opt into via callbacks.
> >>
> >>the idea there isn't all that different than what Paul proposed. You
> >>could have a single flag, if you can provide ancillary information. But
> >>a single flag on its own isn't sufficient.
> >
> >If one thing has come out of this thread, it would seem to be the fact
> >that there is going to be little commonality in the requirements that
> >various LSM's will have for the creation of a namespace.
> yes
Given that and the conversations to date, the open question may be
whether there needs to be a common 'LSM namespace' infrastructure at
all or just punt everything to LSM's that choose to implement
namespaces.
> >Given that, the most infrastructure that the LSM should provide would
> >be a common API for a resource orchestrator to request namespace
> >separation and to provide a framework for configuring the namespace
> >prior to when execution begins in the context of the namespace.
> hrmmm, certainly a common API. Any task could theoretically use the API
> it doesn't have to be a resource orchestrator, but I suppose you could
> call it such.
No argument that any task could call for separation.
We seem to be dancing around the notion that the primary use, nee
demand, for a security namespace will be to allow container specific
security policies. In that scenario, the resource orchestrator or
container runtime will be what is requesting a specific security
model to be implemented in a namespace.
> I also dont know that we need to provide a framework for configuring
> the namespace prior to when execcution begins in the context of the
> namespace. It might be a nice to have, but configuring of LSMs is
> very LSM specific.
>
> We don't even have a common LSM policy load interface atm, though there
> is a proposal. Configuration is a step beyond that. Would it be nice
> to have, sure. Are we going to get that far, I don't know.
At least for model based LSM's, the configuration needs to occur
before execution within the namespace begins in order to avoid
possible races with respect to the security policy that gets effected.
Casey advocates for the use of lsm_set_self_attr(2), which has the
advantage of a common API and is probably sufficient if an LSM elects
to provide a generic management interface.
The system call is currently not namespace aware so the challenge will
be how to direct the configuration payload to the correct namespace.
Given that limitation, it seems highly probably that individual LSM's
will implement configuration/policy management via their various
pseudo-filesystem implementations that will grow awareness for the
namespace context that the commands are being issued for.
> >The first issue to resolve would seem to be what namespace separation
> >implies.
> >
> >John, if I interpret your comments in this discussion correctly, your
> >contention is that when namespace separation is requested, all of the
> >LSM's that implement namespaces will create a subordinate namespace,
> >is that a correct assumption?
> No, not necessarily. The task can request to "unshare/create" LSMs
> similar to requesting a set of system namespaces. Then every LSM,
> whether part of the request or not get to do their thing. If every
> LSM agrees, then a transition hook will process and each LSM will
> again do its thing. This would likely be what was requested but its
> possible that an LSM not in the request will do something, based on
> its model.
>
> In the end usespace gets to make a request, each security policy is
> responsible for staying withing its security model/policy.
This approach seems contrary to what Casey is advocating for in our
conversations, but perhaps we misunderstand what he is saying.
Casey indicated that no other LSM should be able to deny the ability
of another LSM to create a namespace.
As we noted in our exchange with him, this seems to violate the
current LSM model where all of the LSM's need to agree that an event
should be allowed, or it fails.
> >It would seem, consistent with the 'stacking' concept, that any LSM
> >with namespace capability that chooses not to separate, will result in
> >denial of the separation request. That in turn will imply the need to
> Not necessarily. They could allow and choose not to transition. Or
> they could not create a namespace but update some state.
> >unwind or delete any namespace context that other LSM's may have
> >allocated before the refusal occurred.
> The request does need to be split into a permission hook and a
> transition hook similar to exec. If any LSM in the permission hook
> denies, the request is denied. If any LSM in the transition hook
> fails again the request will fail, and the LSMs would get their
> regular clean up hook called for the object associated.
See above, the open question seems to be whether or not there is
agreement that any LSM can generically deny the creation of namespace
creation.
Again, we may misunderstand Casey on this issue.
> >This model also implies that the orchestrator requesting the
> >separation will need to pass a set of parameters describing the
> >characteristics of each namespace, described by the LSM identifier
> >that they pertain to. Since there may be a need to configure multiple
> >namespaces there would be a requirement to pass an array or list of
> >these parameter sets.
> yes it will require a list/array see lsm_set_self_attr(2)
Again, the issue is making this system call namespace aware.
> >There will also be a need to inject, possibly substantial amounts of
> >policy or model information into the namespace, before execution in
> >the context of the namespace begins.
> Allowing for this and requiring this are two different things. Like
> I said above we don't even currently have a common policy load
> interface. Configuration is another step beyond policy load.
It would seem the most straight forward path is to simply punt this to
the LSM's itself. If nothing else, it reduces the issues that
everyone needs to agree on.
> >There will also be a need to decide whether namespace separation
> >should occur at the request of the orchestrator or at the next fork,
> Or allow both, but yes a decision needs to be made
Again, allow both at the discretion of the LSM.
> >the latter model being what the other resource namespaces use. We
> >believe the argument for direct separation can be made by looking at
> >the gymnastics that orchestrators need to jump through with the
> >'change-on-fork' model.
> Looking at current system namespacing we have clone/unshare which
> really or on fork. setns enters existing namespaces.
>
> We either need to create new variants of clone/unshare or potentially
> have an LSM syscall that setups addition parameters that then are
> triggered by clone/unshare. If going the latter route then its just
> a matter whether the LSM call returns a handle that can be operated
> on or not.
We will find that current namespace semantics are challenging with
respect to being a good model for LSM namespaces.
Current namespaces focus on managing a single resource. In contrast,
as we have seen in our discussions, an 'LSM namespace' involves
multiple resources, each with their own specific requirements. On top
of that we have the complication of 'stacking' where anything that
happens will be the composite of what all the LSM's agree on, some of
which may be in the root namespace and some of which may be in
subordinate namespaces.
The notion of a process entering a security namespace, aka setns, will
be interesting. It would seem that this will require callbacks to
every LSM that is participating in the namespace. Presumably all of
the references to LSM security contexts will need to be suspended and
replaced with references to the context(s) for the security namespace
that is being entered.
With respect to managing this effectively, we would advocate for a
64-bit global counter that gets incremented on each successful LSM
namespace creation event. That would provide a unique handle for the
namespace that will never wrap.
> >Case in point, it would seem realistic that a process with sufficient
> >privilege, may desire to place itself in a new LSM namespace context
> >in a manner that does not require re-execution of itself.
> yes, but it is questionable whether security policy should allow that.
> At the very least security policy should be consulted and may deny
> it.
What we are talking about here is the need to support a process
requesting to run in an alternate LSM namespace without forking.
The question of whether this should be allowed will be regulated by
whatever composite security policy is operational, the same as would
be the case with the switch on fork model.
> >With respect to separation, the remaining issue is if a new security
> >capability bit needs to be implemented to gate namespace separation.
> >John, based on your comments, I believe you would support this need?
> No, I don't think a capability (as in posix.1e) per say is needed. I
> think an LSM permission request is.
Once again, that seems inconsistent with what Casey is advocating.
Although I'm sure he is happy that a new capability bit is not in the
offing... :-)
> >>You can do a subset with a single flag and only policy directing things,
> >>but that would cut container managers out of the decision. Without a
> >>universal container identifier that really limits what you can do. In
> >>another email I likend it to the MCS label approach to the container
> >>where you have a single security policy for the container and each
> >>container gets to be a unique instance of that policy. Its not a perfect
> >>analogy as with namespace policy can be loaded into the namespace making
> >>it unique. I don't think the approach is right because not all namespaces
> >>implement a loadable policy, and even when they do I think we can do a
> >>better job if the container manager is allowed to provide additional
> >>context with the namespacing request.
> >
> >In order to be relevant, the configuration of LSM namespaces need to
> >be under control of a resource orchestrator or container manager.
> No, the must be under the control of the LSMs.
I think we are talking past one another.
Configuration was perhaps a poor choice of vernacular, we were
referring to policy or model load.
As we mentioned in our exchange with Casey, the expection for all of
this from the user community will be to allow resource orchestrators
to run a workload under the constraints of a specific security policy.
Where policy should be probably plural.
Stephen even notes this on the slides that are linked from his GitHub
selinuxns site.
> >What we hear from people doing Kubernetes, at scale, is a desire to be
> >able to request that a container be run somewhere in the hardware
> >Resource pool and for that container to implement a security model
> >specific to the needs of the workload running in that container. In a
> >manner that is orthogonal from other security policies that may be in
> >effect for other workloads, on the host or in other containers.
> sure, assuming the host policy allows it. Otherwise it is just a host
> policy by-pass, which can not be allowed. K8s people have a specific
> use case, they need to configure the host for that use case. They can
> not expect that use case to work on host that has been configured
> for say an MLS security constraint.
Given that the concept of LSM stacking is overlaid on top of
namespaces, the result of all this will be security policies that will
be very interesting to reason about, particularly if multiple levels
of namespacing are allowed.
The other issue will be potential performance issues for LSM's that
choose to chase permissions all the way back up to the root namespace.
We've heard continuous suggestions that every pointer de-reference
is problematic from a performance perspective.
So, lots of issues to consider in all of this.
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: John Johansen @ 2025-09-06 2:01 UTC (permalink / raw)
To: Dr. Greg
Cc: Serge E. Hallyn, Stephen Smalley, Paul Moore,
linux-security-module, selinux
In-Reply-To: <20250905221456.GA1206@wind.enjellic.com>
On 9/5/25 15:14, Dr. Greg wrote:
> On Tue, Sep 02, 2025 at 03:55:39AM -0700, John Johansen wrote:
>
> Hi, I hope the week has gone well for everyone.
>
I wish, *sigh*
>> On 9/1/25 09:01, Dr. Greg wrote:
>>> On Thu, Aug 21, 2025 at 07:57:11AM -0700, John Johansen wrote:
>>>
>>> Good morning, I hope the week is starting well for everyone.
>>>
>>> Now that everyone is getting past the summer holiday season, it would
>>> seem useful to specifically clarify some of the LSM namespace
>>> implementation details.
>>>
>>>> On 8/21/25 07:26, Serge E. Hallyn wrote:
>>>>> On Thu, Aug 21, 2025 at 12:46:10AM -0700, John Johansen wrote:
>>>>>> On 8/19/25 10:47, Stephen Smalley wrote:
>>>>>>> On Tue, Aug 19, 2025 at 10:56???AM Paul Moore <paul@paul-moore.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hello all,
>>>>>>>>
>>>>>>>> As most of you are likely aware, Stephen Smalley has been working on
>>>>>>>> adding namespace support to SELinux, and the work has now progressed
>>>>>>>> to the point where a serious discussion on the API is warranted. For
>>>>>>>> those of you are unfamiliar with the details or Stephen's patchset, or
>>>>>>>> simply need a refresher, he has some excellent documentation in his
>>>>>>>> work-in-progress repo:
>>>>>>>>
>>>>>>>> * https://github.com/stephensmalley/selinuxns
>>>>>>>>
>>>>>>>> Stephen also gave a (pre-recorded) presentation at LSS-NA this year
>>>>>>>> about SELinux namespacing, you can watch the presentation here:
>>>>>>>>
>>>>>>>> * https://www.youtube.com/watch?v=AwzGCOwxLoM
>>>>>>>>
>>>>>>>> In the past you've heard me state, rather firmly at times, that I
>>>>>>>> believe namespacing at the LSM framework layer to be a mistake,
>>>>>>>> although if there is something that can be done to help facilitate the
>>>>>>>> namespacing of individual LSMs at the framework layer, I would be
>>>>>>>> supportive of that. I think that a single LSM namespace API, similar
>>>>>>>> to our recently added LSM syscalls, may be such a thing, so I'd like
>>>>>>>> us to have a discussion to see if we all agree on that, and if so,
>>>>>>>> what such an API might look like.
>>>>>>>>
>>>>>>>> At LSS-NA this year, John Johansen and I had a brief discussion where
>>>>>>>> he suggested a single LSM wide clone*(2) flag that individual LSM's
>>>>>>>> could opt into via callbacks. John is directly CC'd on this mail, so
>>>>>>>> I'll let him expand on this idea.
>>>>>>>>
>>>>>>>> While I agree with John that a fs based API is problematic (see all of
>>>>>>>> our discussions around the LSM syscalls), I'm concerned that a single
>>>>>>>> clone*(2) flag will significantly limit our flexibility around how
>>>>>>>> individual LSMs are namespaced, something I don't want to see happen.
>>>>>>>> This makes me wonder about the potential for expanding
>>>>>>>> lsm_set_self_attr(2) to support a new LSM attribute that would support
>>>>>>>> a namespace "unshare" operation, e.g. LSM_ATTR_UNSHARE. This would
>>>>>>>> provide a single LSM framework API for an unshare operation while also
>>>>>>>> providing a mechanism to pass LSM specific via the lsm_ctx struct if
>>>>>>>> needed. Just as we do with the other LSM_ATTR_* flags today,
>>>>>>>> individual LSMs can opt-in to the API fairly easily by providing a
>>>>>>>> setselfattr() LSM callback.
>>>>>>>>
>>>>>>>> Thoughts?
>>>>>>>
>>>>>>> I think we want to be able to unshare a specific security module
>>>>>>> namespace without unsharing the others, i.e. just SELinux or just
>>>>>>> AppArmor.
>>>>>>
>>>>>> yes which is part of the problem with the single flag. That choice
>>>>>> would be entirely at the policy level, without any input from userspace.
>>>>>
>>>>> AIUI Paul's suggestion is the user can pre-set the details of which
>>>>> lsms to unshare and how with the lsm_set_self_attr(), and then a
>>>>> single CLONE_LSM effects that.
>>>
>>>> yes, I was specifically addressing the conversation I had with Paul at
>>>> LSS that Paul brought up. That is
>>>>
>>>> At LSS-NA this year, John Johansen and I had a brief discussion where
>>>> he suggested a single LSM wide clone*(2) flag that individual LSM's
>>>> could opt into via callbacks.
>>>>
>>>> the idea there isn't all that different than what Paul proposed. You
>>>> could have a single flag, if you can provide ancillary information. But
>>>> a single flag on its own isn't sufficient.
>>>
>>> If one thing has come out of this thread, it would seem to be the fact
>>> that there is going to be little commonality in the requirements that
>>> various LSM's will have for the creation of a namespace.
>
>> yes
>
> Given that and the conversations to date, the open question may be
> whether there needs to be a common 'LSM namespace' infrastructure at
> all or just punt everything to LSM's that choose to implement
> namespaces.
>
>>> Given that, the most infrastructure that the LSM should provide would
>>> be a common API for a resource orchestrator to request namespace
>>> separation and to provide a framework for configuring the namespace
>>> prior to when execution begins in the context of the namespace.
>
>> hrmmm, certainly a common API. Any task could theoretically use the API
>> it doesn't have to be a resource orchestrator, but I suppose you could
>> call it such.
>
> No argument that any task could call for separation.
>
> We seem to be dancing around the notion that the primary use, nee
> demand, for a security namespace will be to allow container specific
> security policies. In that scenario, the resource orchestrator or
> container runtime will be what is requesting a specific security
> model to be implemented in a namespace.
>
no that is one use of them.
AppArmor is using namespaces for sub-confinement/priv sep They are also
used for tiered policy restrictions, and global black listing, and
unprivileged user and application policy.
>> I also dont know that we need to provide a framework for configuring
>> the namespace prior to when execcution begins in the context of the
>> namespace. It might be a nice to have, but configuring of LSMs is
>> very LSM specific.
>>
>> We don't even have a common LSM policy load interface atm, though there
>> is a proposal. Configuration is a step beyond that. Would it be nice
>> to have, sure. Are we going to get that far, I don't know.
>
> At least for model based LSM's, the configuration needs to occur
> before execution within the namespace begins in order to avoid
> possible races with respect to the security policy that gets effected.
>
depends on what you mean by configuration. There might be some config
of the namespace, but policy doesn't necessarily need to be loaded.
In both the unprivileged user and unprivileged application policy
cases, policy needs to be loaded after the the namespace is entered.
You can even split the container/orchastrator case, where LXD emulating
a system, will want to load the system policy as part of the OS boot
processes.
Where the docker/k8s/sandboxing use case have an orchestrator sandbox
app setup policy before hand.
> Casey advocates for the use of lsm_set_self_attr(2), which has the
> advantage of a common API and is probably sufficient if an LSM elects
> to provide a generic management interface.
>
yeah that or something similar seems to be the way to go
> The system call is currently not namespace aware so the challenge will
> be how to direct the configuration payload to the correct namespace.
>
yes
> Given that limitation, it seems highly probably that individual LSM's
> will implement configuration/policy management via their various
> pseudo-filesystem implementations that will grow awareness for the
> namespace context that the commands are being issued for.
>
possible. But ideally if we get it right they can expand the syscall
instead of an fs interface.
An fs interface has lots of problems like needing to be available within
a given namespace. If we want to be nesting namespaces (which we do),
then mounting custom FSes into the namespace is extra setup, and things
like proc may not even be available, depending on how the container is
being setup.
>>> The first issue to resolve would seem to be what namespace separation
>>> implies.
>>>
>>> John, if I interpret your comments in this discussion correctly, your
>>> contention is that when namespace separation is requested, all of the
>>> LSM's that implement namespaces will create a subordinate namespace,
>>> is that a correct assumption?
>
>> No, not necessarily. The task can request to "unshare/create" LSMs
>> similar to requesting a set of system namespaces. Then every LSM,
>> whether part of the request or not get to do their thing. If every
>> LSM agrees, then a transition hook will process and each LSM will
>> again do its thing. This would likely be what was requested but its
>> possible that an LSM not in the request will do something, based on
>> its model.
>>
>> In the end usespace gets to make a request, each security policy is
>> responsible for staying withing its security model/policy.
>
> This approach seems contrary to what Casey is advocating for in our
> conversations, but perhaps we misunderstand what he is saying.
>
Maybe, its not what I am getting from him, but I could be misunderstanding
as well.
> Casey indicated that no other LSM should be able to deny the ability
> of another LSM to create a namespace.
>
correct, at least in isolation. However if it is tied to other namespace
creation, say at clone/unshare, an LSM should be able to deny that and
have the whole set fail.
That is an individual LSM can deny the creation of other non-LSM
namespaces that are happening at the same time. This may affect the
creation of other LSM namespaces, but any given individual LSM is
not denying another LSM from creating a namespace.
> As we noted in our exchange with him, this seems to violate the
> current LSM model where all of the LSM's need to agree that an event
> should be allowed, or it fails.
>
there is good reason for it. Experience has shown forcing each LSM to
update policy for the policy of another LSM is problematic. Allowing
each LSM to manage itself based on its own policy while the rest of
the events are allow or fail, is very practical.
>>> It would seem, consistent with the 'stacking' concept, that any LSM
>>> with namespace capability that chooses not to separate, will result in
>>> denial of the separation request. That in turn will imply the need to
>
>> Not necessarily. They could allow and choose not to transition. Or
>> they could not create a namespace but update some state.
>
>>> unwind or delete any namespace context that other LSM's may have
>>> allocated before the refusal occurred.
>
>> The request does need to be split into a permission hook and a
>> transition hook similar to exec. If any LSM in the permission hook
>> denies, the request is denied. If any LSM in the transition hook
>> fails again the request will fail, and the LSMs would get their
>> regular clean up hook called for the object associated.
>
> See above, the open question seems to be whether or not there is
> agreement that any LSM can generically deny the creation of namespace
> creation.
>
> Again, we may misunderstand Casey on this issue.
>
Its not about what an individual LSM is allowed but what is happening
at the system level. If system events are moving with the LSM event
the system event is fair game.
Even if we are talking individual LSM updates a two hook model may be
needed when taking into account the constraints of creds, and non-LSM
permission checks.
>>> This model also implies that the orchestrator requesting the
>>> separation will need to pass a set of parameters describing the
>>> characteristics of each namespace, described by the LSM identifier
>>> that they pertain to. Since there may be a need to configure multiple
>>> namespaces there would be a requirement to pass an array or list of
>>> these parameter sets.
>
>> yes it will require a list/array see lsm_set_self_attr(2)
>
> Again, the issue is making this system call namespace aware.
>
sure or another similar syscall. I don't think we are saying that it
has to be lsm_set_self_attr. More that it provides an example of how
to do this. It could be that it can be extended, it could be it turns
out that doing a new call that is similar but meets the constraints
is needed.
>>> There will also be a need to inject, possibly substantial amounts of
>>> policy or model information into the namespace, before execution in
>>> the context of the namespace begins.
>
>> Allowing for this and requiring this are two different things. Like
>> I said above we don't even currently have a common policy load
>> interface. Configuration is another step beyond policy load.
>
> It would seem the most straight forward path is to simply punt this to
> the LSM's itself. If nothing else, it reduces the issues that
> everyone needs to agree on.
>
Yes, configuration requirements are definitely a per LSM thing.
>>> There will also be a need to decide whether namespace separation
>>> should occur at the request of the orchestrator or at the next fork,
>
>> Or allow both, but yes a decision needs to be made
>
> Again, allow both at the discretion of the LSM.
>
sure
>>> the latter model being what the other resource namespaces use. We
>>> believe the argument for direct separation can be made by looking at
>>> the gymnastics that orchestrators need to jump through with the
>>> 'change-on-fork' model.
>
>> Looking at current system namespacing we have clone/unshare which
>> really or on fork. setns enters existing namespaces.
>>
>> We either need to create new variants of clone/unshare or potentially
>> have an LSM syscall that setups addition parameters that then are
>> triggered by clone/unshare. If going the latter route then its just
>> a matter whether the LSM call returns a handle that can be operated
>> on or not.
>
> We will find that current namespace semantics are challenging with
> respect to being a good model for LSM namespaces.
>
> Current namespaces focus on managing a single resource. In contrast,
> as we have seen in our discussions, an 'LSM namespace' involves
> multiple resources, each with their own specific requirements. On top
> of that we have the complication of 'stacking' where anything that
> happens will be the composite of what all the LSM's agree on, some of
> which may be in the root namespace and some of which may be in
> subordinate namespaces.
>
its easy to see why people call security people crazy :)
> The notion of a process entering a security namespace, aka setns, will
> be interesting. It would seem that this will require callbacks to
> every LSM that is participating in the namespace. Presumably all of
> the references to LSM security contexts will need to be suspended and
> replaced with references to the context(s) for the security namespace
> that is being entered.
>
yes setns from a security pov is problematic.
> With respect to managing this effectively, we would advocate for a
> 64-bit global counter that gets incremented on each successful LSM
> namespace creation event. That would provide a unique handle for the
> namespace that will never wrap.
>
uhmmm, a unique container id? Well I guess that is one way to guarantee
this will never happen.
>>> Case in point, it would seem realistic that a process with sufficient
>>> privilege, may desire to place itself in a new LSM namespace context
>>> in a manner that does not require re-execution of itself.
>
>> yes, but it is questionable whether security policy should allow that.
>> At the very least security policy should be consulted and may deny
>> it.
>
> What we are talking about here is the need to support a process
> requesting to run in an alternate LSM namespace without forking.
>
sure, I support allowing a process to ask
> The question of whether this should be allowed will be regulated by
> whatever composite security policy is operational, the same as would
> be the case with the switch on fork model.
>
>>> With respect to separation, the remaining issue is if a new security
>>> capability bit needs to be implemented to gate namespace separation.
>>> John, based on your comments, I believe you would support this need?
>
>> No, I don't think a capability (as in posix.1e) per say is needed. I
>> think an LSM permission request is.
>
> Once again, that seems inconsistent with what Casey is advocating.
>
> Although I'm sure he is happy that a new capability bit is not in the
> offing... :-)
>
not at all. I think the distinction is the LSM hook is asking the LSM
that is being asked to be namespaced. That is each LSM is consulted about
itself.
>>>> You can do a subset with a single flag and only policy directing things,
>>>> but that would cut container managers out of the decision. Without a
>>>> universal container identifier that really limits what you can do. In
>>>> another email I likend it to the MCS label approach to the container
>>>> where you have a single security policy for the container and each
>>>> container gets to be a unique instance of that policy. Its not a perfect
>>>> analogy as with namespace policy can be loaded into the namespace making
>>>> it unique. I don't think the approach is right because not all namespaces
>>>> implement a loadable policy, and even when they do I think we can do a
>>>> better job if the container manager is allowed to provide additional
>>>> context with the namespacing request.
>>>
>>> In order to be relevant, the configuration of LSM namespaces need to
>>> be under control of a resource orchestrator or container manager.
>
>> No, the must be under the control of the LSMs.
>
> I think we are talking past one another.
>
quite possibly
> Configuration was perhaps a poor choice of vernacular, we were
> referring to policy or model load.
>
which is one part of configuration. Its conceivable that an LSM could
have nobs to turn beyond policy
> As we mentioned in our exchange with Casey, the expection for all of
> this from the user community will be to allow resource orchestrators
> to run a workload under the constraints of a specific security policy.
>
sure that is the expectation of the container community. Its just not
the only use.
> Where policy should be probably plural.
>
> Stephen even notes this on the slides that are linked from his GitHub
> selinuxns site.
>
>>> What we hear from people doing Kubernetes, at scale, is a desire to be
>>> able to request that a container be run somewhere in the hardware
>>> Resource pool and for that container to implement a security model
>>> specific to the needs of the workload running in that container. In a
>>> manner that is orthogonal from other security policies that may be in
>>> effect for other workloads, on the host or in other containers.
>
>> sure, assuming the host policy allows it. Otherwise it is just a host
>> policy by-pass, which can not be allowed. K8s people have a specific
>> use case, they need to configure the host for that use case. They can
>> not expect that use case to work on host that has been configured
>> for say an MLS security constraint.
>
> Given that the concept of LSM stacking is overlaid on top of
> namespaces, the result of all this will be security policies that will
> be very interesting to reason about, particularly if multiple levels
> of namespacing are allowed.
>
"interesting"*TM* indeed
> The other issue will be potential performance issues for LSM's that
> choose to chase permissions all the way back up to the root namespace.
> We've heard continuous suggestions that every pointer de-reference
> is problematic from a performance perspective.
>
oh it is, the perforamance people can get snippy about just a few
cycles. Ultimately that is just the cost of stacking policy. The
more layers you add the higher the cost.
AppArmor is already working towards a jit of policy that will be able
to flatten stacked policy, so the cost is can be pushed back to the
same as non-stacked. That however comes with the cost of increased
memory use, and it will only deal with the AppArmor part of the
whole stack.
> So, lots of issues to consider in all of this.
>
> 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: [PATCH v3 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Tetsuo Handa @ 2025-09-07 7:35 UTC (permalink / raw)
To: Paul Moore, John Johansen
Cc: Roberto Sassu, linux-security-module, linux-integrity, selinux,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <CAHC9VhRjQrjvsn65A-TGKKGrVFjZdnPBu+1vp=7w86SOjoyiUw@mail.gmail.com>
On 2025/09/05 2:52, Paul Moore wrote:
> + if (unlikely(!str)) {
> + char *str_tmp;
> + size_t len_tmp = 0;
> +
Wants a comment that lsm_active_cnt > 0 is guaranteed, or someone
(maybe static analyzers) thinks that we hit ZERO_SIZE_PTR pointer
dereference when lsm_active_cnt == 0.
> + for (i = 0; i < lsm_active_cnt; i++)
> + /* the '+ 1' accounts for either a comma or a NUL */
> + len_tmp += strlen(lsm_idlist[i]->name) + 1;
> +
> + str_tmp = kmalloc(len_tmp, GFP_KERNEL);
> + if (!str_tmp)
> + return -ENOMEM;
> + str_tmp[0] = '\0';
> +
> + for (i = 0; i < lsm_active_cnt; i++) {
> + if (i > 0)
> + strcat(str_tmp, ",");
> + strcat(str_tmp, lsm_idlist[i]->name);
> + }
> +
> + spin_lock(&lock);
> + if (!str) {
> + str = str_tmp;
> + len = len_tmp - 1;
This needs to be
len = len_tmp - 1;
mb();
str = str_tmp;
, or concurrent access might reach simple_read_from_buffer()
with str != 0 and len == 0. (If you don't want mb(), you can use
- if (unlikely(!str)) {
+ if (unlikely(!str || !len)) {
instead).
> + } else
> + kfree(str_tmp);
> + spin_unlock(&lock);
> + }
> +
> + return simple_read_from_buffer(buf, count, ppos, str, len);
> }
^ 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