* Re: [PATCH RESEND] apparmor: Replace deprecated strcpy with memcpy in gen_symlink_name
From: John Johansen @ 2025-11-27 1:32 UTC (permalink / raw)
To: Thorsten Blum, Paul Moore, James Morris, Serge E. Hallyn
Cc: apparmor, linux-security-module, linux-kernel
In-Reply-To: <20251126165701.97158-2-thorsten.blum@linux.dev>
On 11/26/25 08:57, Thorsten Blum wrote:
> strcpy() is deprecated; use memcpy() instead. Unlike strcpy(), memcpy()
> does not copy the NUL terminator from the source string, which would be
> overwritten anyway on every iteration when using strcpy(). snprintf()
> then ensures that 'char *s' is NUL-terminated.
>
> Replace the hard-coded path length to remove the magic number 6, and add
> a comment explaining the extra 11 bytes.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
hey Thorsten,
sorry I have actually pulled these in, and tested them. I didn't send out
the acks yet because I have another patch that I was waiting on a proper
signed-off-by: on.
I am going to have to pull that one so I can push. I'll add acks now but
the push isn't going to happen for a few hours.
Acked-by: John Johansen <john.johansen@canonical.com>
> security/apparmor/apparmorfs.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 391a586d0557..4b2752200ce2 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -1602,16 +1602,20 @@ static char *gen_symlink_name(int depth, const char *dirname, const char *fname)
> {
> char *buffer, *s;
> int error;
> - int size = depth * 6 + strlen(dirname) + strlen(fname) + 11;
> + const char *path = "../../";
> + size_t path_len = strlen(path);
> + int size;
>
> + /* Extra 11 bytes: "raw_data" (9) + two slashes "//" (2) */
> + size = depth * path_len + strlen(dirname) + strlen(fname) + 11;
> s = buffer = kmalloc(size, GFP_KERNEL);
> if (!buffer)
> return ERR_PTR(-ENOMEM);
>
> for (; depth > 0; depth--) {
> - strcpy(s, "../../");
> - s += 6;
> - size -= 6;
> + memcpy(s, path, path_len);
> + s += path_len;
> + size -= path_len;
> }
>
> error = snprintf(s, size, "raw_data/%s/%s", dirname, fname);
^ permalink raw reply
* [PATCH bpf-next 3/3] selftests/bpf: Add tests for bpf_kern_path kfunc
From: Song Liu @ 2025-11-27 0:50 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-security-module
Cc: ast, daniel, andrii, kernel-team, viro, brauner, jack, paul,
jmorris, serge, Song Liu
In-Reply-To: <20251127005011.1872209-1-song@kernel.org>
Add comprehensive selftests for the new bpf_kern_path and bpf_path_put
kfuncs:
1. Functional tests (prog_tests/kern_path.c, progs/test_kern_path.c):
- test_kern_path_basic: Tests successful path resolution using
/proc/self/exe and validates the resolved path with bpf_path_d_path
- test_kern_path_sb_mount: Tests bpf_kern_path with dynamic input
from LSM hook parameter (dev_name from sb_mount), demonstrating
real-world usage where BPF programs resolve paths from hook args
2. Verifier success tests (progs/verifier_kern_path.c):
- kern_path_success: Proper acquire -> use -> release pattern
- kern_path_multiple_paths: Multiple concurrent path acquisitions
3. Verifier failure tests (progs/verifier_kern_path_fail.c):
- kern_path_unreleased: Resource leak detection
- path_put_unacquired: Releasing unacquired path
- path_use_after_put: Use-after-free detection
- double_path_put: Double-free detection
- kern_path_non_lsm: Program type restrictions (LSM only)
- kern_path_non_const_str: reject none const string
These tests verify both the functionality of the kfuncs and that the
verifier properly enforces acquire/release semantics to prevent
resource leaks.
Signed-off-by: Song Liu <song@kernel.org>
---
.../testing/selftests/bpf/bpf_experimental.h | 4 +
.../selftests/bpf/prog_tests/kern_path.c | 82 ++++++++++++++++
.../selftests/bpf/progs/test_kern_path.c | 56 +++++++++++
.../selftests/bpf/progs/verifier_kern_path.c | 52 ++++++++++
.../bpf/progs/verifier_kern_path_fail.c | 97 +++++++++++++++++++
5 files changed, 291 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/test_kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/verifier_kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/verifier_kern_path_fail.c
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 2cd9165c7348..c512c9a14752 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -221,6 +221,10 @@ extern void bpf_put_file(struct file *file) __ksym;
*/
extern int bpf_path_d_path(const struct path *path, char *buf, size_t buf__sz) __ksym;
+extern struct path *bpf_kern_path(const char *pathname, unsigned int flags) __ksym;
+extern void bpf_path_put(struct path *path) __ksym;
+extern int bpf_path_d_path(const struct path *path, char *buf, size_t buf__sz) __ksym;
+
/* This macro must be used to mark the exception callback corresponding to the
* main program. For example:
*
diff --git a/tools/testing/selftests/bpf/prog_tests/kern_path.c b/tools/testing/selftests/bpf/prog_tests/kern_path.c
new file mode 100644
index 000000000000..f4cdfe202a26
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/kern_path.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. */
+
+#include <test_progs.h>
+#include <sys/mount.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include "test_kern_path.skel.h"
+#include "verifier_kern_path.skel.h"
+#include "verifier_kern_path_fail.skel.h"
+
+static void __test_kern_path(void (*trigger)(void))
+{
+ struct test_kern_path *skel;
+ int err;
+
+ skel = test_kern_path__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "test_kern_path__open_and_load"))
+ return;
+
+ skel->bss->monitored_pid = getpid();
+
+ err = test_kern_path__attach(skel);
+ if (!ASSERT_OK(err, "test_kern_path__attach"))
+ goto cleanup;
+
+ trigger();
+
+ /* Verify the bpf_path_d_path worked */
+ ASSERT_GT(skel->bss->path_len, 0, "path_len > 0");
+
+cleanup:
+ test_kern_path__destroy(skel);
+}
+
+static void trigger_file_open(void)
+{
+ int fd;
+
+ fd = open("/dev/null", O_RDONLY);
+ if (!ASSERT_OK_FD(fd, "open /dev/null"))
+ return;
+ close(fd);
+}
+
+static void trigger_sb_mount(void)
+{
+ char tmpdir[] = "/tmp/bpf_kern_path_test_XXXXXX";
+ int err;
+
+ if (!ASSERT_OK_PTR(mkdtemp(tmpdir), "mkdtemp"))
+ return;
+
+ err = mount("/tmp", tmpdir, NULL, MS_BIND, NULL);
+ if (!ASSERT_OK(err, "bind mount"))
+ goto rmdir;
+
+ umount(tmpdir);
+rmdir:
+ rmdir(tmpdir);
+}
+
+void test_kern_path(void)
+{
+ if (test__start_subtest("file_open"))
+ __test_kern_path(trigger_file_open);
+
+ if (test__start_subtest("sb_mount"))
+ __test_kern_path(trigger_sb_mount);
+}
+
+void test_verifier_kern_path(void)
+{
+ RUN_TESTS(verifier_kern_path);
+}
+
+void test_verifier_kern_path_fail(void)
+{
+ RUN_TESTS(verifier_kern_path_fail);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_kern_path.c b/tools/testing/selftests/bpf/progs/test_kern_path.c
new file mode 100644
index 000000000000..e9186a1aa990
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_kern_path.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. */
+
+#include "vmlinux.h"
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+#define MAX_PATH_LEN 256
+
+char buf[MAX_PATH_LEN];
+int path_len = 0;
+u32 monitored_pid = 0;
+
+SEC("lsm.s/file_open")
+int BPF_PROG(test_kern_path_basic, struct file *file)
+{
+ struct path *p;
+ int ret;
+
+ if (bpf_get_current_pid_tgid() >> 32 != monitored_pid)
+ return 0;
+
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (p) {
+ ret = bpf_path_d_path(p, buf, MAX_PATH_LEN);
+ if (ret > 0)
+ path_len = ret;
+ bpf_path_put(p);
+ }
+
+ return 0;
+}
+
+SEC("lsm.s/sb_mount")
+int BPF_PROG(test_kern_path_from_sb_mount, const char *dev_name, const struct path *path,
+ const char *type, unsigned long flags, void *data)
+{
+ struct path *p;
+ int ret;
+
+ if (bpf_get_current_pid_tgid() >> 32 != monitored_pid)
+ return 0;
+
+ p = bpf_kern_path(dev_name, 0);
+ if (p) {
+ ret = bpf_path_d_path(p, buf, MAX_PATH_LEN);
+ if (ret > 0)
+ path_len = ret;
+ bpf_path_put(p);
+ }
+
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/verifier_kern_path.c b/tools/testing/selftests/bpf/progs/verifier_kern_path.c
new file mode 100644
index 000000000000..0e6ccf640b64
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_kern_path.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include <linux/limits.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+static char buf[PATH_MAX];
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(kern_path_success)
+{
+ struct path *p;
+
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (!p)
+ return 0;
+
+ bpf_path_d_path(p, buf, sizeof(buf));
+
+ bpf_path_put(p);
+ return 0;
+}
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(kern_path_multiple_paths)
+{
+ struct path *p1, *p2;
+
+ p1 = bpf_kern_path("/proc/self/exe", 0);
+ if (!p1)
+ return 0;
+
+ p2 = bpf_kern_path("/proc/self/cwd", 0);
+ if (!p2) {
+ bpf_path_put(p1);
+ return 0;
+ }
+
+ bpf_path_d_path(p1, buf, sizeof(buf));
+ bpf_path_d_path(p2, buf, sizeof(buf));
+
+ bpf_path_put(p2);
+ bpf_path_put(p1);
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/verifier_kern_path_fail.c b/tools/testing/selftests/bpf/progs/verifier_kern_path_fail.c
new file mode 100644
index 000000000000..520c227af5ca
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_kern_path_fail.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include <linux/limits.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+static char buf[PATH_MAX];
+
+SEC("lsm.s/file_open")
+__failure __msg("Unreleased reference")
+int BPF_PROG(kern_path_unreleased)
+{
+ struct path *p;
+
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (!p)
+ return 0;
+
+ /* Acquired but never released - should fail verification */
+ return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("pointer type STRUCT path must point to scalar, or struct with scalar")
+int BPF_PROG(path_put_unacquired)
+{
+ struct path p = {};
+
+ /* Can't release an unacquired path - should fail verification */
+ bpf_path_put(&p);
+ return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("pointer type STRUCT path must point to scalar, or struct with scalar")
+int BPF_PROG(path_use_after_put, struct file *file)
+{
+ struct path *p;
+
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (!p)
+ return 0;
+
+ bpf_path_put(p);
+
+ /* Using path after put - should fail verification */
+ bpf_path_d_path(p, buf, sizeof(buf));
+ return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("pointer type STRUCT path must point to scalar, or struct with scalar")
+int BPF_PROG(double_path_put)
+{
+ struct path *p;
+
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (!p)
+ return 0;
+
+ bpf_path_put(p);
+ /* Double put - should fail verification */
+ bpf_path_put(p);
+ return 0;
+}
+
+SEC("fentry/vfs_open")
+__failure __msg("calling kernel function bpf_kern_path is not allowed")
+int BPF_PROG(kern_path_non_lsm)
+{
+ struct path *p;
+
+ /* Calling bpf_kern_path() from a non-LSM BPF program isn't permitted */
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (p)
+ bpf_path_put(p);
+ return 0;
+}
+
+SEC("lsm.s/sb_eat_lsm_opts")
+__failure __msg("arg#0 doesn't point to a const string")
+int BPF_PROG(kern_path_non_const_str, char *options, void **mnt_opts)
+{
+ struct path *p;
+
+ /* Calling bpf_kern_path() from a with non-const string isn't permitted */
+ p = bpf_kern_path(options, 0);
+ if (p)
+ bpf_path_put(p);
+ return 0;
+}
+
+
+char _license[] SEC("license") = "GPL";
--
2.47.3
^ permalink raw reply related
* [PATCH bpf-next 2/3] bpf: Add bpf_kern_path and bpf_path_put kfuncs
From: Song Liu @ 2025-11-27 0:50 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-security-module
Cc: ast, daniel, andrii, kernel-team, viro, brauner, jack, paul,
jmorris, serge, Song Liu
In-Reply-To: <20251127005011.1872209-1-song@kernel.org>
Add two new kfuncs to fs/bpf_fs_kfuncs.c that wrap kern_path() for use
by BPF LSM programs:
bpf_kern_path():
- Resolves a pathname string to a struct path
- Allocates memory for the path structure
- Returns NULL on error or if the path doesn't exist
- Marked with KF_ACQUIRE | KF_SLEEPABLE | KF_RET_NULL
bpf_path_put():
- Releases the path reference and frees the allocated memory
- Marked with KF_RELEASE to enforce acquire/release semantics
These kfuncs enable BPF LSM programs to resolve pathnames provided by
hook arguments (e.g., dev_name from sb_mount) and validate or inspect
the resolved paths. The verifier enforces proper resource management
through acquire/release tracking.
Example usage:
struct path *p = bpf_kern_path("/etc/passwd", LOOKUP_FOLLOW);
if (p) {
// Use the path...
bpf_path_put(p); // Must release
}
Signed-off-by: Song Liu <song@kernel.org>
---
fs/bpf_fs_kfuncs.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 5ace2511fec5..977f8dcbc208 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -11,6 +11,7 @@
#include <linux/file.h>
#include <linux/kernfs.h>
#include <linux/mm.h>
+#include <linux/namei.h>
#include <linux/xattr.h>
__bpf_kfunc_start_defs();
@@ -96,6 +97,61 @@ __bpf_kfunc int bpf_path_d_path(const struct path *path, char *buf, size_t buf__
return len;
}
+/**
+ * bpf_kern_path - resolve a pathname to a struct path
+ * @pathname__str: pathname to resolve
+ * @flags: lookup flags (e.g., LOOKUP_FOLLOW)
+ *
+ * Resolve the pathname for the supplied *pathname__str* and return a pointer
+ * to a struct path. This is a wrapper around kern_path() that allocates and
+ * returns a struct path pointer on success.
+ *
+ * The returned struct path pointer must be released using bpf_path_put().
+ * Failing to call bpf_path_put() on the returned struct path pointer will
+ * result in the BPF program being rejected by the BPF verifier.
+ *
+ * This BPF kfunc may only be called from BPF LSM programs.
+ *
+ * Return: A pointer to an allocated struct path on success, NULL on error.
+ */
+__bpf_kfunc struct path *bpf_kern_path(const char *pathname__str, unsigned int flags)
+{
+ struct path *path;
+ int ret;
+
+ path = kmalloc(sizeof(*path), GFP_KERNEL);
+ if (!path)
+ return NULL;
+
+ ret = kern_path(pathname__str, flags, path);
+ if (ret) {
+ kfree(path);
+ return NULL;
+ }
+
+ return path;
+}
+
+/**
+ * bpf_path_put - release a struct path reference
+ * @path: struct path pointer to release
+ *
+ * Release the struct path pointer that was acquired by bpf_kern_path().
+ * This BPF kfunc calls path_put() on the supplied *path* and then frees
+ * the allocated memory.
+ *
+ * Only struct path pointers acquired by bpf_kern_path() may be passed to
+ * this BPF kfunc. Attempting to pass any other pointer will result in the
+ * BPF program being rejected by the BPF verifier.
+ *
+ * This BPF kfunc may only be called from BPF LSM programs.
+ */
+__bpf_kfunc void bpf_path_put(struct path *path)
+{
+ path_put(path);
+ kfree(path);
+}
+
static bool match_security_bpf_prefix(const char *name__str)
{
return !strncmp(name__str, XATTR_NAME_BPF_LSM, XATTR_NAME_BPF_LSM_LEN);
@@ -363,6 +419,8 @@ BTF_ID_FLAGS(func, bpf_get_task_exe_file,
KF_ACQUIRE | KF_TRUSTED_ARGS | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE)
BTF_ID_FLAGS(func, bpf_path_d_path, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_kern_path, KF_TRUSTED_ARGS | KF_ACQUIRE | KF_SLEEPABLE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_path_put, KF_RELEASE)
BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
--
2.47.3
^ permalink raw reply related
* [PATCH bpf-next 1/3] bpf: Allow const char * from LSM hooks as kfunc const string arguments
From: Song Liu @ 2025-11-27 0:50 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-security-module
Cc: ast, daniel, andrii, kernel-team, viro, brauner, jack, paul,
jmorris, serge, Song Liu
In-Reply-To: <20251127005011.1872209-1-song@kernel.org>
Let the BPF verifier to recognize const char * arguments from LSM hooks
(and other BPF program types) as valid const string pointers that can be
passed to kfuncs expecting KF_ARG_PTR_TO_CONST_STR.
Previously, kfuncs with KF_ARG_PTR_TO_CONST_STR only accepted
PTR_TO_MAP_VALUE from readonly maps. This was limiting for LSM programs
that receive const char * arguments from hooks like sb_mount's dev_name.
Signed-off-by: Song Liu <song@kernel.org>
---
include/linux/btf.h | 1 +
kernel/bpf/btf.c | 33 ++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 51 +++++++++++++++++++++++++++++++++----------
3 files changed, 73 insertions(+), 12 deletions(-)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index f06976ffb63f..bd5a32d33254 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -224,6 +224,7 @@ struct btf *btf_base_btf(const struct btf *btf);
bool btf_type_is_i32(const struct btf_type *t);
bool btf_type_is_i64(const struct btf_type *t);
bool btf_type_is_primitive(const struct btf_type *t);
+bool btf_type_is_const_char_ptr(const struct btf *btf, const struct btf_type *t);
bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
const struct btf_member *m,
u32 expected_offset, u32 expected_size);
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 0de8fc8a0e0b..94a272585b97 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -897,6 +897,25 @@ bool btf_type_is_primitive(const struct btf_type *t)
btf_is_any_enum(t);
}
+bool btf_type_is_const_char_ptr(const struct btf *btf, const struct btf_type *t)
+{
+ const char *tname;
+
+ /* The type chain has to be PTR->CONST->CHAR */
+ if (BTF_INFO_KIND(t->info) != BTF_KIND_PTR)
+ return false;
+
+ t = btf_type_by_id(btf, t->type);
+ if (BTF_INFO_KIND(t->info) != BTF_KIND_CONST)
+ return false;
+
+ t = btf_type_by_id(btf, t->type);
+ tname = btf_name_by_offset(btf, t->name_off);
+ if (tname && strcmp(tname, "char") == 0)
+ return true;
+ return false;
+}
+
/*
* Check that given struct member is a regular int with expected
* offset and size.
@@ -6746,6 +6765,20 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
/* Default prog with MAX_BPF_FUNC_REG_ARGS args */
return true;
t = btf_type_by_id(btf, args[arg].type);
+
+ /*
+ * For const string, we need to match "const char *"
+ * exactly. Therefore, do the check before the skipping
+ * modifiers.
+ */
+ if (btf_type_is_const_char_ptr(btf, t)) {
+ info->reg_type = PTR_TO_BTF_ID;
+ if (prog_args_trusted(prog))
+ info->reg_type |= PTR_TRUSTED;
+ info->btf = btf;
+ info->btf_id = args[arg].type;
+ return true;
+ }
}
/* skip modifiers */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 766695491bc5..a9757c056d4b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9598,8 +9598,12 @@ static enum bpf_dynptr_type dynptr_get_type(struct bpf_verifier_env *env,
return state->stack[spi].spilled_ptr.dynptr.type;
}
-static int check_reg_const_str(struct bpf_verifier_env *env,
- struct bpf_reg_state *reg, u32 regno)
+/*
+ * Check for const string saved in a bpf map. The caller is responsible
+ * to check reg->type == PTR_TO_MAP_VALUE.
+ */
+static int check_reg_const_str_in_map(struct bpf_verifier_env *env,
+ struct bpf_reg_state *reg, u32 regno)
{
struct bpf_map *map = reg->map_ptr;
int err;
@@ -9607,9 +9611,6 @@ static int check_reg_const_str(struct bpf_verifier_env *env,
u64 map_addr;
char *str_ptr;
- if (reg->type != PTR_TO_MAP_VALUE)
- return -EINVAL;
-
if (!bpf_map_is_rdonly(map)) {
verbose(env, "R%d does not point to a readonly map'\n", regno);
return -EACCES;
@@ -9646,6 +9647,26 @@ static int check_reg_const_str(struct bpf_verifier_env *env,
return 0;
}
+/* Check for const string passed in as input to the bpf program. */
+static int check_reg_const_str_arg(struct bpf_reg_state *reg)
+{
+ const struct btf *btf;
+ const struct btf_type *t;
+ const char *tname;
+
+ if (base_type(reg->type) != PTR_TO_BTF_ID)
+ return -EINVAL;
+
+ btf = reg->btf;
+ t = btf_type_by_id(btf, reg->btf_id);
+ if (!t)
+ return -EINVAL;
+
+ if (btf_type_is_const_char_ptr(btf, t))
+ return 0;
+ return -EINVAL;
+}
+
/* Returns constant key value in `value` if possible, else negative error */
static int get_constant_map_key(struct bpf_verifier_env *env,
struct bpf_reg_state *key,
@@ -9964,7 +9985,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
break;
case ARG_PTR_TO_CONST_STR:
{
- err = check_reg_const_str(env, reg, regno);
+ if (reg->type != PTR_TO_MAP_VALUE)
+ return -EINVAL;
+ err = check_reg_const_str_in_map(env, reg, regno);
if (err)
return err;
break;
@@ -13626,13 +13649,17 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
meta->arg_btf_id = reg->btf_id;
break;
case KF_ARG_PTR_TO_CONST_STR:
- if (reg->type != PTR_TO_MAP_VALUE) {
- verbose(env, "arg#%d doesn't point to a const string\n", i);
- return -EINVAL;
+ if (reg->type == PTR_TO_MAP_VALUE) {
+ ret = check_reg_const_str_in_map(env, reg, regno);
+ if (ret)
+ return ret;
+ } else {
+ ret = check_reg_const_str_arg(reg);
+ if (ret) {
+ verbose(env, "arg#%d doesn't point to a const string\n", i);
+ return ret;
+ }
}
- ret = check_reg_const_str(env, reg, regno);
- if (ret)
- return ret;
break;
case KF_ARG_PTR_TO_WORKQUEUE:
if (reg->type != PTR_TO_MAP_VALUE) {
--
2.47.3
^ permalink raw reply related
* [PATCH bpf-next 0/3] Introduce bpf_kern_path and bpf_path_put
From: Song Liu @ 2025-11-27 0:50 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-security-module
Cc: ast, daniel, andrii, kernel-team, viro, brauner, jack, paul,
jmorris, serge, Song Liu
In-Reply-To: <20251127005011.1872209-1-song@kernel.org>
Security solutions use LSM hook security_sb_mount to monitor mount
operations. security_sb_mount takes dev_name as a string. To get a struct
path from dev_name, in-tree LSMs use kern_path. Introduce kfuncs
bpf_kern_path so that bpf LSM can do similar operations. bpf_kern_path
takes a reference on the return value path. Also add kfunc bpf_path_put to
release path returned by bpf_kern_path. Note that, bpf_kern_path only holds
reference on the path during the duration of this bpf program. The verifier
enforces the bpf program release this reference.
Patch 1/3 prepares bpf verifier to handle const char * passed in as hook
argument. Before this change, bpf helpers and kfuncs only consider value
from read only map as const string.
Patch 2/3 adds the two kfuncs.
Patch 3/3 add tests for the new kfuncs.
Song Liu (3):
bpf: Allow const char * from LSM hooks as kfunc const string arguments
bpf: Add bpf_kern_path and bpf_path_put kfuncs
selftests/bpf: Add tests for bpf_kern_path kfunc
fs/bpf_fs_kfuncs.c | 58 +++++++++++
include/linux/btf.h | 1 +
kernel/bpf/btf.c | 33 +++++++
kernel/bpf/verifier.c | 51 +++++++---
.../testing/selftests/bpf/bpf_experimental.h | 4 +
.../selftests/bpf/prog_tests/kern_path.c | 82 ++++++++++++++++
.../selftests/bpf/progs/test_kern_path.c | 56 +++++++++++
.../selftests/bpf/progs/verifier_kern_path.c | 52 ++++++++++
.../bpf/progs/verifier_kern_path_fail.c | 97 +++++++++++++++++++
9 files changed, 422 insertions(+), 12 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/test_kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/verifier_kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/verifier_kern_path_fail.c
--
2.47.3
^ permalink raw reply
* [PATCH bpf-next 3/3] selftests/bpf: Add tests for bpf_kern_path kfunc
From: Song Liu @ 2025-11-27 0:50 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-security-module
Cc: ast, daniel, andrii, kernel-team, viro, brauner, jack, paul,
jmorris, serge, Song Liu
In-Reply-To: <20251127005011.1872209-1-song@kernel.org>
Add comprehensive selftests for the new bpf_kern_path and bpf_path_put
kfuncs:
1. Functional tests (prog_tests/kern_path.c, progs/test_kern_path.c):
- test_kern_path_basic: Tests successful path resolution using
/proc/self/exe and validates the resolved path with bpf_path_d_path
- test_kern_path_sb_mount: Tests bpf_kern_path with dynamic input
from LSM hook parameter (dev_name from sb_mount), demonstrating
real-world usage where BPF programs resolve paths from hook args
2. Verifier success tests (progs/verifier_kern_path.c):
- kern_path_success: Proper acquire -> use -> release pattern
- kern_path_multiple_paths: Multiple concurrent path acquisitions
3. Verifier failure tests (progs/verifier_kern_path_fail.c):
- kern_path_unreleased: Resource leak detection
- path_put_unacquired: Releasing unacquired path
- path_use_after_put: Use-after-free detection
- double_path_put: Double-free detection
- kern_path_non_lsm: Program type restrictions (LSM only)
- kern_path_non_const_str: reject none const string
These tests verify both the functionality of the kfuncs and that the
verifier properly enforces acquire/release semantics to prevent
resource leaks.
Signed-off-by: Song Liu <song@kernel.org>
---
.../testing/selftests/bpf/bpf_experimental.h | 4 +
.../selftests/bpf/prog_tests/kern_path.c | 82 ++++++++++++++++
.../selftests/bpf/progs/test_kern_path.c | 56 +++++++++++
.../selftests/bpf/progs/verifier_kern_path.c | 52 ++++++++++
.../bpf/progs/verifier_kern_path_fail.c | 97 +++++++++++++++++++
5 files changed, 291 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/test_kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/verifier_kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/verifier_kern_path_fail.c
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 2cd9165c7348..c512c9a14752 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -221,6 +221,10 @@ extern void bpf_put_file(struct file *file) __ksym;
*/
extern int bpf_path_d_path(const struct path *path, char *buf, size_t buf__sz) __ksym;
+extern struct path *bpf_kern_path(const char *pathname, unsigned int flags) __ksym;
+extern void bpf_path_put(struct path *path) __ksym;
+extern int bpf_path_d_path(const struct path *path, char *buf, size_t buf__sz) __ksym;
+
/* This macro must be used to mark the exception callback corresponding to the
* main program. For example:
*
diff --git a/tools/testing/selftests/bpf/prog_tests/kern_path.c b/tools/testing/selftests/bpf/prog_tests/kern_path.c
new file mode 100644
index 000000000000..f4cdfe202a26
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/kern_path.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. */
+
+#include <test_progs.h>
+#include <sys/mount.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include "test_kern_path.skel.h"
+#include "verifier_kern_path.skel.h"
+#include "verifier_kern_path_fail.skel.h"
+
+static void __test_kern_path(void (*trigger)(void))
+{
+ struct test_kern_path *skel;
+ int err;
+
+ skel = test_kern_path__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "test_kern_path__open_and_load"))
+ return;
+
+ skel->bss->monitored_pid = getpid();
+
+ err = test_kern_path__attach(skel);
+ if (!ASSERT_OK(err, "test_kern_path__attach"))
+ goto cleanup;
+
+ trigger();
+
+ /* Verify the bpf_path_d_path worked */
+ ASSERT_GT(skel->bss->path_len, 0, "path_len > 0");
+
+cleanup:
+ test_kern_path__destroy(skel);
+}
+
+static void trigger_file_open(void)
+{
+ int fd;
+
+ fd = open("/dev/null", O_RDONLY);
+ if (!ASSERT_OK_FD(fd, "open /dev/null"))
+ return;
+ close(fd);
+}
+
+static void trigger_sb_mount(void)
+{
+ char tmpdir[] = "/tmp/bpf_kern_path_test_XXXXXX";
+ int err;
+
+ if (!ASSERT_OK_PTR(mkdtemp(tmpdir), "mkdtemp"))
+ return;
+
+ err = mount("/tmp", tmpdir, NULL, MS_BIND, NULL);
+ if (!ASSERT_OK(err, "bind mount"))
+ goto rmdir;
+
+ umount(tmpdir);
+rmdir:
+ rmdir(tmpdir);
+}
+
+void test_kern_path(void)
+{
+ if (test__start_subtest("file_open"))
+ __test_kern_path(trigger_file_open);
+
+ if (test__start_subtest("sb_mount"))
+ __test_kern_path(trigger_sb_mount);
+}
+
+void test_verifier_kern_path(void)
+{
+ RUN_TESTS(verifier_kern_path);
+}
+
+void test_verifier_kern_path_fail(void)
+{
+ RUN_TESTS(verifier_kern_path_fail);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_kern_path.c b/tools/testing/selftests/bpf/progs/test_kern_path.c
new file mode 100644
index 000000000000..e9186a1aa990
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_kern_path.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. */
+
+#include "vmlinux.h"
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+#define MAX_PATH_LEN 256
+
+char buf[MAX_PATH_LEN];
+int path_len = 0;
+u32 monitored_pid = 0;
+
+SEC("lsm.s/file_open")
+int BPF_PROG(test_kern_path_basic, struct file *file)
+{
+ struct path *p;
+ int ret;
+
+ if (bpf_get_current_pid_tgid() >> 32 != monitored_pid)
+ return 0;
+
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (p) {
+ ret = bpf_path_d_path(p, buf, MAX_PATH_LEN);
+ if (ret > 0)
+ path_len = ret;
+ bpf_path_put(p);
+ }
+
+ return 0;
+}
+
+SEC("lsm.s/sb_mount")
+int BPF_PROG(test_kern_path_from_sb_mount, const char *dev_name, const struct path *path,
+ const char *type, unsigned long flags, void *data)
+{
+ struct path *p;
+ int ret;
+
+ if (bpf_get_current_pid_tgid() >> 32 != monitored_pid)
+ return 0;
+
+ p = bpf_kern_path(dev_name, 0);
+ if (p) {
+ ret = bpf_path_d_path(p, buf, MAX_PATH_LEN);
+ if (ret > 0)
+ path_len = ret;
+ bpf_path_put(p);
+ }
+
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/verifier_kern_path.c b/tools/testing/selftests/bpf/progs/verifier_kern_path.c
new file mode 100644
index 000000000000..0e6ccf640b64
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_kern_path.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include <linux/limits.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+static char buf[PATH_MAX];
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(kern_path_success)
+{
+ struct path *p;
+
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (!p)
+ return 0;
+
+ bpf_path_d_path(p, buf, sizeof(buf));
+
+ bpf_path_put(p);
+ return 0;
+}
+
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(kern_path_multiple_paths)
+{
+ struct path *p1, *p2;
+
+ p1 = bpf_kern_path("/proc/self/exe", 0);
+ if (!p1)
+ return 0;
+
+ p2 = bpf_kern_path("/proc/self/cwd", 0);
+ if (!p2) {
+ bpf_path_put(p1);
+ return 0;
+ }
+
+ bpf_path_d_path(p1, buf, sizeof(buf));
+ bpf_path_d_path(p2, buf, sizeof(buf));
+
+ bpf_path_put(p2);
+ bpf_path_put(p1);
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/verifier_kern_path_fail.c b/tools/testing/selftests/bpf/progs/verifier_kern_path_fail.c
new file mode 100644
index 000000000000..520c227af5ca
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_kern_path_fail.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include <linux/limits.h>
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+static char buf[PATH_MAX];
+
+SEC("lsm.s/file_open")
+__failure __msg("Unreleased reference")
+int BPF_PROG(kern_path_unreleased)
+{
+ struct path *p;
+
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (!p)
+ return 0;
+
+ /* Acquired but never released - should fail verification */
+ return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("pointer type STRUCT path must point to scalar, or struct with scalar")
+int BPF_PROG(path_put_unacquired)
+{
+ struct path p = {};
+
+ /* Can't release an unacquired path - should fail verification */
+ bpf_path_put(&p);
+ return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("pointer type STRUCT path must point to scalar, or struct with scalar")
+int BPF_PROG(path_use_after_put, struct file *file)
+{
+ struct path *p;
+
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (!p)
+ return 0;
+
+ bpf_path_put(p);
+
+ /* Using path after put - should fail verification */
+ bpf_path_d_path(p, buf, sizeof(buf));
+ return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure __msg("pointer type STRUCT path must point to scalar, or struct with scalar")
+int BPF_PROG(double_path_put)
+{
+ struct path *p;
+
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (!p)
+ return 0;
+
+ bpf_path_put(p);
+ /* Double put - should fail verification */
+ bpf_path_put(p);
+ return 0;
+}
+
+SEC("fentry/vfs_open")
+__failure __msg("calling kernel function bpf_kern_path is not allowed")
+int BPF_PROG(kern_path_non_lsm)
+{
+ struct path *p;
+
+ /* Calling bpf_kern_path() from a non-LSM BPF program isn't permitted */
+ p = bpf_kern_path("/proc/self/exe", 0);
+ if (p)
+ bpf_path_put(p);
+ return 0;
+}
+
+SEC("lsm.s/sb_eat_lsm_opts")
+__failure __msg("arg#0 doesn't point to a const string")
+int BPF_PROG(kern_path_non_const_str, char *options, void **mnt_opts)
+{
+ struct path *p;
+
+ /* Calling bpf_kern_path() from a with non-const string isn't permitted */
+ p = bpf_kern_path(options, 0);
+ if (p)
+ bpf_path_put(p);
+ return 0;
+}
+
+
+char _license[] SEC("license") = "GPL";
--
2.47.3
^ permalink raw reply related
* [PATCH bpf-next 2/3] bpf: Add bpf_kern_path and bpf_path_put kfuncs
From: Song Liu @ 2025-11-27 0:50 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-security-module
Cc: ast, daniel, andrii, kernel-team, viro, brauner, jack, paul,
jmorris, serge, Song Liu
In-Reply-To: <20251127005011.1872209-1-song@kernel.org>
Add two new kfuncs to fs/bpf_fs_kfuncs.c that wrap kern_path() for use
by BPF LSM programs:
bpf_kern_path():
- Resolves a pathname string to a struct path
- Allocates memory for the path structure
- Returns NULL on error or if the path doesn't exist
- Marked with KF_ACQUIRE | KF_SLEEPABLE | KF_RET_NULL
bpf_path_put():
- Releases the path reference and frees the allocated memory
- Marked with KF_RELEASE to enforce acquire/release semantics
These kfuncs enable BPF LSM programs to resolve pathnames provided by
hook arguments (e.g., dev_name from sb_mount) and validate or inspect
the resolved paths. The verifier enforces proper resource management
through acquire/release tracking.
Example usage:
struct path *p = bpf_kern_path("/etc/passwd", LOOKUP_FOLLOW);
if (p) {
// Use the path...
bpf_path_put(p); // Must release
}
Signed-off-by: Song Liu <song@kernel.org>
---
fs/bpf_fs_kfuncs.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 5ace2511fec5..977f8dcbc208 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -11,6 +11,7 @@
#include <linux/file.h>
#include <linux/kernfs.h>
#include <linux/mm.h>
+#include <linux/namei.h>
#include <linux/xattr.h>
__bpf_kfunc_start_defs();
@@ -96,6 +97,61 @@ __bpf_kfunc int bpf_path_d_path(const struct path *path, char *buf, size_t buf__
return len;
}
+/**
+ * bpf_kern_path - resolve a pathname to a struct path
+ * @pathname__str: pathname to resolve
+ * @flags: lookup flags (e.g., LOOKUP_FOLLOW)
+ *
+ * Resolve the pathname for the supplied *pathname__str* and return a pointer
+ * to a struct path. This is a wrapper around kern_path() that allocates and
+ * returns a struct path pointer on success.
+ *
+ * The returned struct path pointer must be released using bpf_path_put().
+ * Failing to call bpf_path_put() on the returned struct path pointer will
+ * result in the BPF program being rejected by the BPF verifier.
+ *
+ * This BPF kfunc may only be called from BPF LSM programs.
+ *
+ * Return: A pointer to an allocated struct path on success, NULL on error.
+ */
+__bpf_kfunc struct path *bpf_kern_path(const char *pathname__str, unsigned int flags)
+{
+ struct path *path;
+ int ret;
+
+ path = kmalloc(sizeof(*path), GFP_KERNEL);
+ if (!path)
+ return NULL;
+
+ ret = kern_path(pathname__str, flags, path);
+ if (ret) {
+ kfree(path);
+ return NULL;
+ }
+
+ return path;
+}
+
+/**
+ * bpf_path_put - release a struct path reference
+ * @path: struct path pointer to release
+ *
+ * Release the struct path pointer that was acquired by bpf_kern_path().
+ * This BPF kfunc calls path_put() on the supplied *path* and then frees
+ * the allocated memory.
+ *
+ * Only struct path pointers acquired by bpf_kern_path() may be passed to
+ * this BPF kfunc. Attempting to pass any other pointer will result in the
+ * BPF program being rejected by the BPF verifier.
+ *
+ * This BPF kfunc may only be called from BPF LSM programs.
+ */
+__bpf_kfunc void bpf_path_put(struct path *path)
+{
+ path_put(path);
+ kfree(path);
+}
+
static bool match_security_bpf_prefix(const char *name__str)
{
return !strncmp(name__str, XATTR_NAME_BPF_LSM, XATTR_NAME_BPF_LSM_LEN);
@@ -363,6 +419,8 @@ BTF_ID_FLAGS(func, bpf_get_task_exe_file,
KF_ACQUIRE | KF_TRUSTED_ARGS | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE)
BTF_ID_FLAGS(func, bpf_path_d_path, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_kern_path, KF_TRUSTED_ARGS | KF_ACQUIRE | KF_SLEEPABLE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_path_put, KF_RELEASE)
BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
--
2.47.3
^ permalink raw reply related
* [PATCH bpf-next 1/3] bpf: Allow const char * from LSM hooks as kfunc const string arguments
From: Song Liu @ 2025-11-27 0:50 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-security-module
Cc: ast, daniel, andrii, kernel-team, viro, brauner, jack, paul,
jmorris, serge, Song Liu
In-Reply-To: <20251127005011.1872209-1-song@kernel.org>
Let the BPF verifier to recognize const char * arguments from LSM hooks
(and other BPF program types) as valid const string pointers that can be
passed to kfuncs expecting KF_ARG_PTR_TO_CONST_STR.
Previously, kfuncs with KF_ARG_PTR_TO_CONST_STR only accepted
PTR_TO_MAP_VALUE from readonly maps. This was limiting for LSM programs
that receive const char * arguments from hooks like sb_mount's dev_name.
Signed-off-by: Song Liu <song@kernel.org>
---
include/linux/btf.h | 1 +
kernel/bpf/btf.c | 33 ++++++++++++++++++++++++++++
kernel/bpf/verifier.c | 51 +++++++++++++++++++++++++++++++++----------
3 files changed, 73 insertions(+), 12 deletions(-)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index f06976ffb63f..bd5a32d33254 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -224,6 +224,7 @@ struct btf *btf_base_btf(const struct btf *btf);
bool btf_type_is_i32(const struct btf_type *t);
bool btf_type_is_i64(const struct btf_type *t);
bool btf_type_is_primitive(const struct btf_type *t);
+bool btf_type_is_const_char_ptr(const struct btf *btf, const struct btf_type *t);
bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
const struct btf_member *m,
u32 expected_offset, u32 expected_size);
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 0de8fc8a0e0b..94a272585b97 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -897,6 +897,25 @@ bool btf_type_is_primitive(const struct btf_type *t)
btf_is_any_enum(t);
}
+bool btf_type_is_const_char_ptr(const struct btf *btf, const struct btf_type *t)
+{
+ const char *tname;
+
+ /* The type chain has to be PTR->CONST->CHAR */
+ if (BTF_INFO_KIND(t->info) != BTF_KIND_PTR)
+ return false;
+
+ t = btf_type_by_id(btf, t->type);
+ if (BTF_INFO_KIND(t->info) != BTF_KIND_CONST)
+ return false;
+
+ t = btf_type_by_id(btf, t->type);
+ tname = btf_name_by_offset(btf, t->name_off);
+ if (tname && strcmp(tname, "char") == 0)
+ return true;
+ return false;
+}
+
/*
* Check that given struct member is a regular int with expected
* offset and size.
@@ -6746,6 +6765,20 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
/* Default prog with MAX_BPF_FUNC_REG_ARGS args */
return true;
t = btf_type_by_id(btf, args[arg].type);
+
+ /*
+ * For const string, we need to match "const char *"
+ * exactly. Therefore, do the check before the skipping
+ * modifiers.
+ */
+ if (btf_type_is_const_char_ptr(btf, t)) {
+ info->reg_type = PTR_TO_BTF_ID;
+ if (prog_args_trusted(prog))
+ info->reg_type |= PTR_TRUSTED;
+ info->btf = btf;
+ info->btf_id = args[arg].type;
+ return true;
+ }
}
/* skip modifiers */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 766695491bc5..a9757c056d4b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9598,8 +9598,12 @@ static enum bpf_dynptr_type dynptr_get_type(struct bpf_verifier_env *env,
return state->stack[spi].spilled_ptr.dynptr.type;
}
-static int check_reg_const_str(struct bpf_verifier_env *env,
- struct bpf_reg_state *reg, u32 regno)
+/*
+ * Check for const string saved in a bpf map. The caller is responsible
+ * to check reg->type == PTR_TO_MAP_VALUE.
+ */
+static int check_reg_const_str_in_map(struct bpf_verifier_env *env,
+ struct bpf_reg_state *reg, u32 regno)
{
struct bpf_map *map = reg->map_ptr;
int err;
@@ -9607,9 +9611,6 @@ static int check_reg_const_str(struct bpf_verifier_env *env,
u64 map_addr;
char *str_ptr;
- if (reg->type != PTR_TO_MAP_VALUE)
- return -EINVAL;
-
if (!bpf_map_is_rdonly(map)) {
verbose(env, "R%d does not point to a readonly map'\n", regno);
return -EACCES;
@@ -9646,6 +9647,26 @@ static int check_reg_const_str(struct bpf_verifier_env *env,
return 0;
}
+/* Check for const string passed in as input to the bpf program. */
+static int check_reg_const_str_arg(struct bpf_reg_state *reg)
+{
+ const struct btf *btf;
+ const struct btf_type *t;
+ const char *tname;
+
+ if (base_type(reg->type) != PTR_TO_BTF_ID)
+ return -EINVAL;
+
+ btf = reg->btf;
+ t = btf_type_by_id(btf, reg->btf_id);
+ if (!t)
+ return -EINVAL;
+
+ if (btf_type_is_const_char_ptr(btf, t))
+ return 0;
+ return -EINVAL;
+}
+
/* Returns constant key value in `value` if possible, else negative error */
static int get_constant_map_key(struct bpf_verifier_env *env,
struct bpf_reg_state *key,
@@ -9964,7 +9985,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
break;
case ARG_PTR_TO_CONST_STR:
{
- err = check_reg_const_str(env, reg, regno);
+ if (reg->type != PTR_TO_MAP_VALUE)
+ return -EINVAL;
+ err = check_reg_const_str_in_map(env, reg, regno);
if (err)
return err;
break;
@@ -13626,13 +13649,17 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
meta->arg_btf_id = reg->btf_id;
break;
case KF_ARG_PTR_TO_CONST_STR:
- if (reg->type != PTR_TO_MAP_VALUE) {
- verbose(env, "arg#%d doesn't point to a const string\n", i);
- return -EINVAL;
+ if (reg->type == PTR_TO_MAP_VALUE) {
+ ret = check_reg_const_str_in_map(env, reg, regno);
+ if (ret)
+ return ret;
+ } else {
+ ret = check_reg_const_str_arg(reg);
+ if (ret) {
+ verbose(env, "arg#%d doesn't point to a const string\n", i);
+ return ret;
+ }
}
- ret = check_reg_const_str(env, reg, regno);
- if (ret)
- return ret;
break;
case KF_ARG_PTR_TO_WORKQUEUE:
if (reg->type != PTR_TO_MAP_VALUE) {
--
2.47.3
^ permalink raw reply related
* [PATCH bpf-next 0/3] Introduce bpf_kern_path and bpf_path_put
From: Song Liu @ 2025-11-27 0:50 UTC (permalink / raw)
To: bpf, linux-fsdevel, linux-security-module
Cc: ast, daniel, andrii, kernel-team, viro, brauner, jack, paul,
jmorris, serge, Song Liu
Security solutions use LSM hook security_sb_mount to monitor mount
operations. security_sb_mount takes dev_name as a string. To get a struct
path from dev_name, in-tree LSMs use kern_path. Introduce kfuncs
bpf_kern_path so that bpf LSM can do similar operations. bpf_kern_path
takes a reference on the return value path. Also add kfunc bpf_path_put to
release path returned by bpf_kern_path. Note that, bpf_kern_path only holds
reference on the path during the duration of this bpf program. The verifier
enforces the bpf program release this reference.
Patch 1/3 prepares bpf verifier to handle const char * passed in as hook
argument. Before this change, bpf helpers and kfuncs only consider value
from read only map as const string.
Patch 2/3 adds the two kfuncs.
Patch 3/3 add tests for the new kfuncs.
Song Liu (3):
bpf: Allow const char * from LSM hooks as kfunc const string arguments
bpf: Add bpf_kern_path and bpf_path_put kfuncs
selftests/bpf: Add tests for bpf_kern_path kfunc
fs/bpf_fs_kfuncs.c | 58 +++++++++++
include/linux/btf.h | 1 +
kernel/bpf/btf.c | 33 +++++++
kernel/bpf/verifier.c | 51 +++++++---
.../testing/selftests/bpf/bpf_experimental.h | 4 +
.../selftests/bpf/prog_tests/kern_path.c | 82 ++++++++++++++++
.../selftests/bpf/progs/test_kern_path.c | 56 +++++++++++
.../selftests/bpf/progs/verifier_kern_path.c | 52 ++++++++++
.../bpf/progs/verifier_kern_path_fail.c | 97 +++++++++++++++++++
9 files changed, 422 insertions(+), 12 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/test_kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/verifier_kern_path.c
create mode 100644 tools/testing/selftests/bpf/progs/verifier_kern_path_fail.c
--
2.47.3
^ permalink raw reply
* Re: [RFC v1 0/1] Implement IMA Event Log Trimming
From: Gregory Lumen @ 2025-11-26 23:40 UTC (permalink / raw)
To: Roberto Sassu
Cc: Anirudh Venkataramanan, linux-integrity, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E . Hallyn, linux-security-module,
Steven Chen, Lakshmi Ramasubramanian, Sush Shringarputale
In-Reply-To: <1e5a3b427fe2783e57e88ca14630f5e38e01fac5.camel@huaweicloud.com>
Greetings Roberto,
If I may chime in a bit:
> The only way to make the verification of measurements list snapshots
> work is that the verification state is stored outside the system to
> evaluate (which can be assumed to be trusted), so that you are sure
> that the system is not advancing the PCR starting value by itself.
You are correct; to make the described approach work, an external source
of trust is required in order to detect unexpected or unauthorized
trimming of the event log (for example, by signing the trim-to PCR values
from the previous verification/attestation cycle). This should be true
regardless of the mechanism of trimming. More generally, I will go so far
as to suggest that any attempt to attest the integrity of a system using
IMA will likely fall into one of two general approaches: either the entire
IMA event log is retained (either in kernel or user space) from boot and
claims of system integrity are built by validating and examining the
entire log for signs of tampering, or an external source of trust is
introduced to allow incremental validation and examination of the log.
Other more innovative approaches may exist, but we make no such claims.
I will also say that it should be possible to implement either approach to
attestation (retaining the entire log, or relying on an external source of
trust) with any sane implementation for IMA log trimming.
As for our proposed implementation, storing the starting PCR values in the
kernel preserving the ability for any arbitrary user space entity to
validate the retained portion of the IMA event log against the TPM PCRs at
any time, without requiring awareness of other user space mechanisms
implemented by other entities that may be initiating IMA trimming
operations. My personal sense is that this capability is worth preserving,
but it is entirely possible the general consensus is that the value
offered does not balance against the additional technical complexity when
compared to simpler alternatives (discussed in a moment). To stress the
point, this capability would only enable validation of the integrity of
the retained portion of the event log and its continuity with the PCRs,
and could not be used to make any claims as to the overall integrity of
the system since, as you observed, an attacker who has successfully
compromised the system could simply trim the event log in order to discard
evidence of the compromise.
If the ability to validate the retained portion of the IMA event log is
not worth designing for, we could instead go with a simpler "Trim-to-N"
approach, where the user space interface allows for the specification of
an absolute index into the IMA log to be used as the trim position (as
opposed to using calculated PCR values to indicate trim position in our
current proposal). To protect against unexpected behavior in the event of
concurrent trims, index counting would need to be fixed (hence absolute)
such that index 0 would always refer to the very first entry written
during boot, even if that entry has already been trimmed, with the number
of trimmed entries (and thus starting index of the retained log) exposed
to use space via a pseudo-file.
With such a trim approach, it should be possible to implement either
general attestation approach: retaining the entire log (copy the log to
user space, then trim the copied entries), or relying on an external
source of trust (quote, determine the log index corresponding to the quote
plus PCRs, trim, then securely store the trim position/starting PCRs for
future cycles).
-Gregory Lumen
^ permalink raw reply
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: John Johansen @ 2025-11-26 22:18 UTC (permalink / raw)
To: david laight, Helge Deller
Cc: Helge Deller, John Paul Adrian Glaubitz, linux-kernel, apparmor,
linux-security-module, linux-parisc
In-Reply-To: <20251126212309.5b21edca@pumpkin>
On 11/26/25 13:23, david laight wrote:
> On Wed, 26 Nov 2025 16:12:23 +0100
> Helge Deller <deller@kernel.org> wrote:
>
>> * david laight <david.laight@runbox.com>:
>>> On Wed, 26 Nov 2025 12:03:03 +0100
>>> Helge Deller <deller@gmx.de> wrote:
>>>
>>>> On 11/26/25 11:44, david laight wrote:
>>> ...
>>>>>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>>>>>> index 26e82ba879d44..3dcc342337aca 100644
>>>>>> --- a/security/apparmor/match.c
>>>>>> +++ b/security/apparmor/match.c
>>>>>> @@ -71,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
>>>>>> u8, u8, byte_to_byte);
>>>>>
>>>>> Is that that just memcpy() ?
>>>>
>>>> No, it's memcpy() only on big-endian machines.
>>>
>>> You've misread the quoting...
>>> The 'data8' case that was only half there is a memcpy().
>>>
>>>> On little-endian machines it converts from big-endian
>>>> 16/32-bit ints to little-endian 16/32-bit ints.
>>>>
>>>> But I see some potential for optimization here:
>>>> a) on big-endian machines just use memcpy()
>>>
>>> true
>>>
>>>> b) on little-endian machines use memcpy() to copy from possibly-unaligned
>>>> memory to then known-to-be-aligned destination. Then use a loop with
>>>> be32_to_cpu() instead of get_unaligned_xx() as it's faster.
>>>
>>> There is a function that does a loop byteswap of a buffer - no reason
>>> to re-invent it.
>>
>> I assumed there must be something, but I did not see it. Which one?
>
> I can't find it either - just some functions to do an in-place swap.
> (Which aren't usually a good idea)
>
>>
>>> But I doubt it is always (if ever) faster to do a copy and then byteswap.
>>> The loop control and extra memory accesses kill performance.
>>
>> Yes, you are probably right.
>>
>>> Not that I've seen a fast get_unaligned() - I don't think gcc or clang
>>> generate optimal code - For LE I think it is something like:
>>> low = *(addr & ~3);
>>> high = *((addr + 3) & ~3);
>>> shift = (addr & 3) * 8;
>>> value = low << shift | high >> (32 - shift);
>>> Note that it is only 2 aligned memory reads - even for 64bit.
>>
>> Ok, then maybe we should keep it simple like this patch:
>>
>> [PATCH v2] apparmor: Optimize table creation from possibly unaligned memory
>>
>> Source blob may come from userspace and might be unaligned.
>> Try to optize the copying process by avoiding unaligned memory accesses.
>
> Not sure that reads right.
> 'Allow for misaligned data from userspace when byteswapping source blob' ?
>
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
>> index 1fbe82f5021b..386da2023d50 100644
>> --- a/security/apparmor/include/match.h
>> +++ b/security/apparmor/include/match.h
>> @@ -104,16 +104,20 @@ struct aa_dfa {
>> struct table_header *tables[YYTD_ID_TSIZE];
>> };
>>
>> -#define byte_to_byte(X) (X)
>> +#define byte_to_byte(X) (*(X))
>
> It's a bit of a shame you need something for the above...
>
We got rid of that in the last patch by just replacing the call to
UNPACK_ARRAY for bytes with just a call to memcpy.
> David
>
>
>>
>> #define UNPACK_ARRAY(TABLE, BLOB, LEN, TTYPE, BTYPE, NTOHX) \
>> do { \
>> typeof(LEN) __i; \
>> TTYPE *__t = (TTYPE *) TABLE; \
>> BTYPE *__b = (BTYPE *) BLOB; \
>> - for (__i = 0; __i < LEN; __i++) { \
>> - __t[__i] = NTOHX(__b[__i]); \
>> - } \
>> + BUILD_BUG_ON(sizeof(TTYPE) != sizeof(BTYPE)); \
>> + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) || sizeof(BTYPE) == 1) \
>> + memcpy(__t, __b, (LEN) * sizeof(BTYPE)); \
>> + else /* copy & convert convert from big-endian */ \
>> + for (__i = 0; __i < LEN; __i++) { \
>> + __t[__i] = NTOHX(&__b[__i]); \
>> + } \
>> } while (0)
>>
>> static inline size_t table_size(size_t len, size_t el_size)
>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>> index c5a91600842a..13e2f6873329 100644
>> --- a/security/apparmor/match.c
>> +++ b/security/apparmor/match.c
>> @@ -15,6 +15,7 @@
>> #include <linux/vmalloc.h>
>> #include <linux/err.h>
>> #include <linux/kref.h>
>> +#include <linux/unaligned.h>
>>
>> #include "include/lib.h"
>> #include "include/match.h"
>> @@ -70,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
>> u8, u8, byte_to_byte);
>> else if (th.td_flags == YYTD_DATA16)
>> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
>> - u16, __be16, be16_to_cpu);
>> + u16, __be16, get_unaligned_be16);
>> else if (th.td_flags == YYTD_DATA32)
>> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
>> - u32, __be32, be32_to_cpu);
>> + u32, __be32, get_unaligned_be32);
>> else
>> goto fail;
>> /* if table was vmalloced make sure the page tables are synced
>
^ permalink raw reply
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: david laight @ 2025-11-26 21:23 UTC (permalink / raw)
To: Helge Deller
Cc: Helge Deller, John Johansen, John Paul Adrian Glaubitz,
linux-kernel, apparmor, linux-security-module, linux-parisc
In-Reply-To: <aScY13MEBATreotz@carbonx1>
On Wed, 26 Nov 2025 16:12:23 +0100
Helge Deller <deller@kernel.org> wrote:
> * david laight <david.laight@runbox.com>:
> > On Wed, 26 Nov 2025 12:03:03 +0100
> > Helge Deller <deller@gmx.de> wrote:
> >
> > > On 11/26/25 11:44, david laight wrote:
> > ...
> > > >> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> > > >> index 26e82ba879d44..3dcc342337aca 100644
> > > >> --- a/security/apparmor/match.c
> > > >> +++ b/security/apparmor/match.c
> > > >> @@ -71,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
> > > >> u8, u8, byte_to_byte);
> > > >
> > > > Is that that just memcpy() ?
> > >
> > > No, it's memcpy() only on big-endian machines.
> >
> > You've misread the quoting...
> > The 'data8' case that was only half there is a memcpy().
> >
> > > On little-endian machines it converts from big-endian
> > > 16/32-bit ints to little-endian 16/32-bit ints.
> > >
> > > But I see some potential for optimization here:
> > > a) on big-endian machines just use memcpy()
> >
> > true
> >
> > > b) on little-endian machines use memcpy() to copy from possibly-unaligned
> > > memory to then known-to-be-aligned destination. Then use a loop with
> > > be32_to_cpu() instead of get_unaligned_xx() as it's faster.
> >
> > There is a function that does a loop byteswap of a buffer - no reason
> > to re-invent it.
>
> I assumed there must be something, but I did not see it. Which one?
I can't find it either - just some functions to do an in-place swap.
(Which aren't usually a good idea)
>
> > But I doubt it is always (if ever) faster to do a copy and then byteswap.
> > The loop control and extra memory accesses kill performance.
>
> Yes, you are probably right.
>
> > Not that I've seen a fast get_unaligned() - I don't think gcc or clang
> > generate optimal code - For LE I think it is something like:
> > low = *(addr & ~3);
> > high = *((addr + 3) & ~3);
> > shift = (addr & 3) * 8;
> > value = low << shift | high >> (32 - shift);
> > Note that it is only 2 aligned memory reads - even for 64bit.
>
> Ok, then maybe we should keep it simple like this patch:
>
> [PATCH v2] apparmor: Optimize table creation from possibly unaligned memory
>
> Source blob may come from userspace and might be unaligned.
> Try to optize the copying process by avoiding unaligned memory accesses.
Not sure that reads right.
'Allow for misaligned data from userspace when byteswapping source blob' ?
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
> index 1fbe82f5021b..386da2023d50 100644
> --- a/security/apparmor/include/match.h
> +++ b/security/apparmor/include/match.h
> @@ -104,16 +104,20 @@ struct aa_dfa {
> struct table_header *tables[YYTD_ID_TSIZE];
> };
>
> -#define byte_to_byte(X) (X)
> +#define byte_to_byte(X) (*(X))
It's a bit of a shame you need something for the above...
David
>
> #define UNPACK_ARRAY(TABLE, BLOB, LEN, TTYPE, BTYPE, NTOHX) \
> do { \
> typeof(LEN) __i; \
> TTYPE *__t = (TTYPE *) TABLE; \
> BTYPE *__b = (BTYPE *) BLOB; \
> - for (__i = 0; __i < LEN; __i++) { \
> - __t[__i] = NTOHX(__b[__i]); \
> - } \
> + BUILD_BUG_ON(sizeof(TTYPE) != sizeof(BTYPE)); \
> + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) || sizeof(BTYPE) == 1) \
> + memcpy(__t, __b, (LEN) * sizeof(BTYPE)); \
> + else /* copy & convert convert from big-endian */ \
> + for (__i = 0; __i < LEN; __i++) { \
> + __t[__i] = NTOHX(&__b[__i]); \
> + } \
> } while (0)
>
> static inline size_t table_size(size_t len, size_t el_size)
> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> index c5a91600842a..13e2f6873329 100644
> --- a/security/apparmor/match.c
> +++ b/security/apparmor/match.c
> @@ -15,6 +15,7 @@
> #include <linux/vmalloc.h>
> #include <linux/err.h>
> #include <linux/kref.h>
> +#include <linux/unaligned.h>
>
> #include "include/lib.h"
> #include "include/match.h"
> @@ -70,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
> u8, u8, byte_to_byte);
> else if (th.td_flags == YYTD_DATA16)
> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> - u16, __be16, be16_to_cpu);
> + u16, __be16, get_unaligned_be16);
> else if (th.td_flags == YYTD_DATA32)
> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> - u32, __be32, be32_to_cpu);
> + u32, __be32, get_unaligned_be32);
> else
> goto fail;
> /* if table was vmalloced make sure the page tables are synced
^ permalink raw reply
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: John Johansen @ 2025-11-26 21:10 UTC (permalink / raw)
To: Helge Deller
Cc: david laight, Helge Deller, John Paul Adrian Glaubitz,
linux-kernel, apparmor, linux-security-module, linux-parisc
In-Reply-To: <aSdfyGv2T88T5FEu@carbonx1>
On 11/26/25 12:15, Helge Deller wrote:
> * John Johansen <john.johansen@canonical.com>:
>> On 11/26/25 07:12, Helge Deller wrote:
>>> * david laight <david.laight@runbox.com>:
>>>> On Wed, 26 Nov 2025 12:03:03 +0100
>>>> Helge Deller <deller@gmx.de> wrote:
>>>>
>>>>> On 11/26/25 11:44, david laight wrote:
>>>> ...
>>>>>>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>>>>>>> index 26e82ba879d44..3dcc342337aca 100644
>>>>>>> --- a/security/apparmor/match.c
>>>>>>> +++ b/security/apparmor/match.c
>>>>>>> @@ -71,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
>>>>>>> u8, u8, byte_to_byte);
>>>>>>
>>>>>> Is that that just memcpy() ?
>>>>>
>>>>> No, it's memcpy() only on big-endian machines.
>>>>
>>>> You've misread the quoting...
>>>> The 'data8' case that was only half there is a memcpy().
>>>>
>>>>> On little-endian machines it converts from big-endian
>>>>> 16/32-bit ints to little-endian 16/32-bit ints.
>>>>>
>>>>> But I see some potential for optimization here:
>>>>> a) on big-endian machines just use memcpy()
>>>>
>>>> true
>>>>
>>>>> b) on little-endian machines use memcpy() to copy from possibly-unaligned
>>>>> memory to then known-to-be-aligned destination. Then use a loop with
>>>>> be32_to_cpu() instead of get_unaligned_xx() as it's faster.
>>>>
>>>> There is a function that does a loop byteswap of a buffer - no reason
>>>> to re-invent it.
>>>
>>> I assumed there must be something, but I did not see it. Which one?
>>>
>>>> But I doubt it is always (if ever) faster to do a copy and then byteswap.
>>>> The loop control and extra memory accesses kill performance.
>>>
>>> Yes, you are probably right.
>>>
>>>> Not that I've seen a fast get_unaligned() - I don't think gcc or clang
>>>> generate optimal code - For LE I think it is something like:
>>>> low = *(addr & ~3);
>>>> high = *((addr + 3) & ~3);
>>>> shift = (addr & 3) * 8;
>>>> value = low << shift | high >> (32 - shift);
>>>> Note that it is only 2 aligned memory reads - even for 64bit.
>>>
>>> Ok, then maybe we should keep it simple like this patch:
>>>
>>> [PATCH v2] apparmor: Optimize table creation from possibly unaligned memory
>>>
>>> Source blob may come from userspace and might be unaligned.
>>> Try to optize the copying process by avoiding unaligned memory accesses.
>>>
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>
>>> diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
>>> index 1fbe82f5021b..386da2023d50 100644
>>> --- a/security/apparmor/include/match.h
>>> +++ b/security/apparmor/include/match.h
>>> @@ -104,16 +104,20 @@ struct aa_dfa {
>>> struct table_header *tables[YYTD_ID_TSIZE];
>>> };
>>> -#define byte_to_byte(X) (X)
>>> +#define byte_to_byte(X) (*(X))
>>> #define UNPACK_ARRAY(TABLE, BLOB, LEN, TTYPE, BTYPE, NTOHX) \
>>> do { \
>>> typeof(LEN) __i; \
>>> TTYPE *__t = (TTYPE *) TABLE; \
>>> BTYPE *__b = (BTYPE *) BLOB; \
>>> - for (__i = 0; __i < LEN; __i++) { \
>>> - __t[__i] = NTOHX(__b[__i]); \
>>> - } \
>>> + BUILD_BUG_ON(sizeof(TTYPE) != sizeof(BTYPE)); \
>>> + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) || sizeof(BTYPE) == 1) \
>>> + memcpy(__t, __b, (LEN) * sizeof(BTYPE)); \
>>> + else /* copy & convert convert from big-endian */ \
>>> + for (__i = 0; __i < LEN; __i++) { \
>>> + __t[__i] = NTOHX(&__b[__i]); \
>>> + } \
>>> } while (0)
>>> static inline size_t table_size(size_t len, size_t el_size)
>>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>>> index c5a91600842a..13e2f6873329 100644
>>> --- a/security/apparmor/match.c
>>> +++ b/security/apparmor/match.c
>>> @@ -15,6 +15,7 @@
>>> #include <linux/vmalloc.h>
>>> #include <linux/err.h>
>>> #include <linux/kref.h>
>>> +#include <linux/unaligned.h>
>>> #include "include/lib.h"
>>> #include "include/match.h"
>>> @@ -70,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
>>> u8, u8, byte_to_byte);
>>> else if (th.td_flags == YYTD_DATA16)
>>> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
>>> - u16, __be16, be16_to_cpu);
>>> + u16, __be16, get_unaligned_be16);
>>> else if (th.td_flags == YYTD_DATA32)
>>> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
>>> - u32, __be32, be32_to_cpu);
>>> + u32, __be32, get_unaligned_be32);
>>> else
>>> goto fail;
>>> /* if table was vmalloced make sure the page tables are synced
>>
>> I think we can make one more tweak, in just not using UNPACK_ARRAY at all for the byte case
>> ie.
>>
>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>> index 26e82ba879d44..389202560675c 100644
>> --- a/security/apparmor/match.c
>> +++ b/security/apparmor/match.c
>> @@ -67,8 +67,7 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
>> table->td_flags = th.td_flags;
>> table->td_lolen = th.td_lolen;
>> if (th.td_flags == YYTD_DATA8)
>> - UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
>> - u8, u8, byte_to_byte);
>> + memcp(table->td_data, blob, th.td_lolen);
>
> True.
> Then byte_to_byte() can go away in match.h as well.
> So, here is a (untested) v3:
>
and lightly tested now
I will pull it into my tree
>
> [PATCH v3] apparmor: Optimize table creation from possibly unaligned memory
>
> Source blob may come from userspace and might be unaligned.
> Try to optize the copying process by avoiding unaligned memory accesses.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
Acked-by: John Johansen <john.johansen@canonical.com>
> diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
> index 1fbe82f5021b..19e72b3e8f49 100644
> --- a/security/apparmor/include/match.h
> +++ b/security/apparmor/include/match.h
> @@ -104,16 +104,18 @@ struct aa_dfa {
> struct table_header *tables[YYTD_ID_TSIZE];
> };
>
> -#define byte_to_byte(X) (X)
> -
> #define UNPACK_ARRAY(TABLE, BLOB, LEN, TTYPE, BTYPE, NTOHX) \
> do { \
> typeof(LEN) __i; \
> TTYPE *__t = (TTYPE *) TABLE; \
> BTYPE *__b = (BTYPE *) BLOB; \
> - for (__i = 0; __i < LEN; __i++) { \
> - __t[__i] = NTOHX(__b[__i]); \
> - } \
> + BUILD_BUG_ON(sizeof(TTYPE) != sizeof(BTYPE)); \
> + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) \
> + memcpy(__t, __b, (LEN) * sizeof(BTYPE)); \
> + else /* copy & convert convert from big-endian */ \
> + for (__i = 0; __i < LEN; __i++) { \
> + __t[__i] = NTOHX(&__b[__i]); \
> + } \
> } while (0)
>
> static inline size_t table_size(size_t len, size_t el_size)
> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> index c5a91600842a..1e32c8ba14ae 100644
> --- a/security/apparmor/match.c
> +++ b/security/apparmor/match.c
> @@ -15,6 +15,7 @@
> #include <linux/vmalloc.h>
> #include <linux/err.h>
> #include <linux/kref.h>
> +#include <linux/unaligned.h>
>
> #include "include/lib.h"
> #include "include/match.h"
> @@ -66,14 +67,13 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
> table->td_flags = th.td_flags;
> table->td_lolen = th.td_lolen;
> if (th.td_flags == YYTD_DATA8)
> - UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> - u8, u8, byte_to_byte);
> + memcpy(table->td_data, blob, th.td_lolen);
> else if (th.td_flags == YYTD_DATA16)
> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> - u16, __be16, be16_to_cpu);
> + u16, __be16, get_unaligned_be16);
> else if (th.td_flags == YYTD_DATA32)
> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> - u32, __be32, be32_to_cpu);
> + u32, __be32, get_unaligned_be32);
> else
> goto fail;
> /* if table was vmalloced make sure the page tables are synced
^ permalink raw reply
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: Helge Deller @ 2025-11-26 20:15 UTC (permalink / raw)
To: John Johansen
Cc: david laight, Helge Deller, John Paul Adrian Glaubitz,
linux-kernel, apparmor, linux-security-module, linux-parisc
In-Reply-To: <f5637038-9661-47fe-ba69-e461760ac975@canonical.com>
* John Johansen <john.johansen@canonical.com>:
> On 11/26/25 07:12, Helge Deller wrote:
> > * david laight <david.laight@runbox.com>:
> > > On Wed, 26 Nov 2025 12:03:03 +0100
> > > Helge Deller <deller@gmx.de> wrote:
> > >
> > > > On 11/26/25 11:44, david laight wrote:
> > > ...
> > > > > > diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> > > > > > index 26e82ba879d44..3dcc342337aca 100644
> > > > > > --- a/security/apparmor/match.c
> > > > > > +++ b/security/apparmor/match.c
> > > > > > @@ -71,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
> > > > > > u8, u8, byte_to_byte);
> > > > >
> > > > > Is that that just memcpy() ?
> > > >
> > > > No, it's memcpy() only on big-endian machines.
> > >
> > > You've misread the quoting...
> > > The 'data8' case that was only half there is a memcpy().
> > >
> > > > On little-endian machines it converts from big-endian
> > > > 16/32-bit ints to little-endian 16/32-bit ints.
> > > >
> > > > But I see some potential for optimization here:
> > > > a) on big-endian machines just use memcpy()
> > >
> > > true
> > >
> > > > b) on little-endian machines use memcpy() to copy from possibly-unaligned
> > > > memory to then known-to-be-aligned destination. Then use a loop with
> > > > be32_to_cpu() instead of get_unaligned_xx() as it's faster.
> > >
> > > There is a function that does a loop byteswap of a buffer - no reason
> > > to re-invent it.
> >
> > I assumed there must be something, but I did not see it. Which one?
> >
> > > But I doubt it is always (if ever) faster to do a copy and then byteswap.
> > > The loop control and extra memory accesses kill performance.
> >
> > Yes, you are probably right.
> >
> > > Not that I've seen a fast get_unaligned() - I don't think gcc or clang
> > > generate optimal code - For LE I think it is something like:
> > > low = *(addr & ~3);
> > > high = *((addr + 3) & ~3);
> > > shift = (addr & 3) * 8;
> > > value = low << shift | high >> (32 - shift);
> > > Note that it is only 2 aligned memory reads - even for 64bit.
> >
> > Ok, then maybe we should keep it simple like this patch:
> >
> > [PATCH v2] apparmor: Optimize table creation from possibly unaligned memory
> >
> > Source blob may come from userspace and might be unaligned.
> > Try to optize the copying process by avoiding unaligned memory accesses.
> >
> > Signed-off-by: Helge Deller <deller@gmx.de>
> >
> > diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
> > index 1fbe82f5021b..386da2023d50 100644
> > --- a/security/apparmor/include/match.h
> > +++ b/security/apparmor/include/match.h
> > @@ -104,16 +104,20 @@ struct aa_dfa {
> > struct table_header *tables[YYTD_ID_TSIZE];
> > };
> > -#define byte_to_byte(X) (X)
> > +#define byte_to_byte(X) (*(X))
> > #define UNPACK_ARRAY(TABLE, BLOB, LEN, TTYPE, BTYPE, NTOHX) \
> > do { \
> > typeof(LEN) __i; \
> > TTYPE *__t = (TTYPE *) TABLE; \
> > BTYPE *__b = (BTYPE *) BLOB; \
> > - for (__i = 0; __i < LEN; __i++) { \
> > - __t[__i] = NTOHX(__b[__i]); \
> > - } \
> > + BUILD_BUG_ON(sizeof(TTYPE) != sizeof(BTYPE)); \
> > + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) || sizeof(BTYPE) == 1) \
> > + memcpy(__t, __b, (LEN) * sizeof(BTYPE)); \
> > + else /* copy & convert convert from big-endian */ \
> > + for (__i = 0; __i < LEN; __i++) { \
> > + __t[__i] = NTOHX(&__b[__i]); \
> > + } \
> > } while (0)
> > static inline size_t table_size(size_t len, size_t el_size)
> > diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> > index c5a91600842a..13e2f6873329 100644
> > --- a/security/apparmor/match.c
> > +++ b/security/apparmor/match.c
> > @@ -15,6 +15,7 @@
> > #include <linux/vmalloc.h>
> > #include <linux/err.h>
> > #include <linux/kref.h>
> > +#include <linux/unaligned.h>
> > #include "include/lib.h"
> > #include "include/match.h"
> > @@ -70,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
> > u8, u8, byte_to_byte);
> > else if (th.td_flags == YYTD_DATA16)
> > UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> > - u16, __be16, be16_to_cpu);
> > + u16, __be16, get_unaligned_be16);
> > else if (th.td_flags == YYTD_DATA32)
> > UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> > - u32, __be32, be32_to_cpu);
> > + u32, __be32, get_unaligned_be32);
> > else
> > goto fail;
> > /* if table was vmalloced make sure the page tables are synced
>
> I think we can make one more tweak, in just not using UNPACK_ARRAY at all for the byte case
> ie.
>
> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> index 26e82ba879d44..389202560675c 100644
> --- a/security/apparmor/match.c
> +++ b/security/apparmor/match.c
> @@ -67,8 +67,7 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
> table->td_flags = th.td_flags;
> table->td_lolen = th.td_lolen;
> if (th.td_flags == YYTD_DATA8)
> - UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> - u8, u8, byte_to_byte);
> + memcp(table->td_data, blob, th.td_lolen);
True.
Then byte_to_byte() can go away in match.h as well.
So, here is a (untested) v3:
[PATCH v3] apparmor: Optimize table creation from possibly unaligned memory
Source blob may come from userspace and might be unaligned.
Try to optize the copying process by avoiding unaligned memory accesses.
Signed-off-by: Helge Deller <deller@gmx.de>
diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
index 1fbe82f5021b..19e72b3e8f49 100644
--- a/security/apparmor/include/match.h
+++ b/security/apparmor/include/match.h
@@ -104,16 +104,18 @@ struct aa_dfa {
struct table_header *tables[YYTD_ID_TSIZE];
};
-#define byte_to_byte(X) (X)
-
#define UNPACK_ARRAY(TABLE, BLOB, LEN, TTYPE, BTYPE, NTOHX) \
do { \
typeof(LEN) __i; \
TTYPE *__t = (TTYPE *) TABLE; \
BTYPE *__b = (BTYPE *) BLOB; \
- for (__i = 0; __i < LEN; __i++) { \
- __t[__i] = NTOHX(__b[__i]); \
- } \
+ BUILD_BUG_ON(sizeof(TTYPE) != sizeof(BTYPE)); \
+ if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) \
+ memcpy(__t, __b, (LEN) * sizeof(BTYPE)); \
+ else /* copy & convert convert from big-endian */ \
+ for (__i = 0; __i < LEN; __i++) { \
+ __t[__i] = NTOHX(&__b[__i]); \
+ } \
} while (0)
static inline size_t table_size(size_t len, size_t el_size)
diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index c5a91600842a..1e32c8ba14ae 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -15,6 +15,7 @@
#include <linux/vmalloc.h>
#include <linux/err.h>
#include <linux/kref.h>
+#include <linux/unaligned.h>
#include "include/lib.h"
#include "include/match.h"
@@ -66,14 +67,13 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
table->td_flags = th.td_flags;
table->td_lolen = th.td_lolen;
if (th.td_flags == YYTD_DATA8)
- UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
- u8, u8, byte_to_byte);
+ memcpy(table->td_data, blob, th.td_lolen);
else if (th.td_flags == YYTD_DATA16)
UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
- u16, __be16, be16_to_cpu);
+ u16, __be16, get_unaligned_be16);
else if (th.td_flags == YYTD_DATA32)
UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
- u32, __be32, be32_to_cpu);
+ u32, __be32, get_unaligned_be32);
else
goto fail;
/* if table was vmalloced make sure the page tables are synced
^ permalink raw reply related
* Re: [PATCH v15 3/4] lsm: count the LSMs enabled at compile time
From: Andy Shevchenko @ 2025-11-26 19:44 UTC (permalink / raw)
To: Paul Moore
Cc: KP Singh, linux-security-module, bpf, ast, casey, andrii,
keescook, daniel, renauld, revest, song, linux, Kui-Feng Lee,
John Johansen
In-Reply-To: <19ac18b9e00.2843.85c95baa4474aabc7814e68940a78392@paul-moore.com>
On Wed, Nov 26, 2025 at 02:02:24PM -0500, Paul Moore wrote:
> On November 26, 2025 12:14:21 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Fri, Aug 16, 2024 at 05:43:06PM +0200, KP Singh wrote:
...
> > > -/* This counts to 12. Any more, it will return 13th argument. */
> > > -#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10,
> > > _11, _12, _n, X...) _n
> > > -#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7,
> > > 6, 5, 4, 3, 2, 1, 0)
> > > +/* This counts to 15. Any more, it will return 16th argument. */
> > > +#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10,
> > > _11, _12, _13, _14, _15, _n, X...) _n
> > > +#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 15, 14, 13, 12, 11,
> > > 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
> >
> > You forgot to update _12 in the upper comment.
>
> Do you plan to send a patch to fix this?
Not really. Too lazy to write a commit message for it.
Appreciate if you or the initial author or somebody else
can do that.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: John Johansen @ 2025-11-26 19:33 UTC (permalink / raw)
To: Helge Deller, david laight
Cc: Helge Deller, John Paul Adrian Glaubitz, linux-kernel, apparmor,
linux-security-module, linux-parisc
In-Reply-To: <aScY13MEBATreotz@carbonx1>
On 11/26/25 07:12, Helge Deller wrote:
> * david laight <david.laight@runbox.com>:
>> On Wed, 26 Nov 2025 12:03:03 +0100
>> Helge Deller <deller@gmx.de> wrote:
>>
>>> On 11/26/25 11:44, david laight wrote:
>> ...
>>>>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>>>>> index 26e82ba879d44..3dcc342337aca 100644
>>>>> --- a/security/apparmor/match.c
>>>>> +++ b/security/apparmor/match.c
>>>>> @@ -71,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
>>>>> u8, u8, byte_to_byte);
>>>>
>>>> Is that that just memcpy() ?
>>>
>>> No, it's memcpy() only on big-endian machines.
>>
>> You've misread the quoting...
>> The 'data8' case that was only half there is a memcpy().
>>
>>> On little-endian machines it converts from big-endian
>>> 16/32-bit ints to little-endian 16/32-bit ints.
>>>
>>> But I see some potential for optimization here:
>>> a) on big-endian machines just use memcpy()
>>
>> true
>>
>>> b) on little-endian machines use memcpy() to copy from possibly-unaligned
>>> memory to then known-to-be-aligned destination. Then use a loop with
>>> be32_to_cpu() instead of get_unaligned_xx() as it's faster.
>>
>> There is a function that does a loop byteswap of a buffer - no reason
>> to re-invent it.
>
> I assumed there must be something, but I did not see it. Which one?
>
>> But I doubt it is always (if ever) faster to do a copy and then byteswap.
>> The loop control and extra memory accesses kill performance.
>
> Yes, you are probably right.
>
>> Not that I've seen a fast get_unaligned() - I don't think gcc or clang
>> generate optimal code - For LE I think it is something like:
>> low = *(addr & ~3);
>> high = *((addr + 3) & ~3);
>> shift = (addr & 3) * 8;
>> value = low << shift | high >> (32 - shift);
>> Note that it is only 2 aligned memory reads - even for 64bit.
>
> Ok, then maybe we should keep it simple like this patch:
>
> [PATCH v2] apparmor: Optimize table creation from possibly unaligned memory
>
> Source blob may come from userspace and might be unaligned.
> Try to optize the copying process by avoiding unaligned memory accesses.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
> index 1fbe82f5021b..386da2023d50 100644
> --- a/security/apparmor/include/match.h
> +++ b/security/apparmor/include/match.h
> @@ -104,16 +104,20 @@ struct aa_dfa {
> struct table_header *tables[YYTD_ID_TSIZE];
> };
>
> -#define byte_to_byte(X) (X)
> +#define byte_to_byte(X) (*(X))
>
> #define UNPACK_ARRAY(TABLE, BLOB, LEN, TTYPE, BTYPE, NTOHX) \
> do { \
> typeof(LEN) __i; \
> TTYPE *__t = (TTYPE *) TABLE; \
> BTYPE *__b = (BTYPE *) BLOB; \
> - for (__i = 0; __i < LEN; __i++) { \
> - __t[__i] = NTOHX(__b[__i]); \
> - } \
> + BUILD_BUG_ON(sizeof(TTYPE) != sizeof(BTYPE)); \
> + if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) || sizeof(BTYPE) == 1) \
> + memcpy(__t, __b, (LEN) * sizeof(BTYPE)); \
> + else /* copy & convert convert from big-endian */ \
> + for (__i = 0; __i < LEN; __i++) { \
> + __t[__i] = NTOHX(&__b[__i]); \
> + } \
> } while (0)
>
> static inline size_t table_size(size_t len, size_t el_size)
> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> index c5a91600842a..13e2f6873329 100644
> --- a/security/apparmor/match.c
> +++ b/security/apparmor/match.c
> @@ -15,6 +15,7 @@
> #include <linux/vmalloc.h>
> #include <linux/err.h>
> #include <linux/kref.h>
> +#include <linux/unaligned.h>
>
> #include "include/lib.h"
> #include "include/match.h"
> @@ -70,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
> u8, u8, byte_to_byte);
> else if (th.td_flags == YYTD_DATA16)
> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> - u16, __be16, be16_to_cpu);
> + u16, __be16, get_unaligned_be16);
> else if (th.td_flags == YYTD_DATA32)
> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
> - u32, __be32, be32_to_cpu);
> + u32, __be32, get_unaligned_be32);
> else
> goto fail;
> /* if table was vmalloced make sure the page tables are synced
I think we can make one more tweak, in just not using UNPACK_ARRAY at all for the byte case
ie.
diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index 26e82ba879d44..389202560675c 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -67,8 +67,7 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
table->td_flags = th.td_flags;
table->td_lolen = th.td_lolen;
if (th.td_flags == YYTD_DATA8)
- UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
- u8, u8, byte_to_byte);
+ memcp(table->td_data, blob, th.td_lolen);
else if (th.td_flags == YYTD_DATA16)
UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
u16, __be16, be16_to_cpu);
^ permalink raw reply related
* [PATCH v4 0/4] Landlock: Disconnected directory handling
From: Mickaël Salaün @ 2025-11-26 19:11 UTC (permalink / raw)
To: Günther Noack, Tingmao Wang
Cc: Mickaël Salaün, Al Viro, Ben Scarlato,
Christian Brauner, Jann Horn, Jeff Xu, Justin Suess,
Mikhail Ivanov, Paul Moore, Song Liu, linux-fsdevel,
linux-security-module
Hi,
This patch series fixes and test Landlock's handling of disconnected
directories.
This fourth version implements an hybrid version of disconnected
directory handling, simpler and safer, suggested by Tingmao. One patch
was merged, and a new one improve variable scope.
Previous versions:
v3: https://lore.kernel.org/r/20250719104204.545188-1-mic@digikod.net
v2: https://lore.kernel.org/r/20250711191938.2007175-1-mic@digikod.net
v1: https://lore.kernel.org/r/20250701183812.3201231-1-mic@digikod.net
Regards,
Mickaël Salaün (3):
landlock: Fix handling of disconnected directories
landlock: Improve variable scope
selftests/landlock: Add disconnected leafs and branch test suites
Tingmao Wang (1):
selftests/landlock: Add tests for access through disconnected paths
security/landlock/errata/abi-1.h | 16 +
security/landlock/fs.c | 43 +-
tools/testing/selftests/landlock/fs_test.c | 1335 +++++++++++++++++++-
3 files changed, 1373 insertions(+), 21 deletions(-)
create mode 100644 security/landlock/errata/abi-1.h
--
2.51.0
^ permalink raw reply
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: John Johansen @ 2025-11-26 19:22 UTC (permalink / raw)
To: david laight
Cc: Helge Deller, Helge Deller, John Paul Adrian Glaubitz,
linux-kernel, apparmor, linux-security-module, linux-parisc
In-Reply-To: <20251126104444.29002552@pumpkin>
On 11/26/25 02:44, david laight wrote:
> On Wed, 26 Nov 2025 01:11:45 -0800
> John Johansen <john.johansen@canonical.com> wrote:
>
>> On 11/25/25 13:13, Helge Deller wrote:
>>> On 11/25/25 20:20, John Johansen wrote:
>>>> On 11/25/25 07:11, Helge Deller wrote:
>>>>> * John Johansen <john.johansen@canonical.com>:
>>>>>> On 11/18/25 04:49, Helge Deller wrote:
>>>>>>> Hi Adrian,
>>>>>>>
>>>>>>> On 11/18/25 12:43, John Paul Adrian Glaubitz wrote:
>>>>>>>> On Tue, 2025-11-18 at 12:09 +0100, Helge Deller wrote:
>>>>>>>>> My patch fixed two call sites, but I suspect you see another call site which
>>>>>>>>> hasn't been fixed yet.
>>>>>>>>>
>>>>>>>>> Can you try attached patch? It might indicate the caller of the function and
>>>>>>>>> maybe prints the struct name/address which isn't aligned.
>>>>>>>>>
>>>>>>>>> Helge
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>>>>>>>>> index c5a91600842a..b477430c07eb 100644
>>>>>>>>> --- a/security/apparmor/match.c
>>>>>>>>> +++ b/security/apparmor/match.c
>>>>>>>>> @@ -313,6 +313,9 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags)
>>>>>>>>> if (size < sizeof(struct table_set_header))
>>>>>>>>> goto fail;
>>>>>>>>> + if (WARN_ON(((unsigned long)data) & (BITS_PER_LONG/8 - 1)))
>>>>>>>>> + pr_warn("dfa blob stream %pS not aligned.\n", data);
>>>>>>>>> +
>>>>>>>>> if (ntohl(*(__be32 *) data) != YYTH_MAGIC)
>>>>>>>>> goto fail;
>>>>>>>>
>>>>>>>> Here is the relevant output with the patch applied:
>>>>>>>>
>>>>>>>> [ 73.840639] ------------[ cut here ]------------
>>>>>>>> [ 73.901376] WARNING: CPU: 0 PID: 341 at security/apparmor/match.c:316 aa_dfa_unpack+0x6cc/0x720
>>>>>>>> [ 74.015867] Modules linked in: binfmt_misc evdev flash sg drm drm_panel_orientation_quirks backlight i2c_core configfs nfnetlink autofs4 ext4 crc16 mbcache jbd2 hid_generic usbhid sr_mod hid cdrom
>>>>>>>> sd_mod ata_generic ohci_pci ehci_pci ehci_hcd ohci_hcd pata_ali libata sym53c8xx scsi_transport_spi tg3 scsi_mod usbcore libphy scsi_common mdio_bus usb_common
>>>>>>>> [ 74.428977] CPU: 0 UID: 0 PID: 341 Comm: apparmor_parser Not tainted 6.18.0-rc6+ #9 NONE
>>>>>>>> [ 74.536543] Call Trace:
>>>>>>>> [ 74.568561] [<0000000000434c24>] dump_stack+0x8/0x18
>>>>>>>> [ 74.633757] [<0000000000476438>] __warn+0xd8/0x100
>>>>>>>> [ 74.696664] [<00000000004296d4>] warn_slowpath_fmt+0x34/0x74
>>>>>>>> [ 74.771006] [<00000000008db28c>] aa_dfa_unpack+0x6cc/0x720
>>>>>>>> [ 74.843062] [<00000000008e643c>] unpack_pdb+0xbc/0x7e0
>>>>>>>> [ 74.910545] [<00000000008e7740>] unpack_profile+0xbe0/0x1300
>>>>>>>> [ 74.984888] [<00000000008e82e0>] aa_unpack+0xe0/0x6a0
>>>>>>>> [ 75.051226] [<00000000008e3ec4>] aa_replace_profiles+0x64/0x1160
>>>>>>>> [ 75.130144] [<00000000008d4d90>] policy_update+0xf0/0x280
>>>>>>>> [ 75.201057] [<00000000008d4fc8>] profile_replace+0xa8/0x100
>>>>>>>> [ 75.274258] [<0000000000766bd0>] vfs_write+0x90/0x420
>>>>>>>> [ 75.340594] [<00000000007670cc>] ksys_write+0x4c/0xe0
>>>>>>>> [ 75.406932] [<0000000000767174>] sys_write+0x14/0x40
>>>>>>>> [ 75.472126] [<0000000000406174>] linux_sparc_syscall+0x34/0x44
>>>>>>>> [ 75.548802] ---[ end trace 0000000000000000 ]---
>>>>>>>> [ 75.609503] dfa blob stream 0xfff0000008926b96 not aligned.
>>>>>>>> [ 75.682695] Kernel unaligned access at TPC[8db2a8] aa_dfa_unpack+0x6e8/0x720
>>>>>>>
>>>>>>> The non-8-byte-aligned address (0xfff0000008926b96) is coming from userspace
>>>>>>> (via the write syscall).
>>>>>>> Some apparmor userspace tool writes into the apparmor ".replace" virtual file with
>>>>>>> a source address which is not correctly aligned.
>>>>>>
>>>>>> the userpace buffer passed to write(2) has to be aligned? Its certainly nice if it
>>>>>> is but the userspace tooling hasn't been treating it as aligned. With that said,
>>>>>> the dfa should be padded to be aligned. So this tripping in the dfa is a bug,
>>>>>> and there really should be some validation to catch it.
>>>>>>
>>>>>>> You should be able to debug/find the problematic code with strace from userspace.
>>>>>>> Maybe someone with apparmor knowledge here on the list has an idea?
>>>>>>>
>>>>>> This is likely an unaligned 2nd profile, being split out and loaded separately
>>>>>> from the rest of the container. Basically the loader for some reason (there
>>>>>> are a few different possible reasons) is poking into the container format and
>>>>>> pulling out the profile at some offset, this gets loaded to the kernel but
>>>>>> it would seem that its causing an issue with the dfa alignment within the container,
>>>>>> which should be aligned to the original container.
>>>>>
>>>>>
>>>>> Regarding this:
>>>>>
>>>>>> Kernel side, we are going to need to add some extra verification checks, it should
>>>>>> be catching this, as unaligned as part of the unpack. Userspace side, we will have
>>>>>> to verify my guess and fix the loader.
>>>>>
>>>>> I wonder if loading those tables are really time critical?
>>>>
>>>> no, most policy is loaded once on boot and then at package upgrades. There are some
>>>> bits that may be loaded at application startup like, snap, libvirt, lxd, basically
>>>> container managers might do some thing custom per container.
>>>>
>>>> Its the run time we want to minimize, the cost of.
>>>>
>>>> Policy already can be unaligned (the container format rework to fix this is low
>>>> priority), and is treated as untrusted. It goes through an unpack, and translation to
>>>> machine native, with as many bounds checks, necessary transforms etc done at unpack
>>>> time as possible, so that the run time costs can be minimized.
>>>>
>>>>> If not, maybe just making the kernel aware that the tables might be unaligned
>>>>> can help, e.g. with the following (untested) patch.
>>>>> Adrian, maybe you want to test?
>>>>>
>>>>
>>>>> ------------------------
>>>>>
>>>>> [PATCH] Allow apparmor to handle unaligned dfa tables
>>>>>
>>>>> The dfa tables can originate from kernel or userspace and 8-byte alignment
>>>>> isn't always guaranteed and as such may trigger unaligned memory accesses
>>>>> on various architectures.
>>>>> Work around it by using the get_unaligned_xx() helpers.
>>>>>
>>>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>>>
>>>> lgtm,
>>>>
>>>> Acked-by: John Johansen <john.johansen@canonical.com>
>>>>
>>>> I'll pull this into my tree regardless of whether it fixes the issue
>>>> for Adrian, as it definitely fixes an issue.
>>>>
>>>> We can added additional patches on top s needed.
>>>
>>> My patch does not modify the UNPACK_ARRAY() macro, which we
>>> possibly should adjust as well.
>>
>> Indeed. See the patch below. I am not surprised testing hasn't triggered this
>> case, but a malicious userspace could certainly construct a policy that would
>> trigger it. Yes it would have to be root, but I still would like to prevent
>> root from being able to trigger this.
>>
>>> Adrian's testing seems to trigger only a few unaligned accesses,
>>> so maybe it's not a issue currently.
>>>
>> I don't think the userspace compiler is generating one that is bad, but it
>> possible to construct one and get it to the point where it can trip in
>> UNPACK_ARRAY
>>
>> commit 2c87528c1e7be3976b61ac797c6c8700364c4c63
>> Author: John Johansen <john.johansen@canonical.com>
>> Date: Tue Nov 25 13:59:32 2025 -0800
>>
>> apparmor: fix unaligned memory access of UNPACK_ARRAY
>>
>> The UNPACK_ARRAY macro has the potential to have unaligned memory
>> access when the unpacking an unaligned profile, which is caused by
>> userspace splitting up a profile container before sending it to the
>> kernel.
>>
>> While this is corner case, policy loaded from userspace should be
>> treated as untrusted so ensure that userspace can not trigger an
>> unaligned access.
>>
>> Signed-off-by: John Johansen <john.johansen@canonical.com>
>>
>> diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
>> index 1fbe82f5021b1..203f7c07529f5 100644
>> --- a/security/apparmor/include/match.h
>> +++ b/security/apparmor/include/match.h
>> @@ -104,7 +104,7 @@ struct aa_dfa {
>> struct table_header *tables[YYTD_ID_TSIZE];
>> };
>>
>> -#define byte_to_byte(X) (X)
>> +#define byte_to_byte(X) *(X)
>
> Even though is is only used once that ought to be (*(X))
>
>>
>> #define UNPACK_ARRAY(TABLE, BLOB, LEN, TTYPE, BTYPE, NTOHX) \
>> do { \
>> @@ -112,7 +112,7 @@ struct aa_dfa {
>> TTYPE *__t = (TTYPE *) TABLE; \
>> BTYPE *__b = (BTYPE *) BLOB; \
>> for (__i = 0; __i < LEN; __i++) { \
>> - __t[__i] = NTOHX(__b[__i]); \
>> + __t[__i] = NTOHX(&__b[__i]); \
>> } \
>> } while (0)
>>
>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>> index 26e82ba879d44..3dcc342337aca 100644
>> --- a/security/apparmor/match.c
>> +++ b/security/apparmor/match.c
>> @@ -71,10 +71,10 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
>> u8, u8, byte_to_byte);
>
> Is that that just memcpy() ?
>
yeah for the byte case it is and we should just replace that case of UNPACK_ARRAY
> David
>
>> else if (th.td_flags == YYTD_DATA16)
>> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
>> - u16, __be16, be16_to_cpu);
>> + u16, __be16, get_unaligned_be16);
>> else if (th.td_flags == YYTD_DATA32)
>> UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
>> - u32, __be32, be32_to_cpu);
>> + u32, __be32, get_unaligned_be32);
>> else
>> goto fail;
>> /* if table was vmalloced make sure the page tables are synced
>>
>>
>>
>
^ permalink raw reply
* [PATCH v4 2/4] landlock: Improve variable scope
From: Mickaël Salaün @ 2025-11-26 19:11 UTC (permalink / raw)
To: Günther Noack, Tingmao Wang
Cc: Mickaël Salaün, Al Viro, Ben Scarlato,
Christian Brauner, Jann Horn, Jeff Xu, Justin Suess,
Mikhail Ivanov, Paul Moore, Song Liu, linux-fsdevel,
linux-security-module
In-Reply-To: <20251126191159.3530363-1-mic@digikod.net>
This is now possible thanks to the disconnected directory fix.
Cc: Günther Noack <gnoack@google.com>
Cc: Song Liu <song@kernel.org>
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v3:
- New patch extracted from the previous one.
---
security/landlock/fs.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 26d5c274a4c9..ee2fa7382a9b 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -837,7 +837,6 @@ static bool is_access_to_paths_allowed(
* restriction.
*/
while (true) {
- struct dentry *parent_dentry;
const struct landlock_rule *rule;
/*
@@ -930,7 +929,9 @@ static bool is_access_to_paths_allowed(
walker_path.dentry = walker_path.mnt->mnt_root;
dget(walker_path.dentry);
} else {
- parent_dentry = dget_parent(walker_path.dentry);
+ struct dentry *const parent_dentry =
+ dget_parent(walker_path.dentry);
+
dput(walker_path.dentry);
walker_path.dentry = parent_dentry;
}
--
2.51.0
^ permalink raw reply related
* [PATCH v4 4/4] selftests/landlock: Add disconnected leafs and branch test suites
From: Mickaël Salaün @ 2025-11-26 19:11 UTC (permalink / raw)
To: Günther Noack, Tingmao Wang
Cc: Mickaël Salaün, Al Viro, Ben Scarlato,
Christian Brauner, Jann Horn, Jeff Xu, Justin Suess,
Mikhail Ivanov, Paul Moore, Song Liu, linux-fsdevel,
linux-security-module
In-Reply-To: <20251126191159.3530363-1-mic@digikod.net>
Test disconnected directories with two test suites and 31 variants to
cover the main corner cases.
These tests are complementary to the previous commit.
Add test_renameat() and test_exchangeat() helpers.
Test coverage for security/landlock is 92.1% of 1927 lines according to
LLVM 20.
Cc: Günther Noack <gnoack@google.com>
Cc: Song Liu <song@kernel.org>
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v3:
- Update tests to reflect the new approach:
* layout4_disconnected_leafs.s1d41_s1d42_disconnected: allow all
* layout4_disconnected_leafs.s3d1_s4d1_new_parent: allow all
* layout4_disconnected_leafs.f1_f2_f3: allow read
* layout5_disconnected_branch.s2d3_mount1_dst_parent: allow all
* layout5_disconnected_branch.s4d1_rename_parent: allow all
- Update test coverage.
Changes since v2:
- Update test coverage.
Changes since v1:
- Rename layout4_disconnected to layout4_disconnected_leafs.
- Fix variable names.
- Add layout5_disconnected_branch test suite with 19 variants to cover
potential implementation issues.
---
tools/testing/selftests/landlock/fs_test.c | 912 +++++++++++++++++++++
1 file changed, 912 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 032dd5dcf5eb..7f9c97219367 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -2267,6 +2267,22 @@ static int test_exchange(const char *const oldpath, const char *const newpath)
return 0;
}
+static int test_renameat(int olddirfd, const char *oldpath, int newdirfd,
+ const char *newpath)
+{
+ if (renameat2(olddirfd, oldpath, newdirfd, newpath, 0))
+ return errno;
+ return 0;
+}
+
+static int test_exchangeat(int olddirfd, const char *oldpath, int newdirfd,
+ const char *newpath)
+{
+ if (renameat2(olddirfd, oldpath, newdirfd, newpath, RENAME_EXCHANGE))
+ return errno;
+ return 0;
+}
+
TEST_F_FORK(layout1, rename_file)
{
const struct rule rules[] = {
@@ -5213,6 +5229,902 @@ TEST_F_FORK(layout1_bind, path_disconnected_link)
}
}
+/*
+ * layout4_disconnected_leafs with bind mount and renames:
+ *
+ * tmp
+ * ├── s1d1
+ * │ └── s1d2 [source of the bind mount]
+ * │ ├── s1d31
+ * │ │ └── s1d41 [now renamed beneath s3d1]
+ * │ │ ├── f1
+ * │ │ └── f2
+ * │ └── s1d32
+ * │ └── s1d42 [now renamed beneath s4d1]
+ * │ ├── f3
+ * │ └── f4
+ * ├── s2d1
+ * │ └── s2d2 [bind mount of s1d2]
+ * │ ├── s1d31
+ * │ │ └── s1d41 [opened FD, now renamed beneath s3d1]
+ * │ │ ├── f1
+ * │ │ └── f2
+ * │ └── s1d32
+ * │ └── s1d42 [opened FD, now renamed beneath s4d1]
+ * │ ├── f3
+ * │ └── f4
+ * ├── s3d1
+ * │ └── s1d41 [renamed here]
+ * │ ├── f1
+ * │ └── f2
+ * └── s4d1
+ * └── s1d42 [renamed here]
+ * ├── f3
+ * └── f4
+ */
+/* clang-format off */
+FIXTURE(layout4_disconnected_leafs) {
+ int s2d2_fd;
+};
+/* clang-format on */
+
+FIXTURE_SETUP(layout4_disconnected_leafs)
+{
+ prepare_layout(_metadata);
+
+ create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d31/s1d41/f1");
+ create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d31/s1d41/f2");
+ create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f3");
+ create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f4");
+ create_directory(_metadata, TMP_DIR "/s2d1/s2d2");
+ create_directory(_metadata, TMP_DIR "/s3d1");
+ create_directory(_metadata, TMP_DIR "/s4d1");
+
+ self->s2d2_fd =
+ open(TMP_DIR "/s2d1/s2d2", O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, self->s2d2_fd);
+
+ set_cap(_metadata, CAP_SYS_ADMIN);
+ ASSERT_EQ(0, mount(TMP_DIR "/s1d1/s1d2", TMP_DIR "/s2d1/s2d2", NULL,
+ MS_BIND, NULL));
+ clear_cap(_metadata, CAP_SYS_ADMIN);
+}
+
+FIXTURE_TEARDOWN_PARENT(layout4_disconnected_leafs)
+{
+ /* umount(TMP_DIR "/s2d1") is handled by namespace lifetime. */
+
+ /* Removes files after renames. */
+ remove_path(TMP_DIR "/s3d1/s1d41/f1");
+ remove_path(TMP_DIR "/s3d1/s1d41/f2");
+ remove_path(TMP_DIR "/s4d1/s1d42/f1");
+ remove_path(TMP_DIR "/s4d1/s1d42/f3");
+ remove_path(TMP_DIR "/s4d1/s1d42/f4");
+ remove_path(TMP_DIR "/s4d1/s1d42/f5");
+
+ cleanup_layout(_metadata);
+}
+
+FIXTURE_VARIANT(layout4_disconnected_leafs)
+{
+ /*
+ * Parent of the bind mount source. It should always be ignored when
+ * testing against files under the s1d41 or s1d42 disconnected directories.
+ */
+ const __u64 allowed_s1d1;
+ /*
+ * Source of bind mount (to s2d2). It should always be enforced when
+ * testing against files under the s1d41 or s1d42 disconnected directories.
+ */
+ const __u64 allowed_s1d2;
+ /*
+ * Original parent of s1d41. It should always be ignored when testing
+ * against files under the s1d41 disconnected directory.
+ */
+ const __u64 allowed_s1d31;
+ /*
+ * Original parent of s1d42. It should always be ignored when testing
+ * against files under the s1d42 disconnected directory.
+ */
+ const __u64 allowed_s1d32;
+ /*
+ * Opened and disconnected source directory. It should always be enforced
+ * when testing against files under the s1d41 disconnected directory.
+ */
+ const __u64 allowed_s1d41;
+ /*
+ * Opened and disconnected source directory. It should always be enforced
+ * when testing against files under the s1d42 disconnected directory.
+ */
+ const __u64 allowed_s1d42;
+ /*
+ * File in the s1d41 disconnected directory. It should always be enforced
+ * when testing against itself under the s1d41 disconnected directory.
+ */
+ const __u64 allowed_f1;
+ /*
+ * File in the s1d41 disconnected directory. It should always be enforced
+ * when testing against itself under the s1d41 disconnected directory.
+ */
+ const __u64 allowed_f2;
+ /*
+ * File in the s1d42 disconnected directory. It should always be enforced
+ * when testing against itself under the s1d42 disconnected directory.
+ */
+ const __u64 allowed_f3;
+ /*
+ * Parent of the bind mount destination. It should always be enforced when
+ * testing against files under the s1d41 or s1d42 disconnected directories.
+ */
+ const __u64 allowed_s2d1;
+ /*
+ * Directory covered by the bind mount. It should always be ignored when
+ * testing against files under the s1d41 or s1d42 disconnected directories.
+ */
+ const __u64 allowed_s2d2;
+ /*
+ * New parent of the renamed s1d41. It should always be ignored when
+ * testing against files under the s1d41 disconnected directory.
+ */
+ const __u64 allowed_s3d1;
+ /*
+ * New parent of the renamed s1d42. It should always be ignored when
+ * testing against files under the s1d42 disconnected directory.
+ */
+ const __u64 allowed_s4d1;
+
+ /* Expected result of the call to open([fd:s1d41]/f1, O_RDONLY). */
+ const int expected_read_result;
+ /* Expected result of the call to renameat([fd:s1d41]/f1, [fd:s1d42]/f1). */
+ const int expected_rename_result;
+ /*
+ * Expected result of the call to renameat([fd:s1d41]/f2, [fd:s1d42]/f3,
+ * RENAME_EXCHANGE).
+ */
+ const int expected_exchange_result;
+ /* Expected result of the call to renameat([fd:s1d42]/f4, [fd:s1d42]/f5). */
+ const int expected_same_dir_rename_result;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d1_mount_src_parent) {
+ /* clang-format on */
+ .allowed_s1d1 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d2_mount_src_refer) {
+ /* clang-format on */
+ .allowed_s1d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d2_mount_src_create) {
+ /* clang-format on */
+ .allowed_s1d2 = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = EXDEV,
+ .expected_exchange_result = EXDEV,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d2_mount_src_rename) {
+ /* clang-format on */
+ .allowed_s1d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d31_s1d32_old_parent) {
+ /* clang-format on */
+ .allowed_s1d31 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .allowed_s1d32 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s1d41_s1d42_disconnected) {
+ /* clang-format on */
+ .allowed_s1d41 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .allowed_s1d42 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s2d1_mount_dst_parent_create) {
+ /* clang-format on */
+ .allowed_s2d1 = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = EXDEV,
+ .expected_exchange_result = EXDEV,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s2d1_mount_dst_parent_refer) {
+ /* clang-format on */
+ .allowed_s2d1 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s2d1_mount_dst_parent_mini) {
+ /* clang-format on */
+ .allowed_s2d1 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s2d2_covered_by_mount) {
+ /* clang-format on */
+ .allowed_s2d2 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* Tests collect_domain_accesses(). */
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, s3d1_s4d1_new_parent) {
+ /* clang-format on */
+ .allowed_s3d1 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .allowed_s4d1 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout4_disconnected_leafs, f1_f2_f3) {
+ /* clang-format on */
+ .allowed_f1 = LANDLOCK_ACCESS_FS_READ_FILE,
+ .allowed_f2 = LANDLOCK_ACCESS_FS_READ_FILE,
+ .allowed_f3 = LANDLOCK_ACCESS_FS_READ_FILE,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+TEST_F_FORK(layout4_disconnected_leafs, read_rename_exchange)
+{
+ const __u64 handled_access =
+ LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_MAKE_REG;
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR "/s1d1",
+ .access = variant->allowed_s1d1,
+ },
+ {
+ .path = TMP_DIR "/s1d1/s1d2",
+ .access = variant->allowed_s1d2,
+ },
+ {
+ .path = TMP_DIR "/s1d1/s1d2/s1d31",
+ .access = variant->allowed_s1d31,
+ },
+ {
+ .path = TMP_DIR "/s1d1/s1d2/s1d32",
+ .access = variant->allowed_s1d32,
+ },
+ {
+ .path = TMP_DIR "/s1d1/s1d2/s1d31/s1d41",
+ .access = variant->allowed_s1d41,
+ },
+ {
+ .path = TMP_DIR "/s1d1/s1d2/s1d32/s1d42",
+ .access = variant->allowed_s1d42,
+ },
+ {
+ .path = TMP_DIR "/s1d1/s1d2/s1d31/s1d41/f1",
+ .access = variant->allowed_f1,
+ },
+ {
+ .path = TMP_DIR "/s1d1/s1d2/s1d31/s1d41/f2",
+ .access = variant->allowed_f2,
+ },
+ {
+ .path = TMP_DIR "/s1d1/s1d2/s1d32/s1d42/f3",
+ .access = variant->allowed_f3,
+ },
+ {
+ .path = TMP_DIR "/s2d1",
+ .access = variant->allowed_s2d1,
+ },
+ /* s2d2_fd */
+ {
+ .path = TMP_DIR "/s3d1",
+ .access = variant->allowed_s3d1,
+ },
+ {
+ .path = TMP_DIR "/s4d1",
+ .access = variant->allowed_s4d1,
+ },
+ {},
+ };
+ int ruleset_fd, s1d41_bind_fd, s1d42_bind_fd;
+
+ ruleset_fd = create_ruleset(_metadata, handled_access, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Adds rule for the covered directory. */
+ if (variant->allowed_s2d2) {
+ ASSERT_EQ(0, landlock_add_rule(
+ ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = self->s2d2_fd,
+ .allowed_access =
+ variant->allowed_s2d2,
+ },
+ 0));
+ }
+ EXPECT_EQ(0, close(self->s2d2_fd));
+
+ s1d41_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d31/s1d41",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d41_bind_fd);
+ s1d42_bind_fd = open(TMP_DIR "/s2d1/s2d2/s1d32/s1d42",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d42_bind_fd);
+
+ /* Disconnects and checks source and destination directories. */
+ EXPECT_EQ(0, test_open_rel(s1d41_bind_fd, "..", O_DIRECTORY));
+ EXPECT_EQ(0, test_open_rel(s1d42_bind_fd, "..", O_DIRECTORY));
+ /* Renames to make it accessible through s3d1/s1d41 */
+ ASSERT_EQ(0, test_renameat(AT_FDCWD, TMP_DIR "/s1d1/s1d2/s1d31/s1d41",
+ AT_FDCWD, TMP_DIR "/s3d1/s1d41"));
+ /* Renames to make it accessible through s4d1/s1d42 */
+ ASSERT_EQ(0, test_renameat(AT_FDCWD, TMP_DIR "/s1d1/s1d2/s1d32/s1d42",
+ AT_FDCWD, TMP_DIR "/s4d1/s1d42"));
+ EXPECT_EQ(ENOENT, test_open_rel(s1d41_bind_fd, "..", O_DIRECTORY));
+ EXPECT_EQ(ENOENT, test_open_rel(s1d42_bind_fd, "..", O_DIRECTORY));
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ EXPECT_EQ(0, close(ruleset_fd));
+
+ EXPECT_EQ(variant->expected_read_result,
+ test_open_rel(s1d41_bind_fd, "f1", O_RDONLY));
+
+ EXPECT_EQ(variant->expected_rename_result,
+ test_renameat(s1d41_bind_fd, "f1", s1d42_bind_fd, "f1"));
+ EXPECT_EQ(variant->expected_exchange_result,
+ test_exchangeat(s1d41_bind_fd, "f2", s1d42_bind_fd, "f3"));
+
+ EXPECT_EQ(variant->expected_same_dir_rename_result,
+ test_renameat(s1d42_bind_fd, "f4", s1d42_bind_fd, "f5"));
+}
+
+/*
+ * layout5_disconnected_branch before rename:
+ *
+ * tmp
+ * ├── s1d1
+ * │ └── s1d2 [source of the first bind mount]
+ * │ └── s1d3
+ * │ ├── s1d41
+ * │ │ ├── f1
+ * │ │ └── f2
+ * │ └── s1d42
+ * │ ├── f3
+ * │ └── f4
+ * ├── s2d1
+ * │ └── s2d2 [source of the second bind mount]
+ * │ └── s2d3
+ * │ └── s2d4 [first s1d2 bind mount]
+ * │ └── s1d3
+ * │ ├── s1d41
+ * │ │ ├── f1
+ * │ │ └── f2
+ * │ └── s1d42
+ * │ ├── f3
+ * │ └── f4
+ * ├── s3d1
+ * │ └── s3d2 [second s2d2 bind mount]
+ * │ └── s2d3
+ * │ └── s2d4 [first s1d2 bind mount]
+ * │ └── s1d3
+ * │ ├── s1d41
+ * │ │ ├── f1
+ * │ │ └── f2
+ * │ └── s1d42
+ * │ ├── f3
+ * │ └── f4
+ * └── s4d1
+ *
+ * After rename:
+ *
+ * tmp
+ * ├── s1d1
+ * │ └── s1d2 [source of the first bind mount]
+ * │ └── s1d3
+ * │ ├── s1d41
+ * │ │ ├── f1
+ * │ │ └── f2
+ * │ └── s1d42
+ * │ ├── f3
+ * │ └── f4
+ * ├── s2d1
+ * │ └── s2d2 [source of the second bind mount]
+ * ├── s3d1
+ * │ └── s3d2 [second s2d2 bind mount]
+ * └── s4d1
+ * └── s2d3 [renamed here]
+ * └── s2d4 [first s1d2 bind mount]
+ * └── s1d3
+ * ├── s1d41
+ * │ ├── f1
+ * │ └── f2
+ * └── s1d42
+ * ├── f3
+ * └── f4
+ *
+ * Decision path: s1d3 -> s1d2 -> s2d2 -> s3d1 -> tmp
+ * s2d3 is ignored, as well as the directories under the mount points.
+ */
+
+/* clang-format off */
+FIXTURE(layout5_disconnected_branch) {
+ int s2d4_fd, s3d2_fd;
+};
+/* clang-format on */
+
+FIXTURE_SETUP(layout5_disconnected_branch)
+{
+ prepare_layout(_metadata);
+
+ create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d3/s1d41/f1");
+ create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d3/s1d41/f2");
+ create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f3");
+ create_file(_metadata, TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f4");
+ create_directory(_metadata, TMP_DIR "/s2d1/s2d2/s2d3/s2d4");
+ create_directory(_metadata, TMP_DIR "/s3d1/s3d2");
+ create_directory(_metadata, TMP_DIR "/s4d1");
+
+ self->s2d4_fd = open(TMP_DIR "/s2d1/s2d2/s2d3/s2d4",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, self->s2d4_fd);
+
+ self->s3d2_fd =
+ open(TMP_DIR "/s3d1/s3d2", O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, self->s3d2_fd);
+
+ set_cap(_metadata, CAP_SYS_ADMIN);
+ ASSERT_EQ(0, mount(TMP_DIR "/s1d1/s1d2", TMP_DIR "/s2d1/s2d2/s2d3/s2d4",
+ NULL, MS_BIND, NULL));
+ ASSERT_EQ(0, mount(TMP_DIR "/s2d1/s2d2", TMP_DIR "/s3d1/s3d2", NULL,
+ MS_BIND | MS_REC, NULL));
+ clear_cap(_metadata, CAP_SYS_ADMIN);
+}
+
+FIXTURE_TEARDOWN_PARENT(layout5_disconnected_branch)
+{
+ /* Bind mounts are handled by namespace lifetime. */
+
+ /* Removes files after renames. */
+ remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d41/f1");
+ remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d41/f2");
+ remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f1");
+ remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f3");
+ remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f4");
+ remove_path(TMP_DIR "/s1d1/s1d2/s1d3/s1d42/f5");
+
+ cleanup_layout(_metadata);
+}
+
+FIXTURE_VARIANT(layout5_disconnected_branch)
+{
+ /*
+ * Parent of all files. It should always be enforced when testing against
+ * files under the s1d41 or s1d42 disconnected directories.
+ */
+ const __u64 allowed_base;
+ /*
+ * Parent of the first bind mount source. It should always be ignored when
+ * testing against files under the s1d41 or s1d42 disconnected directories.
+ */
+ const __u64 allowed_s1d1;
+ const __u64 allowed_s1d2;
+ const __u64 allowed_s1d3;
+ const __u64 allowed_s2d1;
+ const __u64 allowed_s2d2;
+ const __u64 allowed_s2d3;
+ const __u64 allowed_s2d4;
+ const __u64 allowed_s3d1;
+ const __u64 allowed_s3d2;
+ const __u64 allowed_s4d1;
+
+ /* Expected result of the call to open([fd:s1d3]/s1d41/f1, O_RDONLY). */
+ const int expected_read_result;
+ /*
+ * Expected result of the call to renameat([fd:s1d3]/s1d41/f1,
+ * [fd:s1d3]/s1d42/f1).
+ */
+ const int expected_rename_result;
+ /*
+ * Expected result of the call to renameat([fd:s1d3]/s1d41/f2,
+ * [fd:s1d3]/s1d42/f3, RENAME_EXCHANGE).
+ */
+ const int expected_exchange_result;
+ /*
+ * Expected result of the call to renameat([fd:s1d3]/s1d42/f4,
+ * [fd:s1d3]/s1d42/f5).
+ */
+ const int expected_same_dir_rename_result;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d1_mount1_src_parent) {
+ /* clang-format on */
+ .allowed_s1d1 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d2_mount1_src_refer) {
+ /* clang-format on */
+ .allowed_s1d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d2_mount1_src_create) {
+ /* clang-format on */
+ .allowed_s1d2 = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = EXDEV,
+ .expected_exchange_result = EXDEV,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d2_mount1_src_rename) {
+ /* clang-format on */
+ .allowed_s1d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d3_fd_refer) {
+ /* clang-format on */
+ .allowed_s1d3 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d3_fd_create) {
+ /* clang-format on */
+ .allowed_s1d3 = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = EXDEV,
+ .expected_exchange_result = EXDEV,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d3_fd_rename) {
+ /* clang-format on */
+ .allowed_s1d3 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s1d3_fd_full) {
+ /* clang-format on */
+ .allowed_s1d3 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d1_mount2_src_parent) {
+ /* clang-format on */
+ .allowed_s2d1 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d2_mount2_src_refer) {
+ /* clang-format on */
+ .allowed_s2d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d2_mount2_src_create) {
+ /* clang-format on */
+ .allowed_s2d2 = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = EXDEV,
+ .expected_exchange_result = EXDEV,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d2_mount2_src_rename) {
+ /* clang-format on */
+ .allowed_s2d2 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d3_mount1_dst_parent) {
+ /* clang-format on */
+ .allowed_s2d3 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s2d4_mount1_dst) {
+ /* clang-format on */
+ .allowed_s2d4 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s3d1_mount2_dst_parent_refer) {
+ /* clang-format on */
+ .allowed_s3d1 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s3d1_mount2_dst_parent_create) {
+ /* clang-format on */
+ .allowed_s3d1 = LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = EXDEV,
+ .expected_exchange_result = EXDEV,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s3d1_mount2_dst_parent_rename) {
+ /* clang-format on */
+ .allowed_s3d1 = LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s3d2_mount1_dst) {
+ /* clang-format on */
+ .allowed_s3d2 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = EACCES,
+ .expected_same_dir_rename_result = EACCES,
+ .expected_rename_result = EACCES,
+ .expected_exchange_result = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(layout5_disconnected_branch, s4d1_rename_parent) {
+ /* clang-format on */
+ .allowed_s4d1 = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE |
+ LANDLOCK_ACCESS_FS_MAKE_REG,
+ .expected_read_result = 0,
+ .expected_same_dir_rename_result = 0,
+ .expected_rename_result = 0,
+ .expected_exchange_result = 0,
+};
+
+TEST_F_FORK(layout5_disconnected_branch, read_rename_exchange)
+{
+ const __u64 handled_access =
+ LANDLOCK_ACCESS_FS_REFER | LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_MAKE_REG;
+ const struct rule rules[] = {
+ {
+ .path = TMP_DIR "/s1d1",
+ .access = variant->allowed_s1d1,
+ },
+ {
+ .path = TMP_DIR "/s1d1/s1d2",
+ .access = variant->allowed_s1d2,
+ },
+ {
+ .path = TMP_DIR "/s1d1/s1d2/s1d3",
+ .access = variant->allowed_s1d3,
+ },
+ {
+ .path = TMP_DIR "/s2d1",
+ .access = variant->allowed_s2d1,
+ },
+ {
+ .path = TMP_DIR "/s2d1/s2d2",
+ .access = variant->allowed_s2d2,
+ },
+ {
+ .path = TMP_DIR "/s2d1/s2d2/s2d3",
+ .access = variant->allowed_s2d3,
+ },
+ /* s2d4_fd */
+ {
+ .path = TMP_DIR "/s3d1",
+ .access = variant->allowed_s3d1,
+ },
+ /* s3d2_fd */
+ {
+ .path = TMP_DIR "/s4d1",
+ .access = variant->allowed_s4d1,
+ },
+ {},
+ };
+ int ruleset_fd, s1d3_bind_fd;
+
+ ruleset_fd = create_ruleset(_metadata, handled_access, rules);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Adds rules for the covered directories. */
+ if (variant->allowed_s2d4) {
+ ASSERT_EQ(0, landlock_add_rule(
+ ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = self->s2d4_fd,
+ .allowed_access =
+ variant->allowed_s2d4,
+ },
+ 0));
+ }
+ EXPECT_EQ(0, close(self->s2d4_fd));
+
+ if (variant->allowed_s3d2) {
+ ASSERT_EQ(0, landlock_add_rule(
+ ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+ &(struct landlock_path_beneath_attr){
+ .parent_fd = self->s3d2_fd,
+ .allowed_access =
+ variant->allowed_s3d2,
+ },
+ 0));
+ }
+ EXPECT_EQ(0, close(self->s3d2_fd));
+
+ s1d3_bind_fd = open(TMP_DIR "/s3d1/s3d2/s2d3/s2d4/s1d3",
+ O_DIRECTORY | O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, s1d3_bind_fd);
+
+ /* Disconnects and checks source and destination directories. */
+ EXPECT_EQ(0, test_open_rel(s1d3_bind_fd, "../../..", O_DIRECTORY));
+ /* Renames to make it accessible through s3d1/s1d41 */
+ ASSERT_EQ(0, test_renameat(AT_FDCWD, TMP_DIR "/s2d1/s2d2/s2d3",
+ AT_FDCWD, TMP_DIR "/s4d1/s2d3"));
+ EXPECT_EQ(ENOENT, test_open_rel(s1d3_bind_fd, "../../..", O_DIRECTORY));
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ EXPECT_EQ(0, close(ruleset_fd));
+
+ EXPECT_EQ(variant->expected_read_result,
+ test_open_rel(s1d3_bind_fd, "s1d41/f1", O_RDONLY));
+
+ EXPECT_EQ(variant->expected_rename_result,
+ test_renameat(s1d3_bind_fd, "s1d41/f1", s1d3_bind_fd,
+ "s1d42/f1"));
+ EXPECT_EQ(variant->expected_exchange_result,
+ test_exchangeat(s1d3_bind_fd, "s1d41/f2", s1d3_bind_fd,
+ "s1d42/f3"));
+
+ EXPECT_EQ(variant->expected_same_dir_rename_result,
+ test_renameat(s1d3_bind_fd, "s1d42/f4", s1d3_bind_fd,
+ "s1d42/f5"));
+}
+
#define LOWER_BASE TMP_DIR "/lower"
#define LOWER_DATA LOWER_BASE "/data"
static const char lower_fl1[] = LOWER_DATA "/fl1";
--
2.51.0
^ permalink raw reply related
* [PATCH v4 3/4] selftests/landlock: Add tests for access through disconnected paths
From: Mickaël Salaün @ 2025-11-26 19:11 UTC (permalink / raw)
To: Günther Noack, Tingmao Wang
Cc: Al Viro, Ben Scarlato, Christian Brauner, Jann Horn, Jeff Xu,
Justin Suess, Mikhail Ivanov, Paul Moore, Song Liu, linux-fsdevel,
linux-security-module, Mickaël Salaün
In-Reply-To: <20251126191159.3530363-1-mic@digikod.net>
From: Tingmao Wang <m@maowtm.org>
This adds tests for the edge case discussed in [1], with specific ones
for rename and link operations when the operands are through
disconnected paths, as that go through a separate code path in Landlock.
This has resulted in a warning, due to collect_domain_accesses() not
expecting to reach a different root from path->mnt:
# RUN layout1_bind.path_disconnected ...
# OK layout1_bind.path_disconnected
ok 96 layout1_bind.path_disconnected
# RUN layout1_bind.path_disconnected_rename ...
[..] ------------[ cut here ]------------
[..] WARNING: CPU: 3 PID: 385 at security/landlock/fs.c:1065 collect_domain_accesses
[..] ...
[..] RIP: 0010:collect_domain_accesses (security/landlock/fs.c:1065 (discriminator 2) security/landlock/fs.c:1031 (discriminator 2))
[..] current_check_refer_path (security/landlock/fs.c:1205)
[..] ...
[..] hook_path_rename (security/landlock/fs.c:1526)
[..] security_path_rename (security/security.c:2026 (discriminator 1))
[..] do_renameat2 (fs/namei.c:5264)
# OK layout1_bind.path_disconnected_rename
ok 97 layout1_bind.path_disconnected_rename
Move the const char definitions a bit above so that we can use the path
for s4d1 in cleanup code.
Cc: Günther Noack <gnoack@google.com>
Cc: Song Liu <song@kernel.org>
Link: https://lore.kernel.org/r/027d5190-b37a-40a8-84e9-4ccbc352bcdf@maowtm.org [1]
Signed-off-by: Tingmao Wang <m@maowtm.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v3:
- Update layout1_bind.path_disconnected tests according to the semantic
change of the fix.
- Update documentation for the layout1_bind hierarchy with the new s4d1
directory.
Changes since v1:
- Integrate this patch into my patch series, and change the result for
two tests with updated comments. Diff here:
https://lore.kernel.org/r/20250701183812.3201231-2-mic@digikod.net
- Replace most ASSERT with EXPECT, add extra checks, massage commit
message and comments.
- Squash Tingmao's patches:
https://lore.kernel.org/r/09b24128f86973a6022e6aa8338945fcfb9a33e4.1749925391.git.m@maowtm.org
https://lore.kernel.org/r/8ed0bfcd-aefa-44bd-86b6-e12583779187@maowtm.org
https://lore.kernel.org/r/3080e512-64b0-42cf-b379-8f52cfeff78a@maowtm.org
---
tools/testing/selftests/landlock/fs_test.c | 423 ++++++++++++++++++++-
1 file changed, 415 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index fa0f18ec62c4..032dd5dcf5eb 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -4561,6 +4561,18 @@ TEST_F_FORK(ioctl, handle_file_access_file)
FIXTURE(layout1_bind) {};
/* clang-format on */
+static const char bind_dir_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3";
+static const char bind_file1_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3/f1";
+
+/* Move targets for disconnected path tests. */
+static const char dir_s4d1[] = TMP_DIR "/s4d1";
+static const char file1_s4d1[] = TMP_DIR "/s4d1/f1";
+static const char file2_s4d1[] = TMP_DIR "/s4d1/f2";
+static const char dir_s4d2[] = TMP_DIR "/s4d1/s4d2";
+static const char file1_s4d2[] = TMP_DIR "/s4d1/s4d2/f1";
+static const char file1_name[] = "f1";
+static const char file2_name[] = "f2";
+
FIXTURE_SETUP(layout1_bind)
{
prepare_layout(_metadata);
@@ -4576,14 +4588,14 @@ FIXTURE_TEARDOWN_PARENT(layout1_bind)
{
/* umount(dir_s2d2)) is handled by namespace lifetime. */
+ remove_path(file1_s4d1);
+ remove_path(file2_s4d1);
+
remove_layout1(_metadata);
cleanup_layout(_metadata);
}
-static const char bind_dir_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3";
-static const char bind_file1_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3/f1";
-
/*
* layout1_bind hierarchy:
*
@@ -4594,20 +4606,25 @@ static const char bind_file1_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3/f1";
* │ └── s1d2
* │ ├── f1
* │ ├── f2
- * │ └── s1d3
+ * │ └── s1d3 [disconnected by path_disconnected]
* │ ├── f1
* │ └── f2
* ├── s2d1
* │ ├── f1
- * │ └── s2d2
+ * │ └── s2d2 [bind mount from s1d2]
* │ ├── f1
* │ ├── f2
* │ └── s1d3
* │ ├── f1
* │ └── f2
- * └── s3d1
- * └── s3d2
- * └── s3d3
+ * ├── s3d1
+ * │ └── s3d2
+ * │ └── s3d3
+ * └── s4d1 [renamed from s1d3 by path_disconnected]
+ * ├── f1
+ * ├── f2
+ * └── s4d2
+ * └── f1
*/
TEST_F_FORK(layout1_bind, no_restriction)
@@ -4806,6 +4823,396 @@ TEST_F_FORK(layout1_bind, reparent_cross_mount)
ASSERT_EQ(0, rename(bind_file1_s1d3, file1_s2d2));
}
+/*
+ * Make sure access to file through a disconnected path works as expected.
+ * This test moves s1d3 to s4d1.
+ */
+TEST_F_FORK(layout1_bind, path_disconnected)
+{
+ const struct rule layer1_allow_all[] = {
+ {
+ .path = TMP_DIR,
+ .access = ACCESS_ALL,
+ },
+ {},
+ };
+ const struct rule layer2_allow_just_f1[] = {
+ {
+ .path = file1_s1d3,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE,
+ },
+ {},
+ };
+ const struct rule layer3_only_s1d2[] = {
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE,
+ },
+ {},
+ };
+
+ /* Landlock should not deny access just because it is disconnected. */
+ int ruleset_fd_l1 =
+ create_ruleset(_metadata, ACCESS_ALL, layer1_allow_all);
+
+ /* Creates the new ruleset now before we move the dir containing the file. */
+ int ruleset_fd_l2 =
+ create_ruleset(_metadata, ACCESS_RW, layer2_allow_just_f1);
+ int ruleset_fd_l3 =
+ create_ruleset(_metadata, ACCESS_RW, layer3_only_s1d2);
+ int bind_s1d3_fd;
+
+ ASSERT_LE(0, ruleset_fd_l1);
+ ASSERT_LE(0, ruleset_fd_l2);
+ ASSERT_LE(0, ruleset_fd_l3);
+
+ enforce_ruleset(_metadata, ruleset_fd_l1);
+ EXPECT_EQ(0, close(ruleset_fd_l1));
+
+ bind_s1d3_fd = open(bind_dir_s1d3, O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, bind_s1d3_fd);
+
+ /* Tests access is possible before we move. */
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY));
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file2_name, O_RDONLY));
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, "..", O_RDONLY | O_DIRECTORY));
+
+ /* Makes it disconnected. */
+ ASSERT_EQ(0, rename(dir_s1d3, dir_s4d1))
+ {
+ TH_LOG("Failed to rename %s to %s: %s", dir_s1d3, dir_s4d1,
+ strerror(errno));
+ }
+
+ /* Tests that access is still possible. */
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY));
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file2_name, O_RDONLY));
+
+ /*
+ * Tests that ".." is not possible (not because of Landlock, but just
+ * because it's disconnected).
+ */
+ EXPECT_EQ(ENOENT,
+ test_open_rel(bind_s1d3_fd, "..", O_RDONLY | O_DIRECTORY));
+
+ /* This should still work with a narrower rule. */
+ enforce_ruleset(_metadata, ruleset_fd_l2);
+ EXPECT_EQ(0, close(ruleset_fd_l2));
+
+ EXPECT_EQ(0, test_open(file1_s4d1, O_RDONLY));
+ /*
+ * Accessing a file through a disconnected file descriptor can still be
+ * allowed by a rule tied to this file, even if it is no longer visible in
+ * its mount point.
+ */
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open_rel(bind_s1d3_fd, file2_name, O_RDONLY));
+
+ enforce_ruleset(_metadata, ruleset_fd_l3);
+ EXPECT_EQ(0, close(ruleset_fd_l3));
+
+ EXPECT_EQ(EACCES, test_open(file1_s4d1, O_RDONLY));
+ /*
+ * Accessing a file through a disconnected file descriptor can still be
+ * allowed by a rule tied to the original mount point, even if it is no
+ * longer visible in its mount point.
+ */
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open_rel(bind_s1d3_fd, file2_name, O_RDONLY));
+}
+
+/*
+ * Test that renameat with disconnected paths works under Landlock. This test
+ * moves s1d3 to s4d2, so that we can have a rule allowing refers on the move
+ * target's immediate parent.
+ */
+TEST_F_FORK(layout1_bind, path_disconnected_rename)
+{
+ const struct rule layer1[] = {
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_MAKE_DIR |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR |
+ LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_READ_FILE,
+ },
+ {
+ .path = dir_s4d1,
+ .access = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_MAKE_DIR |
+ LANDLOCK_ACCESS_FS_REMOVE_DIR |
+ LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE |
+ LANDLOCK_ACCESS_FS_READ_FILE,
+ },
+ {}
+ };
+
+ /* This layer only handles LANDLOCK_ACCESS_FS_READ_FILE. */
+ const struct rule layer2_only_s1d2[] = {
+ {
+ .path = dir_s1d2,
+ .access = LANDLOCK_ACCESS_FS_READ_FILE,
+ },
+ {},
+ };
+ int ruleset_fd_l1, ruleset_fd_l2;
+ pid_t child_pid;
+ int bind_s1d3_fd, status;
+
+ ASSERT_EQ(0, mkdir(dir_s4d1, 0755))
+ {
+ TH_LOG("Failed to create %s: %s", dir_s4d1, strerror(errno));
+ }
+ ruleset_fd_l1 = create_ruleset(_metadata, ACCESS_ALL, layer1);
+ ruleset_fd_l2 = create_ruleset(_metadata, LANDLOCK_ACCESS_FS_READ_FILE,
+ layer2_only_s1d2);
+ ASSERT_LE(0, ruleset_fd_l1);
+ ASSERT_LE(0, ruleset_fd_l2);
+
+ enforce_ruleset(_metadata, ruleset_fd_l1);
+ EXPECT_EQ(0, close(ruleset_fd_l1));
+
+ bind_s1d3_fd = open(bind_dir_s1d3, O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, bind_s1d3_fd);
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY));
+
+ /* Tests ENOENT priority over EACCES for disconnected directory. */
+ EXPECT_EQ(EACCES, test_open_rel(bind_s1d3_fd, "..", O_DIRECTORY));
+ ASSERT_EQ(0, rename(dir_s1d3, dir_s4d2))
+ {
+ TH_LOG("Failed to rename %s to %s: %s", dir_s1d3, dir_s4d2,
+ strerror(errno));
+ }
+ EXPECT_EQ(ENOENT, test_open_rel(bind_s1d3_fd, "..", O_DIRECTORY));
+
+ /*
+ * The file is no longer under s1d2 but we should still be able to access it
+ * with layer 2 because its mount point is evaluated as the first valid
+ * directory because it was initially a parent. Do a fork to test this so
+ * we don't prevent ourselves from renaming it back later.
+ */
+ child_pid = fork();
+ ASSERT_LE(0, child_pid);
+ if (child_pid == 0) {
+ enforce_ruleset(_metadata, ruleset_fd_l2);
+ EXPECT_EQ(0, close(ruleset_fd_l2));
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY));
+ EXPECT_EQ(EACCES, test_open(file1_s4d2, O_RDONLY));
+
+ /*
+ * Tests that access widening checks indeed prevents us from renaming it
+ * back.
+ */
+ EXPECT_EQ(-1, rename(dir_s4d2, dir_s1d3));
+ EXPECT_EQ(EXDEV, errno);
+
+ /*
+ * Including through the now disconnected fd (but it should return
+ * EXDEV).
+ */
+ EXPECT_EQ(-1, renameat(bind_s1d3_fd, file1_name, AT_FDCWD,
+ file1_s2d2));
+ EXPECT_EQ(EXDEV, errno);
+ _exit(_metadata->exit_code);
+ return;
+ }
+
+ EXPECT_EQ(child_pid, waitpid(child_pid, &status, 0));
+ EXPECT_EQ(1, WIFEXITED(status));
+ EXPECT_EQ(EXIT_SUCCESS, WEXITSTATUS(status));
+
+ ASSERT_EQ(0, rename(dir_s4d2, dir_s1d3))
+ {
+ TH_LOG("Failed to rename %s back to %s: %s", dir_s4d1, dir_s1d3,
+ strerror(errno));
+ }
+
+ /* Now checks that we can access it under l2. */
+ child_pid = fork();
+ ASSERT_LE(0, child_pid);
+ if (child_pid == 0) {
+ enforce_ruleset(_metadata, ruleset_fd_l2);
+ EXPECT_EQ(0, close(ruleset_fd_l2));
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY));
+ EXPECT_EQ(0, test_open(file1_s1d3, O_RDONLY));
+ _exit(_metadata->exit_code);
+ return;
+ }
+
+ EXPECT_EQ(child_pid, waitpid(child_pid, &status, 0));
+ EXPECT_EQ(1, WIFEXITED(status));
+ EXPECT_EQ(EXIT_SUCCESS, WEXITSTATUS(status));
+
+ /*
+ * Also test that we can rename via a disconnected path. We move the
+ * dir back to the disconnected place first, then we rename file1 to
+ * file2 through our dir fd.
+ */
+ ASSERT_EQ(0, rename(dir_s1d3, dir_s4d2))
+ {
+ TH_LOG("Failed to rename %s to %s: %s", dir_s1d3, dir_s4d2,
+ strerror(errno));
+ }
+ ASSERT_EQ(0,
+ renameat(bind_s1d3_fd, file1_name, bind_s1d3_fd, file2_name))
+ {
+ TH_LOG("Failed to rename %s to %s within disconnected %s: %s",
+ file1_name, file2_name, bind_dir_s1d3, strerror(errno));
+ }
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file2_name, O_RDONLY));
+ ASSERT_EQ(0, renameat(bind_s1d3_fd, file2_name, AT_FDCWD, file1_s2d2))
+ {
+ TH_LOG("Failed to rename %s to %s through disconnected %s: %s",
+ file2_name, file1_s2d2, bind_dir_s1d3, strerror(errno));
+ }
+ EXPECT_EQ(0, test_open(file1_s2d2, O_RDONLY));
+ EXPECT_EQ(0, test_open(file1_s1d2, O_RDONLY));
+
+ /* Move it back using the disconnected path as the target. */
+ ASSERT_EQ(0, renameat(AT_FDCWD, file1_s2d2, bind_s1d3_fd, file1_name))
+ {
+ TH_LOG("Failed to rename %s to %s through disconnected %s: %s",
+ file1_s1d2, file1_name, bind_dir_s1d3, strerror(errno));
+ }
+
+ /* Now make it connected again. */
+ ASSERT_EQ(0, rename(dir_s4d2, dir_s1d3))
+ {
+ TH_LOG("Failed to rename %s back to %s: %s", dir_s4d2, dir_s1d3,
+ strerror(errno));
+ }
+
+ /* Checks again that we can access it under l2. */
+ enforce_ruleset(_metadata, ruleset_fd_l2);
+ EXPECT_EQ(0, close(ruleset_fd_l2));
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY));
+ EXPECT_EQ(0, test_open(file1_s1d3, O_RDONLY));
+}
+
+/*
+ * Test that linkat(2) with disconnected paths works under Landlock. This
+ * test moves s1d3 to s4d1.
+ */
+TEST_F_FORK(layout1_bind, path_disconnected_link)
+{
+ /* Ruleset to be applied after renaming s1d3 to s4d1. */
+ const struct rule layer1[] = {
+ {
+ .path = dir_s4d1,
+ .access = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ },
+ {
+ .path = dir_s2d2,
+ .access = LANDLOCK_ACCESS_FS_REFER |
+ LANDLOCK_ACCESS_FS_READ_FILE |
+ LANDLOCK_ACCESS_FS_MAKE_REG |
+ LANDLOCK_ACCESS_FS_REMOVE_FILE,
+ },
+ {}
+ };
+ int ruleset_fd, bind_s1d3_fd;
+
+ /* Removes unneeded files created by layout1, otherwise it will EEXIST. */
+ ASSERT_EQ(0, unlink(file1_s1d2));
+ ASSERT_EQ(0, unlink(file2_s1d3));
+
+ bind_s1d3_fd = open(bind_dir_s1d3, O_PATH | O_CLOEXEC);
+ ASSERT_LE(0, bind_s1d3_fd);
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY));
+
+ /* Disconnects bind_s1d3_fd. */
+ ASSERT_EQ(0, rename(dir_s1d3, dir_s4d1))
+ {
+ TH_LOG("Failed to rename %s to %s: %s", dir_s1d3, dir_s4d1,
+ strerror(errno));
+ }
+
+ /* Need this later to test different parent link. */
+ ASSERT_EQ(0, mkdir(dir_s4d2, 0755))
+ {
+ TH_LOG("Failed to create %s: %s", dir_s4d2, strerror(errno));
+ }
+
+ ruleset_fd = create_ruleset(_metadata, ACCESS_ALL, layer1);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ EXPECT_EQ(0, close(ruleset_fd));
+
+ /* From disconnected to connected. */
+ ASSERT_EQ(0, linkat(bind_s1d3_fd, file1_name, AT_FDCWD, file1_s2d2, 0))
+ {
+ TH_LOG("Failed to link %s to %s via disconnected %s: %s",
+ file1_name, file1_s2d2, bind_dir_s1d3, strerror(errno));
+ }
+
+ /* Tests that we can access via the new link... */
+ EXPECT_EQ(0, test_open(file1_s2d2, O_RDONLY))
+ {
+ TH_LOG("Failed to open newly linked %s: %s", file1_s2d2,
+ strerror(errno));
+ }
+
+ /* ...as well as the old one. */
+ EXPECT_EQ(0, test_open(file1_s4d1, O_RDONLY))
+ {
+ TH_LOG("Failed to open original %s: %s", file1_s4d1,
+ strerror(errno));
+ }
+
+ /* From connected to disconnected. */
+ ASSERT_EQ(0, unlink(file1_s4d1));
+ ASSERT_EQ(0, linkat(AT_FDCWD, file1_s2d2, bind_s1d3_fd, file2_name, 0))
+ {
+ TH_LOG("Failed to link %s to %s via disconnected %s: %s",
+ file1_s2d2, file2_name, bind_dir_s1d3, strerror(errno));
+ }
+ EXPECT_EQ(0, test_open(file2_s4d1, O_RDONLY));
+ ASSERT_EQ(0, unlink(file1_s2d2));
+
+ /* From disconnected to disconnected (same parent). */
+ ASSERT_EQ(0,
+ linkat(bind_s1d3_fd, file2_name, bind_s1d3_fd, file1_name, 0))
+ {
+ TH_LOG("Failed to link %s to %s within disconnected %s: %s",
+ file2_name, file1_name, bind_dir_s1d3, strerror(errno));
+ }
+ EXPECT_EQ(0, test_open(file1_s4d1, O_RDONLY))
+ {
+ TH_LOG("Failed to open newly linked %s: %s", file1_s4d1,
+ strerror(errno));
+ }
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, file1_name, O_RDONLY))
+ {
+ TH_LOG("Failed to open %s through newly created link under disconnected path: %s",
+ file1_name, strerror(errno));
+ }
+ ASSERT_EQ(0, unlink(file2_s4d1));
+
+ /* From disconnected to disconnected (different parent). */
+ ASSERT_EQ(0,
+ linkat(bind_s1d3_fd, file1_name, bind_s1d3_fd, "s4d2/f1", 0))
+ {
+ TH_LOG("Failed to link %s to %s within disconnected %s: %s",
+ file1_name, "s4d2/f1", bind_dir_s1d3, strerror(errno));
+ }
+ EXPECT_EQ(0, test_open(file1_s4d2, O_RDONLY))
+ {
+ TH_LOG("Failed to open %s after link: %s", file1_s4d2,
+ strerror(errno));
+ }
+ EXPECT_EQ(0, test_open_rel(bind_s1d3_fd, "s4d2/f1", O_RDONLY))
+ {
+ TH_LOG("Failed to open %s through disconnected path after link: %s",
+ "s4d2/f1", strerror(errno));
+ }
+}
+
#define LOWER_BASE TMP_DIR "/lower"
#define LOWER_DATA LOWER_BASE "/data"
static const char lower_fl1[] = LOWER_DATA "/fl1";
--
2.51.0
^ permalink raw reply related
* [PATCH v4 1/4] landlock: Fix handling of disconnected directories
From: Mickaël Salaün @ 2025-11-26 19:11 UTC (permalink / raw)
To: Günther Noack, Tingmao Wang
Cc: Mickaël Salaün, Al Viro, Ben Scarlato,
Christian Brauner, Jann Horn, Jeff Xu, Justin Suess,
Mikhail Ivanov, Paul Moore, Song Liu, linux-fsdevel,
linux-security-module
In-Reply-To: <20251126191159.3530363-1-mic@digikod.net>
Disconnected files or directories can appear when they are visible and
opened from a bind mount, but have been renamed or moved from the source
of the bind mount in a way that makes them inaccessible from the mount
point (i.e. out of scope).
Previously, access rights tied to files or directories opened through a
disconnected directory were collected by walking the related hierarchy
down to the root of the filesystem, without taking into account the
mount point because it couldn't be found. This could lead to
inconsistent access results, potential access right widening, and
hard-to-debug renames, especially since such paths cannot be printed.
For a sandboxed task to create a disconnected directory, it needs to
have write access (i.e. FS_MAKE_REG, FS_REMOVE_FILE, and FS_REFER) to
the underlying source of the bind mount, and read access to the related
mount point. Because a sandboxed task cannot acquire more access
rights than those defined by its Landlock domain, this could lead to
inconsistent access rights due to missing permissions that should be
inherited from the mount point hierarchy, while inheriting permissions
from the filesystem hierarchy hidden by this mount point instead.
Landlock now handles files and directories opened from disconnected
directories by taking into account the filesystem hierarchy when the
mount point is not found in the hierarchy walk, and also always taking
into account the mount point from which these disconnected directories
were opened. This ensures that a rename is not allowed if it would
widen access rights [1].
The rationale is that, even if disconnected hierarchies might not be
visible or accessible to a sandboxed task, relying on the collected
access rights from them improves the guarantee that access rights will
not be widened during a rename because of the access right comparison
between the source and the destination (see LANDLOCK_ACCESS_FS_REFER).
It may look like this would grant more access on disconnected files and
directories, but the security policies are always enforced for all the
evaluated hierarchies. This new behavior should be less surprising to
users and safer from an access control perspective.
Remove a wrong WARN_ON_ONCE() canary in collect_domain_accesses() and
fix the related comment.
Because opened files have their access rights stored in the related file
security properties, there is no impact for disconnected or unlinked
files.
Cc: Christian Brauner <brauner@kernel.org>
Cc: Günther Noack <gnoack@google.com>
Cc: Song Liu <song@kernel.org>
Reported-by: Tingmao Wang <m@maowtm.org>
Closes: https://lore.kernel.org/r/027d5190-b37a-40a8-84e9-4ccbc352bcdf@maowtm.org
Closes: https://lore.kernel.org/r/09b24128f86973a6022e6aa8338945fcfb9a33e4.1749925391.git.m@maowtm.org
Fixes: b91c3e4ea756 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER")
Fixes: cb2c7d1a1776 ("landlock: Support filesystem access-control")
Link: https://lore.kernel.org/r/b0f46246-f2c5-42ca-93ce-0d629702a987@maowtm.org [1]
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v3:
- Simplify the approach by checking both the filesystem hierarchy and
the mount point hierarchy for disconnected directories. Remove the
related snapshot mechanism. See [1].
- Update erratum.
- Do not expose path_connected() anymore, and remove related Acked-by.
Changes since v2:
- Fix domain check mode reset, spotted by Tingmao. Replace a
conditional branch (when all accesses are granted) by only looking for
a rule when needed. This simplifies code and remove a call to
path_connected(). Add a new is_dom_check_bkp to safely handle race
conditions and move initial backup for layer_masks_parent1.
- Fix layer masks parent backup initialization, spotted by Tingmao.
- Add more comments, suggested by Tingmao.
- Reformat erratum.
Changes since v1:
- Remove useless else branch in is_access_to_paths_allowed().
- Update commit message and squash "landlock: Remove warning in
collect_domain_accesses()":
https://lore.kernel.org/r/20250618134734.1673254-1-mic@digikod.net
- Remove "extern" for path_connected() in fs.h, requested by Christian.
- Add Acked-by Christian.
- Fix docstring and improve doc for collect_domain_accesses().
- Remove path_connected() check for the real root.
- Fix allowed_parent* resets to be consistent with their initial values
infered from the evaluated domain.
- Add Landlock erratum.
Cc: Christian Brauner <brauner@kernel.org>
Cc: Günther Noack <gnoack@google.com>
Cc: Song Liu <song@kernel.org>
Reported-by: Tingmao Wang <m@maowtm.org>
Closes: https://lore.kernel.org/r/027d5190-b37a-40a8-84e9-4ccbc352bcdf@maowtm.org
Closes: https://lore.kernel.org/r/09b24128f86973a6022e6aa8338945fcfb9a33e4.1749925391.git.m@maowtm.org
Fixes: b91c3e4ea756 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER")
Fixes: cb2c7d1a1776 ("landlock: Support filesystem access-control")
Link: https://lore.kernel.org/r/b0f46246-f2c5-42ca-93ce-0d629702a987@maowtm.org [1]
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v3:
- Simplify the approach by checking both the filesystem hierarchy and
the mount point hierarchy for disconnected directories. Remove the
related snapshot mechanism. See [1].
- Do not expose path_connected() anymore.
Changes since v2:
- Fix domain check mode reset, spotted by Tingmao. Replace a
conditional branch (when all accesses are granted) by only looking for
a rule when needed. This simplifies code and remove a call to
path_connected(). Add a new is_dom_check_bkp to safely handle race
conditions and move initial backup for layer_masks_parent1.
- Fix layer masks parent backup initialization, spotted by Tingmao.
- Add more comments, suggested by Tingmao.
- Reformat erratum.
Changes since v1:
- Remove useless else branch in is_access_to_paths_allowed().
- Update commit message and squash "landlock: Remove warning in
collect_domain_accesses()":
https://lore.kernel.org/r/20250618134734.1673254-1-mic@digikod.net
- Remove "extern" for path_connected() in fs.h, requested by Christian.
- Add Acked-by Christian.
- Fix docstring and improve doc for collect_domain_accesses().
- Remove path_connected() check for the real root.
- Fix allowed_parent* resets to be consistent with their initial values
infered from the evaluated domain.
- Add Landlock erratum.
---
security/landlock/errata/abi-1.h | 16 +++++++++++++
security/landlock/fs.c | 40 ++++++++++++++++++++++----------
2 files changed, 44 insertions(+), 12 deletions(-)
create mode 100644 security/landlock/errata/abi-1.h
diff --git a/security/landlock/errata/abi-1.h b/security/landlock/errata/abi-1.h
new file mode 100644
index 000000000000..e8a2bff2e5b6
--- /dev/null
+++ b/security/landlock/errata/abi-1.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+/**
+ * DOC: erratum_3
+ *
+ * Erratum 3: Disconnected directory handling
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This fix addresses an issue with disconnected directories that occur when a
+ * directory is moved outside the scope of a bind mount. The change ensures
+ * that evaluated access rights include both those from the disconnected file
+ * hierarchy down to its filesystem root and those from the related mount point
+ * hierarchy. This prevents access right widening through rename or link
+ * actions.
+ */
+LANDLOCK_ERRATUM(3)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index d9c12b993fa7..26d5c274a4c9 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -909,21 +909,31 @@ static bool is_access_to_paths_allowed(
break;
}
}
+
if (unlikely(IS_ROOT(walker_path.dentry))) {
- /*
- * Stops at disconnected root directories. Only allows
- * access to internal filesystems (e.g. nsfs, which is
- * reachable through /proc/<pid>/ns/<namespace>).
- */
- if (walker_path.mnt->mnt_flags & MNT_INTERNAL) {
+ if (likely(walker_path.mnt->mnt_flags & MNT_INTERNAL)) {
+ /*
+ * Stops and allows access when reaching disconnected root
+ * directories that are part of internal filesystems (e.g. nsfs,
+ * which is reachable through /proc/<pid>/ns/<namespace>).
+ */
allowed_parent1 = true;
allowed_parent2 = true;
+ break;
}
- break;
+
+ /*
+ * We reached a disconnected root directory from a bind mount.
+ * Let's continue the walk with the current mount root.
+ */
+ dput(walker_path.dentry);
+ walker_path.dentry = walker_path.mnt->mnt_root;
+ dget(walker_path.dentry);
+ } else {
+ parent_dentry = dget_parent(walker_path.dentry);
+ dput(walker_path.dentry);
+ walker_path.dentry = parent_dentry;
}
- parent_dentry = dget_parent(walker_path.dentry);
- dput(walker_path.dentry);
- walker_path.dentry = parent_dentry;
}
path_put(&walker_path);
@@ -1021,6 +1031,9 @@ static access_mask_t maybe_remove(const struct dentry *const dentry)
* file. While walking from @dir to @mnt_root, we record all the domain's
* allowed accesses in @layer_masks_dom.
*
+ * Because of disconnected directories, this walk may not reach @mnt_dir. In
+ * this case, the walk will continue to @mnt_dir after this call.
+ *
* This is similar to is_access_to_paths_allowed() but much simpler because it
* only handles walking on the same mount point and only checks one set of
* accesses.
@@ -1062,8 +1075,11 @@ static bool collect_domain_accesses(
break;
}
- /* We should not reach a root other than @mnt_root. */
- if (dir == mnt_root || WARN_ON_ONCE(IS_ROOT(dir)))
+ /*
+ * Stops at the mount point or the filesystem root for a disconnected
+ * directory.
+ */
+ if (dir == mnt_root || unlikely(IS_ROOT(dir)))
break;
parent_dentry = dget_parent(dir);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v15 3/4] lsm: count the LSMs enabled at compile time
From: Paul Moore @ 2025-11-26 19:02 UTC (permalink / raw)
To: Andy Shevchenko, KP Singh
Cc: linux-security-module, bpf, ast, casey, andrii, keescook, daniel,
renauld, revest, song, linux, Kui-Feng Lee, John Johansen
In-Reply-To: <aSc1aAdOeSuuoKTH@black.igk.intel.com>
On November 26, 2025 12:14:21 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> On Fri, Aug 16, 2024 at 05:43:06PM +0200, KP Singh wrote:
>> These macros are a clever trick to determine a count of the number of
>> LSMs that are enabled in the config to ascertain the maximum number of
>> static calls that need to be configured per LSM hook.
>>
>> Without this one would need to generate static calls for the total
>> number of LSMs in the kernel (even if they are not compiled) times the
>> number of LSM hooks which ends up being quite wasteful.
>
> ...
>
>> -/* This counts to 12. Any more, it will return 13th argument. */
>> -#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11,
>> _12, _n, X...) _n
>> -#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4,
>> 3, 2, 1, 0)
>> +/* This counts to 15. Any more, it will return 16th argument. */
>> +#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11,
>> _12, _13, _14, _15, _n, X...) _n
>> +#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 15, 14, 13, 12, 11, 10, 9, 8,
>> 7, 6, 5, 4, 3, 2, 1, 0)
>
> You forgot to update _12 in the upper comment.
Do you plan to send a patch to fix this?
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 0/2] apparmor unaligned memory fixes
From: John Paul Adrian Glaubitz @ 2025-11-26 17:26 UTC (permalink / raw)
To: Helge Deller, Helge Deller, John Johansen, david laight
Cc: linux-kernel, apparmor, linux-security-module, linux-parisc
In-Reply-To: <0d03ddb9-d01f-435e-a57c-fbea32844b66@gmx.de>
Hi Helge,
On Wed, 2025-11-26 at 17:58 +0100, Helge Deller wrote:
> On 11/26/25 17:16, John Paul Adrian Glaubitz wrote:
> > Hi Helge,
> >
> > On Wed, 2025-11-26 at 12:31 +0100, Helge Deller wrote:
> > > Like this (untested!) patch:
> > >
> > > [PATCH] apparmor: Optimize table creation from possibly unaligned memory
> > >
> > > Source blob may come from userspace and might be unaligned.
> > > Try to optize the copying process by avoiding unaligned memory accesses.
> > >
> > > Signed-off-by: Helge Deller <deller@gmx.de>
> > >
> > > diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
> > > index 1fbe82f5021b..225df6495c84 100644
> > > --- a/security/apparmor/include/match.h
> > > +++ b/security/apparmor/include/match.h
> > > @@ -111,9 +111,14 @@ struct aa_dfa {
> > > typeof(LEN) __i; \
> > > TTYPE *__t = (TTYPE *) TABLE; \
> > > BTYPE *__b = (BTYPE *) BLOB; \
> > > - for (__i = 0; __i < LEN; __i++) { \
> > > - __t[__i] = NTOHX(__b[__i]); \
> > > - } \
> > > + BUILD_BUG_ON(sizeof(TTYPE) != sizeof(BTYPE)); \
> > > + /* copy to naturally aligned table address */ \
> > > + memcpy(__t, __b, (LEN) * sizeof(BTYPE)); \
> > > + /* convert from big-endian if necessary */ \
> > > + if (!IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)) \
> > > + for (__i = 0; __i < LEN; __i++, __t++) { \
> > > + *__t = NTOHX(*__t); \
> > > + } \
> > > } while (0)
> > >
> > > static inline size_t table_size(size_t len, size_t el_size)
> >
> > So, is this patch supposed to replace all the other proposed patches?
>
> It just replaces the patch from John.
> But please test the v2 patch I sent instead...
OK, so we need your previous patch that I already tested and your v2 of John's patch?
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply
* Re: [PATCH v15 3/4] lsm: count the LSMs enabled at compile time
From: Andy Shevchenko @ 2025-11-26 17:14 UTC (permalink / raw)
To: KP Singh
Cc: linux-security-module, bpf, ast, paul, casey, andrii, keescook,
daniel, renauld, revest, song, linux, Kui-Feng Lee, John Johansen
In-Reply-To: <20240816154307.3031838-4-kpsingh@kernel.org>
On Fri, Aug 16, 2024 at 05:43:06PM +0200, KP Singh wrote:
> These macros are a clever trick to determine a count of the number of
> LSMs that are enabled in the config to ascertain the maximum number of
> static calls that need to be configured per LSM hook.
>
> Without this one would need to generate static calls for the total
> number of LSMs in the kernel (even if they are not compiled) times the
> number of LSM hooks which ends up being quite wasteful.
...
> -/* This counts to 12. Any more, it will return 13th argument. */
> -#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
> -#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
> +/* This counts to 15. Any more, it will return 16th argument. */
> +#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, _15, _n, X...) _n
> +#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
You forgot to update _12 in the upper comment.
--
With Best Regards,
Andy Shevchenko
^ 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