* Re: [PATCH v2 01/17] landlock: Prepare ruleset and domain type split
From: Tingmao Wang @ 2026-04-12 16:29 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Christian Brauner, Günther Noack, Steven Rostedt, Jann Horn,
Jeff Xu, Justin Suess, Kees Cook, Masami Hiramatsu,
Mathieu Desnoyers, Matthieu Buffet, Mikhail Ivanov, kernel-team,
linux-fsdevel, linux-security-module, linux-trace-kernel
In-Reply-To: <20260406143717.1815792-2-mic@digikod.net>
On 4/6/26 15:36, Mickaël Salaün wrote:
> [...]
Hi Mickaël,
I like this approach, as I basically ended up doing similar refactoring
previously for the hashtable / array-based domain changes, and having this
done first should make it easier to adopt the domain data structure
changes in the future.
I assume it's fine for me to add:
Reviewed-by: Tingmao Wang <m@maowtm.org>
> @@ -175,19 +163,24 @@ static void free_rule(struct landlock_rule *const rule,
>
> static void build_check_ruleset(void)
> {
> - const struct landlock_ruleset ruleset = {
> + const struct landlock_rules rules = {
> .num_rules = ~0,
> + };
> + const struct landlock_ruleset ruleset = {
> .num_layers = ~0,
> };
>
> - BUILD_BUG_ON(ruleset.num_rules < LANDLOCK_MAX_NUM_RULES);
> + BUILD_BUG_ON(rules.num_rules < LANDLOCK_MAX_NUM_RULES);
> BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS);
> }
>
> /**
> - * insert_rule - Create and insert a rule in a ruleset
> + * insert_rule - Create and insert a rule in a rule set
^^^^^^^^
Should this be rule storage to be consistent with the next 2 lines?
Alternatively maybe we can just say "struct landlock_rules" to avoid
inventing new names?
> *
> - * @ruleset: The ruleset to be updated.
> + * @rules: The rule storage to be updated. The caller is responsible for
> + * any required locking. For rulesets, this means holding
> + * landlock_ruleset.lock. For domains under construction, no lock is
> + * needed because the domain is not yet visible to other tasks.
> * @id: The ID to build the new rule with. The underlying kernel object, if
> * any, must be held by the caller.
> * @layers: One or multiple layers to be copied into the new rule.
^ permalink raw reply
* Re: [PATCH v2 03/17] landlock: Split struct landlock_domain from struct landlock_ruleset
From: Tingmao Wang @ 2026-04-12 16:27 UTC (permalink / raw)
To: Mickaël Salaün, Günther Noack
Cc: Christian Brauner, Steven Rostedt, Jann Horn, Jeff Xu,
Justin Suess, Kees Cook, Masami Hiramatsu, Mathieu Desnoyers,
Matthieu Buffet, Mikhail Ivanov, kernel-team, linux-fsdevel,
linux-security-module, linux-trace-kernel
In-Reply-To: <20260406143717.1815792-4-mic@digikod.net>
On 4/6/26 15:37, Mickaël Salaün wrote:
> [...]
> @@ -197,10 +179,10 @@ static void build_check_ruleset(void)
> *
> * Return: 0 on success, -errno on failure.
> */
> -static int insert_rule(struct landlock_rules *const rules,
> - const struct landlock_id id,
> - const struct landlock_layer (*layers)[],
> - const size_t num_layers)
> +int landlock_rule_insert(struct landlock_rules *const rules,
> + const struct landlock_id id,
> + const struct landlock_layer (*layers)[],
> + const size_t num_layers)
Maybe this is slightly off topic, but previously I've found this function,
along with create_rule and merge_tree, to be quite confusing, because the
logic for three different use cases (creating a copy of an old domain,
merging a new layer into this domain, and inserting a new rule into an
unmerged ruleset) are mixed in the code, and personally now that I'm
reading it again I still find these functions to be hard to reason about.
Therefore, given that we're refactoring these areas, I think this might be
a good opportunity to rewrite them (while getting the necessary testing
for this rewrite "for free" as part of this whole domain refactor).
For example, for this snippet:
/* Only a single-level layer should match an existing rule. */
if (WARN_ON_ONCE(num_layers != 1))
return -EINVAL;
Someone unfamiliar with how this function is being used by its caller may
not realize that the reason the comment is true is because the only use
case where we have multiple layers in @layers being passed into this
function is when we're copying an existing domain into a new domain, and
so we should never match something that already exists.
Also, further along in this function, there is this snippet:
/*
* Intersects access rights when it is a merge between a
* ruleset and a domain.
*/
new_rule = create_rule(id, &this->layers, this->num_layers,
&(*layers)[0]);
I also found the comment to be confusing because it's not "intersect"ing
anything (it's adding a new layer to an existing rule in the domain when
the object pointed to by "id" exists already in the domain, and the
intersection is only a consequence of how Landlock works when there are
multiple layers). This realization is made harder by the fact that a few
lines above we just OR'd the access rights (for modification of an
unmerged ruleset), and so it makes it sound like it's doing a similar
thing except with AND instead of OR.
I think it might make sense to have separate functions, even if they result in some slight code duplication, for use by:
1. Copying a domain: inherit_ruleset() -> inherit_tree() -> _____()
RB tree search to find the insertion point + create_rule(). Maybe
this logic could just be in inherit_tree() without creating a separate
function?
2. Merging a rule into a domain: merge_ruleset() -> merge_tree() -> _____()
insert_or_append_rule()? RB tree search, call create_rule() to create
a new rule with the new layer added, then either
rb_link_node()+rb_insert_color() or rb_replace_node().
Neither functions above will contain any actual logical AND/OR.
3. Inserting a new rule into an unmerged ruleset: landlock_add_rule() -> ... -> _____()
insert_or_update_rule()? RB tree search, and either update the access
rights of an existing rule in-place (as we currently do), or create a
new rule if the search fails.
We could create a utility static function or macro for the shared RB tree
search code.
How does this sound?
^ permalink raw reply
* [PATCH 3/5] selftests/landlock: add tests for chmod and chown restrictions
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>
This patch adds basic tests for the support of chmod and chown system
calls restriction in landlock.
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
---
tools/testing/selftests/landlock/fs_test.c | 99 +++++++++++++++++++++-
1 file changed, 98 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index e5898dc7e53e..13d276558146 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -578,7 +578,9 @@ TEST_F_FORK(layout1, inval)
LANDLOCK_ACCESS_FS_WRITE_FILE | \
LANDLOCK_ACCESS_FS_READ_FILE | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CHMOD | \
+ LANDLOCK_ACCESS_FS_CHOWN)
#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL_DEV
@@ -4111,6 +4113,101 @@ TEST_F_FORK(ftruncate, open_and_ftruncate_in_different_processes)
ASSERT_EQ(0, close(socket_fds[1]));
}
+static int test_chmod(const char *path, mode_t mode)
+{
+ if (chmod(path, mode) == -1)
+ return errno;
+ return 0;
+}
+
+TEST_F_FORK(layout1, chmod_file)
+{
+ const char *const file_rw_no_chmod = file1_s1d1;
+ const char *const file_chmod = file1_s1d2;
+
+ const struct rule rules[] = {
+ {
+ .path = file_rw_no_chmod,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE,
+ },
+ {
+ .path = file_chmod,
+ .access = LANDLOCK_ACCESS_FS_CHMOD,
+ },
+ {},
+ };
+
+ const __u64 handled = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE |
+ LANDLOCK_ACCESS_FS_CHMOD;
+ int ruleset_fd;
+
+ /* Enables Landlock. */
+ ruleset_fd = create_ruleset(_metadata, handled, rules);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Checks chmod rights when it is not allowed, mode is arbitrary */
+ EXPECT_EQ(EACCES, test_chmod(file_rw_no_chmod, 777));
+
+ /* Checks chmod rights when it is allowed, mode is arbitrary */
+ EXPECT_EQ(0, test_chmod(file_chmod, 777));
+}
+
+static int test_chown(const char *path, uid_t owner, gid_t group)
+{
+ if (chown(path, owner, group) == -1)
+ return errno;
+ return 0;
+}
+
+TEST_F_FORK(layout1, chown_file)
+{
+ const char *const file_rw_no_chown = file1_s1d1;
+ const char *const file_chown = file1_s1d2;
+
+ const struct rule rules[] = {
+ {
+ .path = file_rw_no_chown,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE,
+ },
+ {
+ .path = file_chown,
+ .access = LANDLOCK_ACCESS_FS_CHOWN,
+ },
+ {},
+ };
+
+ const __u64 handled = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_WRITE_FILE |
+ LANDLOCK_ACCESS_FS_CHOWN;
+ int ruleset_fd;
+
+ /* Enables Landlock. */
+ ruleset_fd = create_ruleset(_metadata, handled, rules);
+
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * Checks chown rights when it is not allowed, owner and group are
+ * arbitrary.
+ */
+ EXPECT_EQ(EACCES, test_chown(file_rw_no_chown, 0, 0));
+
+ /*
+ * Checks chown rights when it is allowed, owner and group are
+ * arbitrary.
+ */
+ EXPECT_EQ(0, test_chown(file_chown, 0, 0));
+}
+
+
/* Invokes the FS_IOC_GETFLAGS IOCTL and returns its errno or 0. */
static int test_fs_ioc_getflags_ioctl(int fd)
{
--
2.53.0
^ permalink raw reply related
* [PATCH 2/5] landlock: add support for chmod and chown
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>
Modifying file permissions and owner are operation of interest when
exploiting applications. This patch adds support for both chmod and
chown system calls family in landlock, allowing one to restrict it for
a given userland application.
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
---
include/uapi/linux/landlock.h | 13 ++++++++++---
security/landlock/access.h | 2 +-
security/landlock/audit.c | 2 ++
security/landlock/fs.c | 17 ++++++++++++++++-
security/landlock/limits.h | 2 +-
5 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f88fa1f68b77..815577bda274 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -248,6 +248,12 @@ struct landlock_net_port_attr {
*
* This access right is available since the fifth version of the Landlock
* ABI.
+ * - %LANDLOCK_ACCESS_FS_CHMOD: Modify permissions on a file with
+ * :manpage:`chmod(2)` family system calls (:manpage:`fchmod(2)`,
+ * :manpage:`fchmodat(2)`, :manpage:`fchmodat2(2)`).
+ * - %LANDLOCK_ACCESS_FS_CHOWN: Change owner of a file with
+ * :manpage:`chown(2)` family system calls (:manpage:`fchown(2)`,
+ * :manpage:`fchownat(2)`, :manpage:`lchown(2)`).
*
* Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used
* with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
@@ -311,9 +317,8 @@ struct landlock_net_port_attr {
*
* It is currently not possible to restrict some file-related actions
* accessible through these syscall families: :manpage:`chdir(2)`,
- * :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`chmod(2)`,
- * :manpage:`chown(2)`, :manpage:`setxattr(2)`, :manpage:`utime(2)`,
- * :manpage:`fcntl(2)`, :manpage:`access(2)`.
+ * :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`setxattr(2)`,
+ * :manpage:`utime(2)`, :manpage:`fcntl(2)`, :manpage:`access(2)`.
* Future Landlock evolutions will enable to restrict them.
*/
/* clang-format off */
@@ -333,6 +338,8 @@ struct landlock_net_port_attr {
#define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)
#define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)
#define LANDLOCK_ACCESS_FS_IOCTL_DEV (1ULL << 15)
+#define LANDLOCK_ACCESS_FS_CHMOD (1ULL << 16)
+#define LANDLOCK_ACCESS_FS_CHOWN (1ULL << 17)
/* clang-format on */
/**
diff --git a/security/landlock/access.h b/security/landlock/access.h
index 42c95747d7bd..89dc8e7b93da 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -34,7 +34,7 @@
LANDLOCK_ACCESS_FS_IOCTL_DEV)
/* clang-format on */
-typedef u16 access_mask_t;
+typedef u32 access_mask_t;
/* Makes sure all filesystem access rights can be stored. */
static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 60ff217ab95b..a4dec40d5395 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -37,6 +37,8 @@ static const char *const fs_access_strings[] = {
[BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
[BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
[BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
+ [BIT_INDEX(LANDLOCK_ACCESS_FS_CHMOD)] = "fs.chmod",
+ [BIT_INDEX(LANDLOCK_ACCESS_FS_CHOWN)] = "fs.chown",
};
static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index e764470f588c..b32d91b733b9 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -314,7 +314,9 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
LANDLOCK_ACCESS_FS_WRITE_FILE | \
LANDLOCK_ACCESS_FS_READ_FILE | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CHMOD | \
+ LANDLOCK_ACCESS_FS_CHOWN)
/* clang-format on */
/*
@@ -1561,6 +1563,17 @@ static int hook_path_truncate(const struct path *const path)
return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
}
+static int hook_path_chmod(const struct path *const path, umode_t mode)
+{
+ return current_check_access_path(path, LANDLOCK_ACCESS_FS_CHMOD);
+}
+
+static int hook_path_chown(const struct path *const path, kuid_t uid,
+ kgid_t gid)
+{
+ return current_check_access_path(path, LANDLOCK_ACCESS_FS_CHOWN);
+}
+
/* File hooks */
/**
@@ -1838,6 +1851,8 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
LSM_HOOK_INIT(path_unlink, hook_path_unlink),
LSM_HOOK_INIT(path_rmdir, hook_path_rmdir),
LSM_HOOK_INIT(path_truncate, hook_path_truncate),
+ LSM_HOOK_INIT(path_chmod, hook_path_chmod),
+ LSM_HOOK_INIT(path_chown, hook_path_chown),
LSM_HOOK_INIT(file_alloc_security, hook_file_alloc_security),
LSM_HOOK_INIT(file_open, hook_file_open),
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index eb584f47288d..231d60d5bf8b 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -19,7 +19,7 @@
#define LANDLOCK_MAX_NUM_LAYERS 16
#define LANDLOCK_MAX_NUM_RULES U32_MAX
-#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL_DEV
+#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_CHOWN
#define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
#define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
--
2.53.0
^ permalink raw reply related
* [PATCH 1/5] selftests/landlock: fix return condition on create_directory
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>
If path exists when calling create_directory() in tests, i-e. when
mkdir() return with EEXISTS, directory creation fails. This patch
fixes it by allowing create_directory to use eventual existing
directories.
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
---
tools/testing/selftests/landlock/fs_test.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 968a91c927a4..e5898dc7e53e 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -218,8 +218,11 @@ static void mkdir_parents(struct __test_metadata *const _metadata,
static void create_directory(struct __test_metadata *const _metadata,
const char *const path)
{
+ int err;
+
mkdir_parents(_metadata, path);
- ASSERT_EQ(0, mkdir(path, 0700))
+ err = mkdir(path, 0700);
+ ASSERT_FALSE(err && errno != EEXIST)
{
TH_LOG("Failed to create directory \"%s\": %s", path,
strerror(errno));
base-commit: 82544d36b1729153c8aeb179e84750f0c085d3b1
--
2.53.0
^ permalink raw reply related
* [PATCH 4/5] samples/landlock: add support for chown and chmod
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>
Update sandboxer.c sample code with restriction for chown and chmod
system calls families
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
---
samples/landlock/sandboxer.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e7af02f98208..551e9a33665a 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -111,7 +111,9 @@ static int parse_path(char *env_path, const char ***const path_list)
LANDLOCK_ACCESS_FS_WRITE_FILE | \
LANDLOCK_ACCESS_FS_READ_FILE | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CHMOD | \
+ LANDLOCK_ACCESS_FS_CHOWN)
/* clang-format on */
@@ -295,7 +297,9 @@ static bool check_ruleset_scope(const char *const env_var,
LANDLOCK_ACCESS_FS_MAKE_SYM | \
LANDLOCK_ACCESS_FS_REFER | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_CHMOD | \
+ LANDLOCK_ACCESS_FS_CHOWN)
/* clang-format on */
--
2.53.0
^ permalink raw reply related
* landlock: Add support for chmod and chown system calls families
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
Hi,
This patch serie add support for chmod and chown system calls families
in Landlock.
These system calls could be used when exploiting applications. Two new
flags are added for struct landlock_ruleset_attr:
* LANDLOCK_ACCESS_FS_CHMOD
* LANDLOCK_ACCESS_FS_CHOWN
Restriction is limited to files as the security.c hooks for both
system calls seem to only applies to files. More digging is needed
before being able to restrict calls to chmod and chown on directories.
It adds basic tests for both family operations, one for when it is
allowed, one for when it is not.
First patch also fixes a bug I encountered when writing the tests.
^ permalink raw reply
* [PATCH 5/5] landlock: Document chmod and chown support in example code
From: Jeffrey Bencteux @ 2026-04-12 9:50 UTC (permalink / raw)
To: mic, gnoack, paul, jmorris, serge; +Cc: linux-security-module, jeff
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>
Add missing LANDLOCK_ACCESS_FS_* flags in Landlock documentation
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
---
Documentation/userspace-api/landlock.rst | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 13134bccdd39..4eb7ed3dcbfe 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -77,7 +77,9 @@ to be explicit about the denied-by-default access rights.
LANDLOCK_ACCESS_FS_MAKE_SYM |
LANDLOCK_ACCESS_FS_REFER |
LANDLOCK_ACCESS_FS_TRUNCATE |
- LANDLOCK_ACCESS_FS_IOCTL_DEV,
+ LANDLOCK_ACCESS_FS_IOCTL_DEV |
+ LANDLOCK_ACCESS_FS_CHMOD |
+ LANDLOCK_ACCESS_FS_CHOWN,
.handled_access_net =
LANDLOCK_ACCESS_NET_BIND_TCP |
LANDLOCK_ACCESS_NET_CONNECT_TCP,
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 0/4] Firmware LSM hook
From: Leon Romanovsky @ 2026-04-12 9:00 UTC (permalink / raw)
To: Paul Moore
Cc: Roberto Sassu, KP Singh, Matt Bobrowski, Alexei Starovoitov,
Daniel Borkmann, John Fastabend, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
Jason Gunthorpe, Saeed Mahameed, Itay Avraham, Dave Jiang,
Jonathan Cameron, bpf, linux-kernel, linux-kselftest, linux-rdma,
Chiara Meiohas, Maher Sanalla, linux-security-module
In-Reply-To: <CAHC9VhT1X4HX4bGrK=mEzu=g=mZ-Wg-LDXVgZVe-e6oM+W9aHg@mail.gmail.com>
On Thu, Apr 09, 2026 at 05:04:24PM -0400, Paul Moore wrote:
> On Thu, Apr 9, 2026 at 8:45 AM Leon Romanovsky <leon@kernel.org> wrote:
> > On Thu, Apr 09, 2026 at 02:27:43PM +0200, Roberto Sassu wrote:
> > > On Thu, 2026-04-09 at 15:12 +0300, Leon Romanovsky wrote:
> > > > On Tue, Mar 31, 2026 at 08:56:32AM +0300, Leon Romanovsky wrote:
> > > > > From Chiara:
> > > > >
> > > > > This patch set introduces a new BPF LSM hook to validate firmware commands
> > > > > triggered by userspace before they are submitted to the device. The hook
> > > > > runs after the command buffer is constructed, right before it is sent
> > > > > to firmware.
> > > >
> > > > <...>
> > > >
> > > > > ---
> > > > > Chiara Meiohas (4):
> > > > > bpf: add firmware command validation hook
> > > > > selftests/bpf: add test cases for fw_validate_cmd hook
> > > > > RDMA/mlx5: Externally validate FW commands supplied in DEVX interface
> > > > > fwctl/mlx5: Externally validate FW commands supplied in fwctl
> > > >
> > > > Hi,
> > > >
> > > > Can we get Ack from BPF/LSM side?
> > >
> > > + Paul, linux-security-module ML
> > >
> > > Hi
> > >
> > > probably you also want to get an Ack from the LSM maintainer (added in
> > > CC with the list). Most likely, he will also ask you to create the
> > > security_*() functions counterparts of the BPF hooks.
> >
> > We implemented this approach in v1:
> > https://patch.msgid.link/20260309-fw-lsm-hook-v1-0-4a6422e63725@nvidia.com
> > and were advised to pursue a different direction.
>
> I'm assuming you are referring to my comments? If so, that isn't exactly what I said,
> I mentioned at least one other option besides
> going directly to BPF. Ultimately, it is your choice to decide how
> you want to proceed, but to claim I advised you to avoid a LSM based
> solution isn't strictly correct.
Yes, this matches how we understood your comments:
https://lore.kernel.org/all/20260311081955.GS12611@unreal/
In the end, the goal is to build something practical and avoid adding
unnecessary complexity that brings no real benefit to users.
>
> Regardless, looking at your v2 patchset, it looks like you've taken an
> unusual approach of using some of the LSM mechanisms, e.g. LSM_HOOK(),
> but not actually exposing a LSM hook with proper callbacks.
> Unfortunately, that's not something we want to support. If you want
> to pursue an LSM based solution, complete with a security_XXX() hook,
> use of LSM_HOOK() macros, etc. then that's fine, I'm happy to work
> with you on that.
The issue is that the sentence below was the reason we did not merge v1 with v2:
https://github.com/LinuxSecurityModule/kernel/blob/main/README.md#new-lsm-hooks
"pass through implementations, such as the BPF LSM, are not eligible for
LSM hook reference implementations."
> However, if you've decided that your preferred
> option is to create a BPF hook you should avoid using things like
> LSM_HOOK() and locating your hook/code in bpf_lsm.c.
We are not limited to LSM solution, the goal is to intercept commands
which are submitted to the FW and "security" bucket sounded right to us.
>
> The good news is that there are plenty of other examples of BPF
> plugable code that you could use as an example, one such thing is the
> update_socket_protocol() BPF hook that was originally proposed as a
> LSM hook, but moved to a dedicated BPF hook as we generally want to
> avoid changing non-LSM kernel objects within the scope of the LSMs.
> While your proposed case is slightly different, I think the basic idea
> and mechanism should still be useful.
>
> https://lore.kernel.org/all/cover.1692147782.git.geliang.tang@suse.com
Thanks
>
> --
> paul-moore.com
>
^ permalink raw reply
* [PATCH] trusted-keys: move pr_fmt out of trusted-type.h
From: Josh Snyder @ 2026-04-11 20:12 UTC (permalink / raw)
To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Ahmad Fatoum, Pengutronix Kernel Team, Paul Moore, James Morris,
Serge E. Hallyn, David Gstir, sigma star Kernel Team,
Srish Srinivasan, Nayna Jain, Sumit Garg
Cc: linux-integrity, keyrings, linux-kernel, linux-security-module,
Josh Snyder
Defining pr_fmt in a widely-included header leaks the "trusted_key: "
prefix into every translation unit that transitively includes
<keys/trusted-type.h>. dm-crypt, for example, ends up printing
trusted_key: device-mapper: crypt: dm-10: INTEGRITY AEAD ERROR ...
dm-crypt began including <keys/trusted-type.h> in commit 363880c4eb36
("dm crypt: support using trusted keys"), which predates the pr_fmt
addition, so the regression has been live from the moment the header
gained its own pr_fmt definition.
Move the pr_fmt definition into the trusted-keys source files that
actually want the prefix.
Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Josh Snyder <josh@code406.com>
---
| 6 ------
| 2 ++
| 2 ++
| 2 ++
| 2 ++
| 2 ++
| 2 ++
7 files changed, 12 insertions(+), 6 deletions(-)
--git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 03527162613f7..54da1f174aeab 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -11,12 +11,6 @@
#include <linux/rcupdate.h>
#include <linux/tpm.h>
-#ifdef pr_fmt
-#undef pr_fmt
-#endif
-
-#define pr_fmt(fmt) "trusted_key: " fmt
-
#define MIN_KEY_SIZE 32
#define MAX_KEY_SIZE 128
#if IS_ENABLED(CONFIG_TRUSTED_KEYS_PKWM)
--git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
index 601943ce0d60f..a31fd89c0e5c5 100644
--- a/security/keys/trusted-keys/trusted_caam.c
+++ b/security/keys/trusted-keys/trusted_caam.c
@@ -4,6 +4,8 @@
* Copyright 2025 NXP
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <keys/trusted_caam.h>
#include <keys/trusted-type.h>
#include <linux/build_bug.h>
--git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index 0b142d941cd2e..159af9dcfc774 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -6,6 +6,8 @@
* See Documentation/security/keys/trusted-encrypted.rst
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <keys/user-type.h>
#include <keys/trusted-type.h>
#include <keys/trusted_tee.h>
--git a/security/keys/trusted-keys/trusted_dcp.c b/security/keys/trusted-keys/trusted_dcp.c
index 7b6eb655df0cb..f15ec400848ce 100644
--- a/security/keys/trusted-keys/trusted_dcp.c
+++ b/security/keys/trusted-keys/trusted_dcp.c
@@ -3,6 +3,8 @@
* Copyright (C) 2021 sigma star gmbh
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <crypto/aead.h>
#include <crypto/aes.h>
#include <crypto/algapi.h>
--git a/security/keys/trusted-keys/trusted_pkwm.c b/security/keys/trusted-keys/trusted_pkwm.c
index bf42c6679245a..94c92b90d88da 100644
--- a/security/keys/trusted-keys/trusted_pkwm.c
+++ b/security/keys/trusted-keys/trusted_pkwm.c
@@ -3,6 +3,8 @@
* Copyright (C) 2025 IBM Corporation, Srish Srinivasan <ssrish@linux.ibm.com>
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <keys/trusted_pkwm.h>
#include <keys/trusted-type.h>
#include <linux/build_bug.h>
--git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 6ea728f1eae6f..69dac20e4bf23 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -6,6 +6,8 @@
* See Documentation/security/keys/trusted-encrypted.rst
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <crypto/hash_info.h>
#include <crypto/sha1.h>
#include <crypto/utils.h>
--git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 6340823f8b53c..f47ae952a0e7c 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -4,6 +4,8 @@
* Copyright (C) 2014 Intel Corporation
*/
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#include <linux/asn1_encoder.h>
#include <linux/oid_registry.h>
#include <linux/string.h>
---
base-commit: cc13002a9f984d37906e9476f3e532a8cdd126f5
change-id: 20260411-trusted-key-header-a544a4f149d2
Best regards,
--
Josh
^ permalink raw reply related
* [PATCH 3/3] selftests/landlock: Test OverlayFS renames w/o LANDLOCK_ACCESS_FS_MAKE_CHAR
From: Günther Noack @ 2026-04-11 9:09 UTC (permalink / raw)
To: Mickaël Salaün, Christian Brauner
Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
Serge Hallyn, Günther Noack
In-Reply-To: <20260411090944.3131168-2-gnoack@google.com>
Even though OverlayFS uses vfs_rename() with RENAME_WHITEOUT under the
hood, and even though RENAME_WHITEOUT requires
LANDLOCK_ACCESS_FS_MAKE_CHAR, a process that renames files in an OverlayFS
can do so without having the LANDLOCK_ACCESS_FS_MAKE_CHAR right in that
location. This works because OverlayFS uses the credentials determined at
mount time for the internal vfs_rename() operation.
Signed-off-by: Günther Noack <gnoack@google.com>
---
security/landlock/fs.c | 11 +++++---
tools/testing/selftests/landlock/fs_test.c | 31 ++++++++++++++++++++++
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 2b84a229e4d8..9b49f6c3e5da 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1523,11 +1523,14 @@ static int hook_path_rename(const struct path *const old_dir,
int err;
/*
- * This check would better be done together with other path
- * walks which are already happening for the normal rename check
- * in current_check_refer_path().
+ * Rename with RENAME_WHITEOUT creates a whiteout object
+ * (character device file with major=minor=0) in the old
+ * location, so we check the access right for creating that.
+ *
+ * See Documentation/filesystems/overlayfs.rst and renameat2(2).
*/
- err = current_check_access_path(old_dir, LANDLOCK_ACCESS_FS_MAKE_CHAR);
+ err = current_check_access_path(old_dir,
+ LANDLOCK_ACCESS_FS_MAKE_CHAR);
if (err)
return err;
}
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index d867016e3fd3..4cf6fc0bcb71 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -6962,6 +6962,37 @@ TEST_F_FORK(layout2_overlay, same_content_different_file)
}
}
+TEST_F_FORK(layout2_overlay, rename_in_overlay_without_make_char)
+{
+ struct stat st;
+ const char *merge_fl1_renamed = MERGE_DATA "/fl1_renamed";
+
+ if (self->skip_test)
+ SKIP(return, "overlayfs is not supported (test)");
+
+ enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_CHAR, NULL);
+
+ /*
+ * Execute a regular file rename within OverlayFS.
+ * merge_fl1 originates from lower layer, so this triggers a copy-up
+ * and creation of a whiteout in the upper layer.
+ */
+ EXPECT_EQ(0, rename(merge_fl1, merge_fl1_renamed));
+
+ /* Check that the rename worked. */
+ EXPECT_EQ(0, stat(merge_fl1_renamed, &st));
+ EXPECT_EQ(-1, stat(merge_fl1, &st));
+ EXPECT_EQ(ENOENT, errno);
+
+ /*
+ * Check that the whiteout object on the underlying "upper" filesystem
+ * exists after the rename. This is OK because it was done with the
+ * credentials of the OverlayFS.
+ */
+ EXPECT_EQ(0, stat(UPPER_DATA "/fl1", &st));
+ EXPECT_TRUE(S_ISCHR(st.st_mode));
+ EXPECT_EQ(0, st.st_rdev);
+}
FIXTURE(layout3_fs)
{
--
2.54.0.rc0.605.g598a273b03-goog
^ permalink raw reply related
* [PATCH 2/3] selftests/landlock: Add test for RENAME_WHITEOUT denial
From: Günther Noack @ 2026-04-11 9:09 UTC (permalink / raw)
To: Mickaël Salaün, Christian Brauner
Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
Serge Hallyn, Günther Noack
In-Reply-To: <20260411090944.3131168-2-gnoack@google.com>
Add a test to check that renames with RENAME_WHITEOUT are guarded by
LANDLOCK_ACCESS_FS_MAKE_CHAR.
Signed-off-by: Günther Noack <gnoack@google.com>
---
tools/testing/selftests/landlock/fs_test.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index cdb47fc1fc0a..d867016e3fd3 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -2247,6 +2247,19 @@ TEST_F_FORK(layout1, rename_file)
RENAME_EXCHANGE));
}
+TEST_F_FORK(layout1, rename_whiteout_denied)
+{
+ enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_CHAR, NULL);
+
+ /*
+ * Try to rename a file with RENAME_WHITEOUT.
+ * file1_s3d3 is in dir_s3d2 (tmpfs), so it supports RENAME_WHITEOUT.
+ */
+ EXPECT_EQ(-1, renameat2(AT_FDCWD, file1_s3d3, AT_FDCWD,
+ TMP_DIR "/s3d1/s3d2/s3d3/f2", RENAME_WHITEOUT));
+ EXPECT_EQ(EACCES, errno);
+}
+
TEST_F_FORK(layout1, rename_dir)
{
const struct rule rules[] = {
@@ -6949,6 +6962,7 @@ TEST_F_FORK(layout2_overlay, same_content_different_file)
}
}
+
FIXTURE(layout3_fs)
{
bool has_created_dir;
--
2.54.0.rc0.605.g598a273b03-goog
^ permalink raw reply related
* [PATCH 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_CHAR for RENAME_WHITEOUT
From: Günther Noack @ 2026-04-11 9:09 UTC (permalink / raw)
To: Mickaël Salaün, Christian Brauner
Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
Serge Hallyn, Günther Noack
In-Reply-To: <20260411090944.3131168-2-gnoack@google.com>
renameat2(2) with the RENAME_WHITEOUT flag places a whiteout character
device file in the source file location in place of the moved file,
bypassing the LANDLOCK_ACCESS_FS_MAKE_CHAR right.
Fix this by checking for LANDLOCK_ACCESS_FS_MAKE_CHAR if RENAME_WHITEOUT is
passed.
This does not affect normal renames within layered OverlayFS mounts: When
OverlayFS invokes rename with RENAME_WHITEOUT as part of a "normal" rename
operation, it does so in ovl_rename() using the credentials that were set
at the time of mounting the OverlayFS.
Suggested-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Günther Noack <gnoack@google.com>
---
security/landlock/fs.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index c1ecfe239032..2b84a229e4d8 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1519,6 +1519,19 @@ static int hook_path_rename(const struct path *const old_dir,
const unsigned int flags)
{
/* old_dir refers to old_dentry->d_parent and new_dir->mnt */
+ if (flags & RENAME_WHITEOUT) {
+ int err;
+
+ /*
+ * This check would better be done together with other path
+ * walks which are already happening for the normal rename check
+ * in current_check_refer_path().
+ */
+ err = current_check_access_path(old_dir, LANDLOCK_ACCESS_FS_MAKE_CHAR);
+ if (err)
+ return err;
+ }
+
return current_check_refer_path(old_dentry, new_dir, new_dentry, true,
!!(flags & RENAME_EXCHANGE));
}
--
2.54.0.rc0.605.g598a273b03-goog
^ permalink raw reply related
* [PATCH 0/3] landlock: Restrict renameat2 with RENAME_WHITEOUT
From: Günther Noack @ 2026-04-11 9:09 UTC (permalink / raw)
To: Mickaël Salaün, Christian Brauner
Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
Serge Hallyn, Günther Noack
Hello!
As discussed in [1], the renameat2() syscall's RENAME_WHITEOUT flag allows
the creation of chardev directory entries with major=minor=0 as "whiteout
objects" in the location of the rename source file [2].
This functionality is available even without having any OverlayFS mounted
and can be invoked with the regular renameat2(2) syscall [3].
Motivation
==========
The RENAME_WHITEOUT flag side-steps Landlock's LANDLOCK_ACCESS_FS_MAKE_CHAR
right, which is designed to restrict the creation of chardev device files.
This patch set fixes that by adding a check in Landlock's path_rename hook.
Tradeoffs considered in the implementation
==========================================
Q: Should we guard it with a dedicated LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
right?
Pros:
* This would be the fully backwards compatible solution,
and Linux always strives for full backward compatibility.
Cons:
* Complicates the Landlock API surface for a very minor use case.
In Debian Code search, the only use of RENAME_WHITEOUT from userspace
seems to be for fuse-overlayfs. It is used there for the same purpose
as in the kernel OverlayFS and it likely does not run in a Landlock
domain.
The tradeoff does not seem worth it to me. The chances that we break
anyone with this seem very low, and I'm inclined to treat it as a bugfix
for the existing LANDLOCK_ACCESS_FS_MAKE_CHAR right.
Q: Should we add a Landlock erratum for this?
I punted on it for now, but we can do it if needed.
Q: Should the access right check be merged into the longer
current_check_refer_path() function?
I am leaning towards keeping it as a special case earlier. This means
that we traverse the source path twice, but as we have seen in Debian
Code Search, there are apparently no legitimate callers of renameat2()
with RENAME_WHITEOUT who are calling this from within a Landlock domain.
(fuse-overlayfs is legitimate, but is not landlocked)
It doesn't seem worth complicating our common rename code for a corner
case that doesn't happen in practice.
[1] https://lore.kernel.org/all/adUBCQXrt7kmgqJT@google.com/
[2] https://docs.kernel.org/filesystems/overlayfs.html#whiteouts-and-opaque-directories
[3] https://man7.org/linux/man-pages/man2/renameat2.2.html#DESCRIPTION
[4] https://codesearch.debian.net/search?q=rename.*RENAME_WHITEOUT&literal=0
Günther Noack (3):
landlock: Require LANDLOCK_ACCESS_FS_MAKE_CHAR for RENAME_WHITEOUT
selftests/landlock: Add test for RENAME_WHITEOUT denial
selftests/landlock: Test OverlayFS renames w/o
LANDLOCK_ACCESS_FS_MAKE_CHAR
security/landlock/fs.c | 16 ++++++++
tools/testing/selftests/landlock/fs_test.c | 45 ++++++++++++++++++++++
2 files changed, 61 insertions(+)
--
2.54.0.rc0.605.g598a273b03-goog
^ permalink raw reply
* Re: LSM: Whiteout chardev creation sidesteps mknod hook
From: Günther Noack @ 2026-04-11 8:36 UTC (permalink / raw)
To: Christian Brauner
Cc: Serge Hallyn, Miklos Szeredi, Amir Goldstein,
Mickaël Salaün, Paul Moore, linux-security-module
In-Reply-To: <20260409-entbrennen-turnschuh-54af9b45610e@brauner>
On Thu, Apr 09, 2026 at 02:47:16PM +0200, Christian Brauner wrote:
> On Tue, Apr 07, 2026 at 12:15:00PM -0500, Serge Hallyn wrote:
> > Apr 7, 2026 08:05:43 Günther Noack <gnoack@google.com>:
> >
> > > Hello Christian, Paul, Mickaël and LSM maintainers!
> > >
> > > I discovered the following bug in Landlock, which potentially also
> > > affects other LSMs:
> > >
> > > With renameat2(2)'s RENAME_WHITEOUT flag, it is possible to create a
> > > "whiteout object" at the source of the rename. Whiteout objects are
> > > character devices with major/minor (0, 0) -- these devices are not
> > > bound to any driver, so they are harmless, but still, the creation of
> > > these files can sidestep the LANDLOCK_ACCESS_FS_MAKE_CHAR access right
> > > in Landlock.
>
> They aren't devices.
The LANDLOCK_ACCESS_FS_MAKE_CHAR access right is about the *creation of
character device directory entries*.
These files do not hook up to any of the kernel's device driver subsystems, but
they *are* directory entries of the chardev type, and the creation of these is
still sidestepping the LANDLOCK_ACCESS_FS_MAKE_CHAR right.
> > > I am unconvinced which is the right fix here -- do you have an opinion
> > > on this from the VFS/LSM side?
> > >
> > >
> > > Option 1: Make filesystems call security_path_mknod() during RENAME_WHITEOUT?
>
> No.
>
> > >
> > > Do it in the VFS rename hook.
> > >
> > > * Pro: Fixes it for all LSMs
> > > * Con: Call would have to be done in multiple filesystems
> > >
> > >
> > > Option 2: Handle it in security_{path,inode}_rename()
> > >
> > > Make Landlock handle it in security_inode_rename() by looking for the
> > > RENAME_WHITEOUT flag.
> > >
> > > * Con: Operation should only be denied if the file system even
> > > implements RENAME_WHITEOUT, and we would have to maintain a list of
>
> Why? Just deny RENAME_WHITEOUT. What does it matter if the filesystem
> implements it or not. Overlayfs would fall back to non-RENAME_WHITEOUT
> if not provided by the upper fs anway.
I'll send a patch with this approach for discussion.
It turns out it is less difficult than I feared:
* OverlayFS uses its own credentials object, and since that is not under a
Landlock policy, the OverlayFS-internal vfs_rename() invocations do not have
that problem. (Under a Landlock policy, mount(2) is not permitted, so the
OverlayFS-internal credentials are not Landlocked.)
* The remaining use case is only when a user calls renameat2(...,
RENAME_WHITEOUT) directly on a filesystem (which is not necessarily part of an
OverlayFS). That case can be restricted with Landlock.
We might have slight error code inconsistencies yes, but as Mickaël is saying on
the sibling mail thread, it would not be worth the tradeoff to maintain a list
of supported file systems just to get the error codes right.
> > > affected filesystems for that. (That feels like solving it at the
> > > wrong layer of abstraction.)
> > > * Con: Unclear whether other LSMs need a similar fix
> > >
> > >
> > > Option 3: Declare that this is working as intended?
> >
> > Option 3 has my vote.
>
> Seconded.
(See also discussion on sibling thread)
I also don't currently see how an attacker would abuse this, but I still see
this as a violation of Landlock's security model if we can create a policy that
denies the creation of character device directory entries, and then we still
have a way to make them appear there where we previously had a different file.
I'll send a tentative patch for option 2 for discussion. We can discuss more in
the context of that more concrete proposal, if needed.
—Günther
^ permalink raw reply
* Re: LSM: Whiteout chardev creation sidesteps mknod hook
From: Günther Noack @ 2026-04-11 8:26 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Christian Brauner, Paul Moore, linux-security-module,
John Johansen, Georgia Garcia, Kentaro Takeda, Tetsuo Handa
In-Reply-To: <20260408.beu1Eing5aFo@digikod.net>
On Wed, Apr 08, 2026 at 01:01:28PM +0200, Mickaël Salaün wrote:
> On Tue, Apr 07, 2026 at 03:05:13PM +0200, Günther Noack wrote:
> > Hello Christian, Paul, Mickaël and LSM maintainers!
> >
> > I discovered the following bug in Landlock, which potentially also
> > affects other LSMs:
> >
> > With renameat2(2)'s RENAME_WHITEOUT flag, it is possible to create a
> > "whiteout object" at the source of the rename. Whiteout objects are
> > character devices with major/minor (0, 0) -- these devices are not
> > bound to any driver, so they are harmless, but still, the creation of
> > these files can sidestep the LANDLOCK_ACCESS_FS_MAKE_CHAR access right
> > in Landlock.
>
> Any way to "write" on the filesystem should properly be controlled. The
> man page says that RENAME_WHITEOUT requires CAP_MKNOD, however, looking
> at vfs_mknod(), there is an explicit exception to not check CAP_MKNOD
> for whiteout devices. See commit a3c751a50fe6 ("vfs: allow unprivileged
> whiteout creation").
Agreed, it should be possible to restrict it.
> > Option 2: Handle it in security_{path,inode}_rename()
> >
> > Make Landlock handle it in security_inode_rename() by looking for the
> > RENAME_WHITEOUT flag.
> >
> > * Con: Operation should only be denied if the file system even
> > implements RENAME_WHITEOUT, and we would have to maintain a list of
> > affected filesystems for that. (That feels like solving it at the
> > wrong layer of abstraction.)
>
> Why would we need to maintain such list? If it's only about the errno,
> well, that would not be perfect be ok with a proper doc.
Yes, it would be only about the errno. At the time of writing the initial mail,
I was also worried that OverlayFS would get confused if its internal
vfs_rename() call would sometimes work and sometimes be denied, but as it turns
out, since OverlayFS uses its own credentials internally, this is a non-issue.
I'll send a tentative patch for option 2 for discussion.
> I'm mostly worried that there might be other (future) call paths to
> create whiteout devices.
>
> I think option 2 would be the most practical approach for Landlock, with
> a new LANDLOCK_ACCESS_FS_MAKE_WHITEOUT right.
Given that this only affect immediate renameat2() calls, I would actually argue
that we can probably get away with guarding this with the existing
LANDLOCK_ACCESS_FS_MAKE_CHAR right?
I checked Debian code search for usages:
https://codesearch.debian.net/search?q=rename.*RENAME_WHITEOUT&literal=0
Apart from the usual proliferation of copied-around kernel headers and wrapper
pass-through wrapper libraries around renameat2(), the only actual use I found
for the immediate renameat2() syscall with RENAME_WHITEOUT was in fuse-overlayfs
(for the exact same reason). Fuse-overlayfs is likely not running under a
Landlock policy given that it doesn't have Landlock support itself and given
that it also has to do a mount(), which is forbidden under Landlock, so users
are also unlikely to wrap it in a Landlock domain.
> I'm also wondering how are the chances that other kind of special file
> type like a whiteout device could come up in the future. Any guess
> Christian?
>
> > * Con: Unclear whether other LSMs need a similar fix
>
> I guess at least AppArmor and Tomoyo would consider that an issue.
>
> >
> >
> > Option 3: Declare that this is working as intended?
>
> We need to be able to controle any file creation, which is not currently
> the case because of this whiteout exception.
Seconded. Landlock offers a long list of access rights to restrict the creation
of new directory entries, and this is a way to create a new directory entry
anyway. Even though it's not immediately clear to me how this can be abused for
an actual attack, it is a violation of the previously defined Landlock policy if
directory entries can be created this way.
—Günther
^ permalink raw reply
* Re: [PATCH] security: remove BUG_ON in security_skb_classify_flow
From: Serge E. Hallyn @ 2026-04-10 23:34 UTC (permalink / raw)
To: Jiayuan Chen
Cc: Serge E. Hallyn, Stephen Smalley, linux-security-module, paul,
jmorris, linux-kernel, Kaiyan Mei, Yinhao Hu, Dongliang Mu
In-Reply-To: <a17199c6-fb52-493b-b76a-505faf27cfa0@linux.dev>
On Fri, Apr 10, 2026 at 09:56:22AM +0800, Jiayuan Chen wrote:
>
> On 4/10/26 8:58 AM, Serge E. Hallyn wrote:
> > On Wed, Apr 08, 2026 at 07:42:57PM +0800, Jiayuan Chen wrote:
> > > A BPF program attached to the xfrm_decode_session hook can return a
> > > non-zero value, which causes BUG_ON(rc) in security_skb_classify_flow()
> > > to trigger a kernel panic.
> > It would seem worth it to have pointed at the previous discussion at
> >
> > https://lore.kernel.org/all/CAEjxPJ5aA01in+Z1yLF1cwe-3uqL_E8SKGK4J294D5eRG5__5Q@mail.gmail.com/
> >
> > Based on that, I guess this is probably ok, but still,
> >
> > > Remove the BUG_ON and change the return type from void to int, so that
> > > callers can optionally handle the error.
> > but you don't have the existing callers handling the error. It's
> > conceivable they won't care, but it's also possible that they were
> > counting on a BUG_ON in that case.
> >
> > What *should* callers (icmp_reply, etc) do if an error code is
> > returned? Should they ignore it? In that case, would it be
> > better to change security_skb_classify_flow() to return void?
> >
> Thanks for your pointer.
>
> So I think Feng's patch is sufficient and can by applied ?
Well, selinux_xfrm_decode_session() calls selinux_xfrm_skb_sid_ingress()
which *can* return -EINVAL.
So I'd like to know, what is supposed to happen in that case?
Stephen, do you know? Is it safe for callers to ignore this?
^ permalink raw reply
* Re: [GIT PULL] lsm/lsm-pr-20260410
From: Paul Moore @ 2026-04-10 23:28 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module, linux-kernel
In-Reply-To: <9a2a59bd8d9548fb5dab128f4859fa3d@paul-moore.com>
On Fri, Apr 10, 2026 at 7:26 PM Paul Moore <paul@paul-moore.com> wrote:
>
> - Fix problems with the mmap() and mprotect() LSM hooks on overlayfs
I forgot to add that you may see a minor merge conflict with the VFS
tree, but based on what was seen in linux-next it was trivial and
easily resolved. I know you prefer to resolve those yourself, but if
you need a rebased branch/pull-request let me know.
--
paul-moore.com
^ permalink raw reply
* [GIT PULL] selinux/selinux-pr-20260410
From: Paul Moore @ 2026-04-10 23:26 UTC (permalink / raw)
To: Linus Torvalds; +Cc: selinux, linux-security-module, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1054 bytes --]
Linus,
Just a single patch from the SELinux tree for v7.1, although you will find
an important SELinux fix in the LSM pull request (if you haven't processed
that already).
- Annotate a known race condition to soothe KCSAN
Paul
--
The following changes since commit 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f:
Linux 7.0-rc1 (2026-02-22 13:18:59 -0800)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
tags/selinux-pr-20260410
for you to fetch changes up to 8dc51459ef702bcc0ef5fb26bb4d362b38aa56c2:
selinux: annotate intentional data race in inode_doinit_with_dentry()
(2026-02-23 11:14:29 -0500)
----------------------------------------------------------------
selinux/stable-7.1 PR 20260410
----------------------------------------------------------------
Christian Göttsche (1):
selinux: annotate intentional data race in
inode_doinit_with_dentry()
security/selinux/hooks.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--
paul-moore.com
^ permalink raw reply
* [GIT PULL] lsm/lsm-pr-20260410
From: Paul Moore @ 2026-04-10 23:26 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module, linux-kernel
Linus,
We only have five patches in the LSM tree, but three of the five are for
an important bugfix relating to overlayfs and the mmap() and mprotect()
access controls for LSMs. Highlights below:
- Fix problems with the mmap() and mprotect() LSM hooks on overlayfs
As we are dealing with problems both in mmap() and mprotect() there are
essentially two components to this fix, spread across three patches with
all marked for stable.
The simplest portion of the fix is the creation of a new LSM hook,
security_mmap_backing_file(), that is used to enforce LSM mmap() access
controls on backing files in the stacked/overlayfs case. The existing
security_mmap_file() does not have visibility past the user file. You
can see from the associated SELinux hook callback the code is fairly
straightforward.
The mprotect() fix is a bit more complicated as there is no way in the
mprotect() code path to inspect both the user and backing files, and
bolting on a second file reference to vm_area_struct wasn't really an
option. The solution taken here adds a LSM security blob and associated
hooks to the backing_file struct that LSMs can use to capture and store
relevant information from the user file. While the necessary SELinux
information is relatively small, a single u32, I expect other LSMs to
require more than that, and a dedicated backing_file LSM blob provides
a storage mechanism without negatively impacting other filesystems.
I want to note that other LSMs beyond SELinux have been involved in the
discussion of the fixes presented here and they are working on their own
related changes using these new hooks, but due to other issues those
patches will be coming at a later date.
- Use kstrdup_const()/kfree_const() for securityfs symlink targets
- Resolve a handful of kernel-doc warnings in cred.h
Paul
--
The following changes since commit 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f:
Linux 7.0-rc1 (2026-02-22 13:18:59 -0800)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
tags/lsm-pr-20260410
for you to fetch changes up to 82544d36b1729153c8aeb179e84750f0c085d3b1:
selinux: fix overlayfs mmap() and mprotect() access checks
(2026-04-03 16:53:50 -0400)
----------------------------------------------------------------
lsm/stable-7.1 PR 20260410
----------------------------------------------------------------
Amir Goldstein (1):
fs: prepare for adding LSM blob to backing_file
Dmitry Antipov (1):
securityfs: use kstrdup_const() to manage symlink targets
Paul Moore (2):
lsm: add backing_file LSM hooks
selinux: fix overlayfs mmap() and mprotect() access checks
Randy Dunlap (1):
cred: fix kernel-doc warnings in cred.h
fs/backing-file.c | 18 +-
fs/erofs/ishare.c | 10 +
fs/file_table.c | 43 ++++-
fs/fuse/passthrough.c | 2
fs/internal.h | 3
fs/overlayfs/dir.c | 2
fs/overlayfs/file.c | 2
include/linux/backing-file.h | 4
include/linux/cred.h | 10 -
include/linux/fs.h | 13 +
include/linux/lsm_audit.h | 2
include/linux/lsm_hook_defs.h | 5
include/linux/lsm_hooks.h | 1
include/linux/security.h | 22 ++
security/inode.c | 10 -
security/lsm.h | 1
security/lsm_init.c | 9 +
security/security.c | 102 +++++++++++
security/selinux/hooks.c | 256 +++++++++++++++++++++---------
security/selinux/include/objsec.h | 11 +
20 files changed, 431 insertions(+), 95 deletions(-)
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3] KEYS: trusted: Debugging as a feature
From: Srish Srinivasan @ 2026-04-10 17:33 UTC (permalink / raw)
To: Jarkko Sakinen, linux-integrity, keyrings
Cc: Nayna Jain, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, Ahmad Fatoum,
Pengutronix Kernel Team, linux-kernel, linux-security-module
In-Reply-To: <20260409160752.988713-1-jarkko@kernel.org>
On 4/9/26 9:37 PM, Jarkko Sakinen wrote:
> From: Jarkko Sakkinen <jarkko@kernel.org>
>
> TPM_DEBUG, and other similar flags, are a non-standard way to specify a
> feature in Linux kernel. Introduce CONFIG_TRUSTED_KEYS_DEBUG for trusted
> keys, and use it to replace these ad-hoc feature flags.
>
> Given that trusted keys debug dumps can contain sensitive data, harden the
> feature as follows:
>
> 1. In the Kconfig description postulate that pr_debug() statements must be
> used.
> 2. Use pr_debug() statements in TPM 1.x driver to print the protocol dump.
> 3. Require trusted.debug=1 on the kernel command line (default: 0) to
> activate dumps at runtime, even when CONFIG_TRUSTED_KEYS_DEBUG=y.
>
> Traces, when actually needed, can be easily enabled by providing
> trusted.dyndbg='+p' and trusted.debug=1 in the kernel command-line.
>
> Cc: Srish Srinivasan <ssrish@linux.ibm.com>
> Reported-by: Nayna Jain <nayna@linux.ibm.com>
> Closes: https://lore.kernel.org/all/7f8b8478-5cd8-4d97-bfd0-341fd5cf10f9@linux.ibm.com/
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
Tested on PKWM and emulated TPM backends.
Tested-by: Srish Srinivasan <ssrish@linux.ibm.com>
> ---
> v3:
> - Add kernel-command line option for enabling the traces.
> - Add safety information to the Kconfig entry.
> v2:
> - Implement for all trusted keys backends.
> - Add HAVE_TRUSTED_KEYS_DEBUG as it is a good practice despite full
> coverage.
> ---
> include/keys/trusted-type.h | 21 ++++++-----
> security/keys/trusted-keys/Kconfig | 23 ++++++++++++
> security/keys/trusted-keys/trusted_caam.c | 7 ++--
> security/keys/trusted-keys/trusted_core.c | 6 ++++
> security/keys/trusted-keys/trusted_tpm1.c | 44 +++++++++++++----------
> 5 files changed, 71 insertions(+), 30 deletions(-)
>
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index 03527162613f..9f9940482da4 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -83,18 +83,21 @@ struct trusted_key_source {
>
> extern struct key_type key_type_trusted;
>
> -#define TRUSTED_DEBUG 0
> +#ifdef CONFIG_TRUSTED_KEYS_DEBUG
> +extern bool trusted_debug;
>
> -#if TRUSTED_DEBUG
> static inline void dump_payload(struct trusted_key_payload *p)
> {
> - pr_info("key_len %d\n", p->key_len);
> - print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE,
> - 16, 1, p->key, p->key_len, 0);
> - pr_info("bloblen %d\n", p->blob_len);
> - print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE,
> - 16, 1, p->blob, p->blob_len, 0);
> - pr_info("migratable %d\n", p->migratable);
> + if (!trusted_debug)
> + return;
> +
> + pr_debug("key_len %d\n", p->key_len);
> + print_hex_dump_debug("key ", DUMP_PREFIX_NONE,
> + 16, 1, p->key, p->key_len, 0);
> + pr_debug("bloblen %d\n", p->blob_len);
> + print_hex_dump_debug("blob ", DUMP_PREFIX_NONE,
> + 16, 1, p->blob, p->blob_len, 0);
> + pr_debug("migratable %d\n", p->migratable);
> }
> #else
> static inline void dump_payload(struct trusted_key_payload *p)
> diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
> index 9e00482d886a..c1ae7db1f612 100644
> --- a/security/keys/trusted-keys/Kconfig
> +++ b/security/keys/trusted-keys/Kconfig
> @@ -1,10 +1,29 @@
> config HAVE_TRUSTED_KEYS
> bool
>
> +config HAVE_TRUSTED_KEYS_DEBUG
> + bool
> +
> +config TRUSTED_KEYS_DEBUG
> + bool "Debug trusted keys"
> + depends on HAVE_TRUSTED_KEYS_DEBUG
> + default n
> + help
> + Trusted keys backends and core code that support debug traces can
> + opt-in that feature here. Traces must only use debug level output, as
> + sensitive data may pass by. In the kernel-command line traces can be
> + enabled via trusted.dyndbg='+p'.
> +
> + SAFETY: Debug dumps are inactive at runtime until trusted.debug=1 is
> + set on the kernel command-line. Use at your utmost consideration when
> + enabling this feature on a production build. The general advice is not
> + to do this.
> +
> config TRUSTED_KEYS_TPM
> bool "TPM-based trusted keys"
> depends on TCG_TPM >= TRUSTED_KEYS
> default y
> + select HAVE_TRUSTED_KEYS_DEBUG
> select CRYPTO_HASH_INFO
> select CRYPTO_LIB_SHA1
> select CRYPTO_LIB_UTILS
> @@ -23,6 +42,7 @@ config TRUSTED_KEYS_TEE
> bool "TEE-based trusted keys"
> depends on TEE >= TRUSTED_KEYS
> default y
> + select HAVE_TRUSTED_KEYS_DEBUG
> select HAVE_TRUSTED_KEYS
> help
> Enable use of the Trusted Execution Environment (TEE) as trusted
> @@ -33,6 +53,7 @@ config TRUSTED_KEYS_CAAM
> depends on CRYPTO_DEV_FSL_CAAM_JR >= TRUSTED_KEYS
> select CRYPTO_DEV_FSL_CAAM_BLOB_GEN
> default y
> + select HAVE_TRUSTED_KEYS_DEBUG
> select HAVE_TRUSTED_KEYS
> help
> Enable use of NXP's Cryptographic Accelerator and Assurance Module
> @@ -42,6 +63,7 @@ config TRUSTED_KEYS_DCP
> bool "DCP-based trusted keys"
> depends on CRYPTO_DEV_MXS_DCP >= TRUSTED_KEYS
> default y
> + select HAVE_TRUSTED_KEYS_DEBUG
> select HAVE_TRUSTED_KEYS
> help
> Enable use of NXP's DCP (Data Co-Processor) as trusted key backend.
> @@ -50,6 +72,7 @@ config TRUSTED_KEYS_PKWM
> bool "PKWM-based trusted keys"
> depends on PSERIES_PLPKS >= TRUSTED_KEYS
> default y
> + select HAVE_TRUSTED_KEYS_DEBUG
> select HAVE_TRUSTED_KEYS
> help
> Enable use of IBM PowerVM Key Wrapping Module (PKWM) as a trusted key backend.
> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
> index 601943ce0d60..6a33dbf2a7f5 100644
> --- a/security/keys/trusted-keys/trusted_caam.c
> +++ b/security/keys/trusted-keys/trusted_caam.c
> @@ -28,10 +28,13 @@ static const match_table_t key_tokens = {
> {opt_err, NULL}
> };
>
> -#ifdef CAAM_DEBUG
> +#ifdef CONFIG_TRUSTED_KEYS_DEBUG
> static inline void dump_options(const struct caam_pkey_info *pkey_info)
> {
> - pr_info("key encryption algo %d\n", pkey_info->key_enc_algo);
> + if (!trusted_debug)
> + return;
> +
> + pr_debug("key encryption algo %d\n", pkey_info->key_enc_algo);
> }
> #else
> static inline void dump_options(const struct caam_pkey_info *pkey_info)
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index 9046123d94de..9ce2459d14b4 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -31,6 +31,12 @@ static char *trusted_rng = "default";
> module_param_named(rng, trusted_rng, charp, 0);
> MODULE_PARM_DESC(rng, "Select trusted key RNG");
>
> +#ifdef CONFIG_TRUSTED_KEYS_DEBUG
> +bool trusted_debug;
> +module_param_named(debug, trusted_debug, bool, 0);
> +MODULE_PARM_DESC(debug, "Enable trusted keys debug traces (default: 0)");
> +#endif
> +
> static char *trusted_key_source;
> module_param_named(source, trusted_key_source, charp, 0);
> MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee, caam, dcp or pkwm)");
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index c865c97aa1b4..b9fa2b4205cf 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -46,38 +46,44 @@ enum {
> SRK_keytype = 4
> };
>
> -#define TPM_DEBUG 0
> -
> -#if TPM_DEBUG
> +#ifdef CONFIG_TRUSTED_KEYS_DEBUG
> static inline void dump_options(struct trusted_key_options *o)
> {
> - pr_info("sealing key type %d\n", o->keytype);
> - pr_info("sealing key handle %0X\n", o->keyhandle);
> - pr_info("pcrlock %d\n", o->pcrlock);
> - pr_info("pcrinfo %d\n", o->pcrinfo_len);
> - print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> - 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> + if (!trusted_debug)
> + return;
> +
> + pr_debug("sealing key type %d\n", o->keytype);
> + pr_debug("sealing key handle %0X\n", o->keyhandle);
> + pr_debug("pcrlock %d\n", o->pcrlock);
> + pr_debug("pcrinfo %d\n", o->pcrinfo_len);
> + print_hex_dump_debug("pcrinfo ", DUMP_PREFIX_NONE,
> + 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> }
>
> static inline void dump_sess(struct osapsess *s)
> {
> - print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> - 16, 1, &s->handle, 4, 0);
> - pr_info("secret:\n");
> - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> - 16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> - pr_info("trusted-key: enonce:\n");
> - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> - 16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> + if (!trusted_debug)
> + return;
> +
> + print_hex_dump_debug("trusted-key: handle ", DUMP_PREFIX_NONE,
> + 16, 1, &s->handle, 4, 0);
> + pr_debug("secret:\n");
> + print_hex_dump_debug("", DUMP_PREFIX_NONE,
> + 16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> + pr_debug("trusted-key: enonce:\n");
> + print_hex_dump_debug("", DUMP_PREFIX_NONE,
> + 16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> }
>
> static inline void dump_tpm_buf(unsigned char *buf)
> {
> int len;
>
> - pr_info("\ntpm buffer\n");
> + if (!trusted_debug)
> + return;
> + pr_debug("\ntpm buffer\n");
> len = LOAD32(buf, TPM_SIZE_OFFSET);
> - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> + print_hex_dump_debug("", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> }
> #else
> static inline void dump_options(struct trusted_key_options *o)
^ permalink raw reply
* Re: [PATCH 04/61] ext4: Prefer IS_ERR_OR_NULL over manual NULL check
From: Theodore Ts'o @ 2026-04-10 15:18 UTC (permalink / raw)
To: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Philipp Hahn
Cc: Theodore Ts'o, Andreas Dilger
In-Reply-To: <20260310-b4-is_err_or_null-v1-4-bd63b656022d@avm.de>
On Tue, 10 Mar 2026 12:48:30 +0100, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Change generated with coccinelle.
Applied, thanks!
[04/61] ext4: Prefer IS_ERR_OR_NULL over manual NULL check
commit: 1d749e110277ce4103f27bd60d6181e52c0cc1e3
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
^ permalink raw reply
* Re: [RFC PATCH 00/20] BPF interface for applying Landlock rulesets
From: Justin Suess @ 2026-04-10 12:43 UTC (permalink / raw)
To: Mickaël Salaün
Cc: andrii, ast, bpf, brauner, daniel, eddyz87, fred, gnoack, jack,
jmorris, john.fastabend, kees, kpsingh, linux-fsdevel,
linux-kernel, linux-security-module, m, martin.lau, paul
In-Reply-To: <20260408.ainu5Chohnge@digikod.net>
On Wed, Apr 08, 2026 at 09:21:11PM +0200, Mickaël Salaün wrote:
> On Wed, Apr 08, 2026 at 01:10:28PM -0400, Justin Suess wrote:
> >
> > Add a flag LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS, which executes
> > task_set_no_new_privs on the current credentials, but only if
> > the process lacks the CAP_SYS_ADMIN capability.
> >
> > While this operation is redundant for code running from userspace
> > (indeed callers may achieve the same logic by calling
> > prctl w/ PR_SET_NO_NEW_PRIVS), this flag enables callers without access
> > to the syscall abi (defined in subsequent patches) to restrict processes
> > from gaining additional capabilities. This is important to ensure that
> > consumers can meet the task_no_new_privs || CAP_SYS_ADMIN invariant
> > enforced by Landlock without having syscall access.
> >
> > This is done by hooking bprm_committing_creds along with a
> > landlock_cred_security flag to indicate that the next execution should
> > task_set_no_new_privs if the process doesn't possess CAP_SYS_ADMIN. This
> > is done to ensure that task_set_no_new_privs is being done past the
> > point of no return.
> >
> > Cc: Mickaël Salaün <mic@digikod.net>
> > Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> > ---
> >
> > On Wed, Apr 08, 2026 at 02:00:00 -0000, Mickaël Salaün wrote:
> > > > Points of Feedback
> > > > ===
> > > >
> > > > First, the new set_nnp_on_point_of_no_return field in struct linux_binprm.
> > > > This field was needed to request that task_set_no_new_privs be set during an
> > > > execution, but only after the execution has proceeded beyond the point of no
> > > > return. I couldn't find a way to express this semantic without adding a new
> > > > bitfield to struct linux_binprm and a conditional in fs/exec.c. Please see
> > > > patch 2.
> >
> > > What about using security_bprm_committing_creds()?
> >
> > Good idea. Definitely cleaner.
> >
> > Something like this? Then dropping the "execve: Add set_nnp_on_point_of_no_return"
> > commit.
> >
> > This adds a bitfield to the landlock_cred_security struct to indicate that the flag
> > should be set on the next exec(s).
> >
> > include/uapi/linux/landlock.h | 14 ++++++++++++++
> > security/landlock/cred.c | 13 +++++++++++++
> > security/landlock/cred.h | 7 +++++++
> > security/landlock/limits.h | 2 +-
> > security/landlock/ruleset.c | 15 ++++++++++++---
> > security/landlock/syscalls.c | 5 +++++
> > 6 files changed, 52 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> > index f88fa1f68b77..edd9d9a7f60e 100644
> > --- a/include/uapi/linux/landlock.h
> > +++ b/include/uapi/linux/landlock.h
> > @@ -129,12 +129,26 @@ struct landlock_ruleset_attr {
> > *
> > * If the calling thread is running with no_new_privs, this operation
> > * enables no_new_privs on the sibling threads as well.
> > + *
> > + * %LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS
> > + * Sets no_new_privs on the calling thread before applying the Landlock domain.
> > + * This flag is useful for convenience as well as for applying a ruleset from
> > + * an outside context (e.g BPF). This flag only has an effect on when both
> > + * no_new_privs isn't already set and the caller doesn't possess CAP_SYS_ADMIN.
> > + *
> > + * This flag has slightly different behavior when used from BPF. Instead of
> > + * setting no_new_privs on the current task, it sets a flag on the bprm so that
> > + * no_new_privs is set on the task at exec point-of-no-return. This guarantees
> > + * that the current execution is unaffected, and may escalate as usual until the
> > + * next exec, but the resulting task cannot gain more privileges through later
> > + * exec transitions.
> > */
> > /* clang-format off */
> > #define LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF (1U << 0)
> > #define LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON (1U << 1)
> > #define LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF (1U << 2)
> > #define LANDLOCK_RESTRICT_SELF_TSYNC (1U << 3)
> > +#define LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS (1U << 4)
> > /* clang-format on */
> >
> > /**
> > diff --git a/security/landlock/cred.c b/security/landlock/cred.c
> > index 0cb3edde4d18..bcc9b716916f 100644
> > --- a/security/landlock/cred.c
> > +++ b/security/landlock/cred.c
> > @@ -43,6 +43,18 @@ static void hook_cred_free(struct cred *const cred)
> > landlock_put_ruleset_deferred(dom);
> > }
> >
> > +static void hook_bprm_committing_creds(const struct linux_binprm *bprm)
> > +{
> > + struct landlock_cred_security *const llcred = landlock_cred(bprm->cred);
> > +
> > + if (llcred->set_nnp_on_committing_creds &&
> > + !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN)) {
>
> If asked by the caller, NNP must be set, whatever the capabilities of
> the task.
>
Gotcha. I suppose checking the capability is possible from BPF anyway
(at least from bprm_creds_from_file) so that makes sense.
> > + task_set_no_new_privs(current);
> > + /* Don't need to set it again for subsequent execution. */
> > + llcred->set_nnp_on_committing_creds = false;
> > + }
>
> Thinking more about it, it would make more sense to add another flag to
> enforce restriction on the next exec. This new cred bit would then be
> generic and enforce both NNP (if set) and the domain once we know the
Problem is enforcing NNP after the escalation (and past the point of no
return) is NOT safe from userspace side, (at least not without CAP_SYS_ADMIN
already)
Imagine this (contrived) scenario where Landlock enforces NNP after the
point of no return:
1. Sudo is configured like this: (some system file is critical to
enforcing policy)
/etc/sudoers.d/policy.blah.conf
/etc/sudoers.d/policy.keep_bob_out.conf
2. Bob creates a program that enforces a landlock ruleset forbidding
access to /etc/sudoers.d/policy.keep_bob_out.conf but allowing access to
other configs. Then it launches sudo /bin/sh
3. Bob can now escalate because the policy file keeping him out could
not be read. NNP is only enforced after exec, so NNP only takes
place after sudo escalates already.
This is just an example, but there are other cases I'm probably not
thinking of where it's dangerous to bypass the NNP check and enforce it
on the next exec.
To be safe, NNP must be enforced BEFORE the escalation in the
unprivileged side, but problem is the escalation happens just
before the point of no return, so exec may still fail!
So the conditions
1. NNP must happen after exec cannot fail, to not leave
side effects.
2. NNP must happen before escalation, to avoid confused deputy attacks.
Are currently unsatisfiable.
> execution is ok. That should also bring the required plumbing to
> create the domain at syscall (or kfunc) time and handle memory
> allocation issue there, but only enforce it at exec time with
> security_bprm_committing_creds() (without any possible error).
>
I like that flow.
I guess this poses the question about what happens if a ruleset is asked
for "on next exec" from userspace and then
bpf_landlock_restrict_binprm() is called during the same execution?
Which would get priority? Would they
be merged? (etc). What happens if one requests NNP and the other
doesn't?
This needs some thought.
> > +}
> > +
> > #ifdef CONFIG_AUDIT
> >
> > static int hook_bprm_creds_for_exec(struct linux_binprm *const bprm)
> > @@ -55,6 +67,7 @@ static int hook_bprm_creds_for_exec(struct linux_binprm *const bprm)
> > #endif /* CONFIG_AUDIT */
> >
> > static struct security_hook_list landlock_hooks[] __ro_after_init = {
> > + LSM_HOOK_INIT(bprm_committing_creds, hook_bprm_committing_creds),
> > LSM_HOOK_INIT(cred_prepare, hook_cred_prepare),
> > LSM_HOOK_INIT(cred_transfer, hook_cred_transfer),
> > LSM_HOOK_INIT(cred_free, hook_cred_free),
> > diff --git a/security/landlock/cred.h b/security/landlock/cred.h
> > index c10a06727eb1..7ec6dd12ebc3 100644
> > --- a/security/landlock/cred.h
> > +++ b/security/landlock/cred.h
> > @@ -49,6 +49,13 @@ struct landlock_cred_security {
> > * not require a current domain.
> > */
> > u8 log_subdomains_off : 1;
> > + /**
> > + * @set_nnp_on_committing_creds: Set if the domain should set NO_NEW_PRIVS on the
> > + * execution past the point of no return in security_bprm_committing_creds().
> > + * This is not a hierarchy configuration because the nnp state is inherited by
> > + * exec and doesn't need further configuration.
> > + */
> > + u8 set_nnp_on_committing_creds : 1;
> > #endif /* CONFIG_AUDIT */
> > } __packed;
> >
> > diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> > index eb584f47288d..d298086a4180 100644
> > --- a/security/landlock/limits.h
> > +++ b/security/landlock/limits.h
> > @@ -31,7 +31,7 @@
> > #define LANDLOCK_MASK_SCOPE ((LANDLOCK_LAST_SCOPE << 1) - 1)
> > #define LANDLOCK_NUM_SCOPE __const_hweight64(LANDLOCK_MASK_SCOPE)
> >
> > -#define LANDLOCK_LAST_RESTRICT_SELF LANDLOCK_RESTRICT_SELF_TSYNC
> > +#define LANDLOCK_LAST_RESTRICT_SELF LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS
> > #define LANDLOCK_MASK_RESTRICT_SELF ((LANDLOCK_LAST_RESTRICT_SELF << 1) - 1)
> >
> > /* clang-format on */
> > diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> > index 1d6fa74f2a52..ad0bd5994ec5 100644
> > --- a/security/landlock/ruleset.c
> > +++ b/security/landlock/ruleset.c
> > @@ -121,11 +121,13 @@ int landlock_restrict_cred_precheck(const __u32 flags,
> >
> > /*
> > * Similar checks as for seccomp(2), except that an -EPERM may be
> > - * returned.
> > + * returned, or no_new_privs may be set by the caller via
> > + * LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS.
> > */
> > if (!task_no_new_privs(current) &&
> > !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN)) {
> > - return -EPERM;
> > + if (!(flags & LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS))
> > + return -EPERM;
> > }
> >
> > if (flags & ~LANDLOCK_MASK_RESTRICT_SELF)
> > @@ -140,7 +142,7 @@ int landlock_restrict_cred(struct cred *const cred,
> > {
> > struct landlock_cred_security *new_llcred;
> > bool __maybe_unused log_same_exec, log_new_exec, log_subdomains,
> > - prev_log_subdomains;
> > + prev_log_subdomains, set_nnp_on_committing_creds;
> >
> > /*
> > * It is allowed to set LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF without
> > @@ -157,6 +159,12 @@ int landlock_restrict_cred(struct cred *const cred,
> > log_new_exec = !!(flags & LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON);
> > /* Translates "off" flag to boolean. */
> > log_subdomains = !(flags & LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF);
> > + /*
> > + * Translates "on" flag to boolean. This flag is not inherited by exec,
> > + * but the resulting nnp state is.
> > + */
> > + set_nnp_on_committing_creds =
> > + !!(flags & LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS);
> >
> > new_llcred = landlock_cred(cred);
> >
> > @@ -165,6 +173,7 @@ int landlock_restrict_cred(struct cred *const cred,
> > new_llcred->log_subdomains_off = !prev_log_subdomains ||
> > !log_subdomains;
> > #endif /* CONFIG_AUDIT */
> > + new_llcred->set_nnp_on_committing_creds = set_nnp_on_committing_creds;
> >
> > /*
> > * The only case when a ruleset may not be set is if
> > diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> > index c6c7be7698a2..f3520c764360 100644
> > --- a/security/landlock/syscalls.c
> > +++ b/security/landlock/syscalls.c
> > @@ -397,6 +397,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
> > * - %LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON
> > * - %LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF
> > * - %LANDLOCK_RESTRICT_SELF_TSYNC
> > + * - %LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS
> > *
> > * This system call enforces a Landlock ruleset on the current thread.
> > * Enforcing a ruleset requires that the task has %CAP_SYS_ADMIN in its
> > @@ -450,6 +451,10 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> > if (!new_cred)
> > return -ENOMEM;
> >
> > + if (flags & LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS &&
> > + !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN))
> > + task_set_no_new_privs(current);
> > +
> > err = landlock_restrict_cred(new_cred, ruleset, flags);
> > if (err) {
> > abort_creds(new_cred);
> > --
> > 2.53.0
> >
> >
^ permalink raw reply
* [bug report] apparmor: add support loading per permission tagging
From: Dan Carpenter @ 2026-04-10 10:16 UTC (permalink / raw)
To: John Johansen; +Cc: apparmor, linux-security-module
Hello John Johansen,
Commit 3d28e2397af7 ("apparmor: add support loading per permission
tagging") from Apr 1, 2025 (linux-next), leads to the following
Smatch static checker warning:
security/apparmor/policy_unpack.c:883 unpack_tags()
warn: missing error code 'error'
security/apparmor/policy_unpack.c
852 static int unpack_tags(struct aa_ext *e, struct aa_tags_struct *tags,
853 const char **info)
854 {
855 int error = -EPROTO;
856 void *pos = e->pos;
857
858 AA_BUG(!tags);
859 /* policy tags are optional */
860 if (aa_unpack_nameX(e, AA_STRUCT, "tags")) {
861 u32 version;
862
863 if (!aa_unpack_u32(e, &version, "version") || version != 1) {
864 *info = "invalid tags version";
865 goto fail_reset;
866 }
867 error = unpack_strs_table(e, "strs", true, &tags->strs);
868 if (error) {
869 *info = "failed to unpack profile tag.strs";
870 goto fail;
871 }
872 error = unpack_tag_headers(e, tags);
873 if (error) {
874 *info = "failed to unpack profile tag.headers";
875 goto fail;
876 }
877 error = unpack_tagsets(e, tags);
878 if (error) {
879 *info = "failed to unpack profile tag.sets";
880 goto fail;
881 }
882 if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL))
--> 883 goto fail;
set the error code here
884
885 if (!verify_tags(tags, info))
886 goto fail;
and here
887 }
888
889 return 0;
890
891 fail:
892 aa_destroy_tags(tags);
893 fail_reset:
894 e->pos = pos;
895 return error;
896 }
This email is a free service from the Smatch-CI project [smatch.sf.net].
regards,
dan carpenter
^ permalink raw reply
* Re: [RFC PATCH v1 01/11] security: add LSM blob and hooks for namespaces
From: Christian Brauner @ 2026-04-10 9:35 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Paul Moore, Serge E . Hallyn, Justin Suess,
Lennart Poettering, Mikhail Ivanov, Nicolas Bouchinet,
Shervin Oloumi, Tingmao Wang, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module, Daniel Durning
In-Reply-To: <20260409.Mei6Yei0beeZ@digikod.net>
On Thu, Apr 09, 2026 at 06:40:03PM +0200, Mickaël Salaün wrote:
> On Wed, Mar 25, 2026 at 01:31:30PM +0100, Christian Brauner wrote:
> > On Thu, Mar 12, 2026 at 11:04:34AM +0100, Mickaël Salaün wrote:
> > > From: Christian Brauner <brauner@kernel.org>
> > >
> > > All namespace types now share the same ns_common infrastructure. Extend
> > > this to include a security blob so LSMs can start managing namespaces
> > > uniformly without having to add one-off hooks or security fields to
> > > every individual namespace type.
> > >
> > > Add a ns_security pointer to ns_common and the corresponding lbs_ns
> > > blob size to lsm_blob_sizes. Allocation and freeing hooks are called
> > > from the common __ns_common_init() and __ns_common_free() paths so
> > > every namespace type gets covered in one go. All information about the
> > > namespace type and the appropriate casting helpers to get at the
> > > containing namespace are available via ns_common making it
> > > straightforward for LSMs to differentiate when they need to.
> > >
> > > A namespace_install hook is called from validate_ns() during setns(2)
> > > giving LSMs a chance to enforce policy on namespace transitions.
> > >
> > > Individual namespace types can still have their own specialized security
> > > hooks when needed. This is just the common baseline that makes it easy
> > > to track and manage namespaces from the security side without requiring
> > > every namespace type to reinvent the wheel.
> > >
> > > Cc: Günther Noack <gnoack@google.com>
> > > Cc: Paul Moore <paul@paul-moore.com>
> > > Cc: Serge E. Hallyn <serge@hallyn.com>
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > Link: https://lore.kernel.org/r/20260216-work-security-namespace-v1-1-075c28758e1f@kernel.org
> > > ---
> > > include/linux/lsm_hook_defs.h | 3 ++
> > > include/linux/lsm_hooks.h | 1 +
> > > include/linux/ns/ns_common_types.h | 3 ++
> > > include/linux/security.h | 20 ++++++++
> > > kernel/nscommon.c | 12 +++++
> > > kernel/nsproxy.c | 8 +++-
> > > security/lsm_init.c | 2 +
> > > security/security.c | 76 ++++++++++++++++++++++++++++++
> > > 8 files changed, 124 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> > > index 8c42b4bde09c..fefd3aa6d8f4 100644
> > > --- a/include/linux/lsm_hook_defs.h
> > > +++ b/include/linux/lsm_hook_defs.h
> > > @@ -260,6 +260,9 @@ LSM_HOOK(int, -ENOSYS, task_prctl, int option, unsigned long arg2,
> > > LSM_HOOK(void, LSM_RET_VOID, task_to_inode, struct task_struct *p,
> > > struct inode *inode)
> > > LSM_HOOK(int, 0, userns_create, const struct cred *cred)
> > > +LSM_HOOK(int, 0, namespace_alloc, struct ns_common *ns)
> > > +LSM_HOOK(void, LSM_RET_VOID, namespace_free, struct ns_common *ns)
> > > +LSM_HOOK(int, 0, namespace_install, const struct nsset *nsset, struct ns_common *ns)
> > > LSM_HOOK(int, 0, ipc_permission, struct kern_ipc_perm *ipcp, short flag)
> > > LSM_HOOK(void, LSM_RET_VOID, ipc_getlsmprop, struct kern_ipc_perm *ipcp,
> > > struct lsm_prop *prop)
> > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > > index d48bf0ad26f4..3e7afe76e86c 100644
> > > --- a/include/linux/lsm_hooks.h
> > > +++ b/include/linux/lsm_hooks.h
> > > @@ -111,6 +111,7 @@ struct lsm_blob_sizes {
> > > unsigned int lbs_ipc;
> > > unsigned int lbs_key;
> > > unsigned int lbs_msg_msg;
> > > + unsigned int lbs_ns;
> > > unsigned int lbs_perf_event;
> > > unsigned int lbs_task;
> > > unsigned int lbs_xattr_count; /* num xattr slots in new_xattrs array */
> > > diff --git a/include/linux/ns/ns_common_types.h b/include/linux/ns/ns_common_types.h
> > > index 0014fbc1c626..170288e2e895 100644
> > > --- a/include/linux/ns/ns_common_types.h
> > > +++ b/include/linux/ns/ns_common_types.h
> > > @@ -115,6 +115,9 @@ struct ns_common {
> > > struct dentry *stashed;
> > > const struct proc_ns_operations *ops;
> > > unsigned int inum;
> > > +#ifdef CONFIG_SECURITY
> > > + void *ns_security;
> > > +#endif
> > > union {
> > > struct ns_tree;
> > > struct rcu_head ns_rcu;
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> > > index 83a646d72f6f..611b9098367d 100644
> > > --- a/include/linux/security.h
> > > +++ b/include/linux/security.h
> > > @@ -67,6 +67,7 @@ enum fs_value_type;
> > > struct watch;
> > > struct watch_notification;
> > > struct lsm_ctx;
> > > +struct nsset;
> > >
> > > /* Default (no) options for the capable function */
> > > #define CAP_OPT_NONE 0x0
> > > @@ -80,6 +81,7 @@ struct lsm_ctx;
> > >
> > > struct ctl_table;
> > > struct audit_krule;
> > > +struct ns_common;
> > > struct user_namespace;
> > > struct timezone;
> > >
> > > @@ -533,6 +535,9 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> > > unsigned long arg4, unsigned long arg5);
> > > void security_task_to_inode(struct task_struct *p, struct inode *inode);
> > > int security_create_user_ns(const struct cred *cred);
> > > +int security_namespace_alloc(struct ns_common *ns);
> > > +void security_namespace_free(struct ns_common *ns);
> > > +int security_namespace_install(const struct nsset *nsset, struct ns_common *ns);
> > > int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag);
> > > void security_ipc_getlsmprop(struct kern_ipc_perm *ipcp, struct lsm_prop *prop);
> > > int security_msg_msg_alloc(struct msg_msg *msg);
> > > @@ -1407,6 +1412,21 @@ static inline int security_create_user_ns(const struct cred *cred)
> > > return 0;
> > > }
> > >
> > > +static inline int security_namespace_alloc(struct ns_common *ns)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline void security_namespace_free(struct ns_common *ns)
> > > +{
> > > +}
> > > +
> > > +static inline int security_namespace_install(const struct nsset *nsset,
> > > + struct ns_common *ns)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > static inline int security_ipc_permission(struct kern_ipc_perm *ipcp,
> > > short flag)
> > > {
> > > diff --git a/kernel/nscommon.c b/kernel/nscommon.c
> > > index bdc3c86231d3..de774e374f9d 100644
> > > --- a/kernel/nscommon.c
> > > +++ b/kernel/nscommon.c
> > > @@ -4,6 +4,7 @@
> > > #include <linux/ns_common.h>
> > > #include <linux/nstree.h>
> > > #include <linux/proc_ns.h>
> > > +#include <linux/security.h>
> > > #include <linux/user_namespace.h>
> > > #include <linux/vfsdebug.h>
> > >
> > > @@ -59,6 +60,9 @@ int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_ope
> > >
> > > refcount_set(&ns->__ns_ref, 1);
> > > ns->stashed = NULL;
> > > +#ifdef CONFIG_SECURITY
> > > + ns->ns_security = NULL;
> > > +#endif
> > > ns->ops = ops;
> > > ns->ns_id = 0;
> > > ns->ns_type = ns_type;
> > > @@ -77,6 +81,13 @@ int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_ope
> > > ret = proc_alloc_inum(&ns->inum);
> > > if (ret)
> > > return ret;
> > > +
> > > + ret = security_namespace_alloc(ns);
> > > + if (ret) {
> > > + proc_free_inum(ns->inum);
> >
> > ret = security_namespace_alloc(ns);
> > if (ret && !inum)
> > proc_free_inum(ns->inum);
> > return ret;
> >
> >
> > > + return ret;
> > > + }
> > > +
> > > /*
> > > * Tree ref starts at 0. It's incremented when namespace enters
> > > * active use (installed in nsproxy) and decremented when all
> > > @@ -91,6 +102,7 @@ int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_ope
> > >
> > > void __ns_common_free(struct ns_common *ns)
> > > {
> > > + security_namespace_free(ns);
> > > proc_free_inum(ns->inum);
> > > }
> > >
> > > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
> > > index 259c4b4f1eeb..f0b30d1907e7 100644
> > > --- a/kernel/nsproxy.c
> > > +++ b/kernel/nsproxy.c
> > > @@ -379,7 +379,13 @@ static int prepare_nsset(unsigned flags, struct nsset *nsset)
> > >
> > > static inline int validate_ns(struct nsset *nsset, struct ns_common *ns)
> > > {
> > > - return ns->ops->install(nsset, ns);
> > > + int ret;
> > > +
> > > + ret = ns->ops->install(nsset, ns);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + return security_namespace_install(nsset, ns);
> >
> > In my local tree I had that moved before the ->install() and I think
> > that's the correct thing to do. So please switch to that.
>
> Looks good, I'll include your fixes in the next version.
Thanks!
>
> >
> > The rest looks good to me, thanks.
>
> Another issue raised by Daniel Durning [1] is freeing of anonymous
> namespaces.
>
> I'll extend this patch with this new hunk if that's ok:
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 854f4fc66469..f6977e59be7d 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -4186,6 +4186,8 @@ static void free_mnt_ns(struct mnt_namespace *ns)
> {
> if (!is_anon_ns(ns))
> ns_common_free(ns);
> + else
> + security_namespace_free(&ns->ns);
> dec_mnt_namespaces(ns->ucounts);
> mnt_ns_tree_remove(ns);
> }
I think that's fixing it at the wrong layer. It's probably better to do
sm like:
diff --git a/include/uapi/linux/nsfs.h b/include/uapi/linux/nsfs.h
index a25e38d1c874..ea0f0267d90f 100644
--- a/include/uapi/linux/nsfs.h
+++ b/include/uapi/linux/nsfs.h
@@ -55,6 +55,7 @@ enum init_ns_ino {
MNT_NS_INIT_INO = 0xEFFFFFF8U,
#ifdef __KERNEL__
MNT_NS_ANON_INO = 0xEFFFFFF7U,
+ MNT_NS_INO_SPECIAL_MAX = MNT_NS_ANON_INO,
#endif
};
diff --git a/kernel/nscommon.c b/kernel/nscommon.c
index 3166c1fd844a..e7a3dd2189cc 100644
--- a/kernel/nscommon.c
+++ b/kernel/nscommon.c
@@ -91,7 +91,10 @@ int __ns_common_init(struct ns_common *ns, u32 ns_type, const struct proc_ns_ope
void __ns_common_free(struct ns_common *ns)
{
- proc_free_inum(ns->inum);
+ security_namespace_free(&ns->ns);
+
+ if (ns->inum > MNT_NS_INO_SPECIAL_MAX)
+ proc_free_inum(ns->inum);
}
struct ns_common *__must_check ns_owner(struct ns_common *ns)
>
> Daniel, could you please confirm that this fixes the memory leak?
>
> [1] https://lore.kernel.org/all/20260330193100.3603-1-danieldurning.work@gmail.com/
>
>
> > > +/**
> > > + * security_namespace_free() - Release LSM security data from a namespace
> > > + * @ns: the namespace being freed
> > > + *
> > > + * Release security data attached to the namespace. Called before the
> > > + * namespace structure is freed.
> > > + *
> > > + * Note: The namespace may be freed via kfree_rcu(). LSMs must use
> > > + * RCU-safe freeing for any data that might be accessed by concurrent
> > > + * RCU readers.
> > > + */
> > > +void security_namespace_free(struct ns_common *ns)
> > > +{
> > > + if (!ns->ns_security)
> > > + return;
> > > +
> > > + call_void_hook(namespace_free, ns);
> > > +
>
> > > + kfree(ns->ns_security);
> > > + ns->ns_security = NULL;
>
> I think it would be safer to replace these two lines with:
> kfree_rcu_mightsleep(ns->ns_security)
>
> > > +}
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox