Add is_memfd_file() function to reliably detect memfd files by checking for "memfd:" prefix in dentry names on shmem-backed files. This distinguishes true memfd files from regular shmem files.

Move domain_is_scoped() to domain.c for reuse across subsystems. Add comprehensive kunit tests for memfd detection edge cases.

Signed-off-by: Abhinav Saxena xandfury@gmail.com --- security/landlock/domain.c | 67 +++++++++++++++ security/landlock/domain.h | 4 + security/landlock/fs.c | 210 +++++++++++++++++++++++++++++++++++++++++++++ security/landlock/task.c | 67 --------------- 4 files changed, 281 insertions(+), 67 deletions(-)

diff --git a/security/landlock/domain.c b/security/landlock/domain.c index a647b68e8d06..993c299ca263 100644 --- a/security/landlock/domain.c +++ b/security/landlock/domain.c @@ -262,3 +262,70 @@ kunit_test_suite(test_suite); #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */

#endif /* CONFIG_AUDIT */ + +/** + * domain_is_scoped - Checks if the client domain is scoped in the same + * domain as the server. + * + * @client: IPC sender domain. + * @server: IPC receiver domain. + * @scope: The scope restriction criteria. + * + * Returns: True if the @client domain is scoped to access the @server, + * unless the @server is also scoped in the same domain as @client. + */ +bool domain_is_scoped(const struct landlock_ruleset *const client, + const struct landlock_ruleset *const server, + access_mask_t scope) +{ + int client_layer, server_layer; + const struct landlock_hierarchy *client_walker, *server_walker; + + /* Quick return if client has no domain */ + if (WARN_ON_ONCE(!client)) + return false; + + client_layer = client->num_layers - 1; + client_walker = client->hierarchy; + /* + * client_layer must be a signed integer with greater capacity + * than client->num_layers to ensure the following loop stops. + */ + BUILD_BUG_ON(sizeof(client_layer) > sizeof(client->num_layers)); + + server_layer = server ? (server->num_layers - 1) : -1; + server_walker = server ? server->hierarchy : NULL; + + /* + * Walks client's parent domains down to the same hierarchy level + * as the server's domain, and checks that none of these client's + * parent domains are scoped. + */ + for (; client_layer > server_layer; client_layer--) { + if (landlock_get_scope_mask(client, client_layer) & scope) + return true; + + client_walker = client_walker->parent; + } + /* + * Walks server's parent domains down to the same hierarchy level as + * the client's domain. + */ + for (; server_layer > client_layer; server_layer--) + server_walker = server_walker->parent; + + for (; client_layer >= 0; client_layer--) { + if (landlock_get_scope_mask(client, client_layer) & scope) { + /* + * Client and server are at the same level in the + * hierarchy. If the client is scoped, the request is + * only allowed if this domain is also a server's + * ancestor. + */ + return server_walker != client_walker; + } + client_walker = client_walker->parent; + server_walker = server_walker->parent; + } + return false; +} diff --git a/security/landlock/domain.h b/security/landlock/domain.h index 7fb70b25f85a..21a9eea644bd 100644 --- a/security/landlock/domain.h +++ b/security/landlock/domain.h @@ -171,4 +171,8 @@ static inline void landlock_put_hierarchy(struct landlock_hierarchy *hierarchy) } }

+bool domain_is_scoped(const struct landlock_ruleset *const client, + const struct landlock_ruleset *const server, + access_mask_t scope); + #endif /* _SECURITY_LANDLOCK_DOMAIN_H */ diff --git a/security/landlock/fs.c b/security/landlock/fs.c index da862fda329b..d86d21034f4c 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -18,6 +18,7 @@ #include <linux/dcache.h> #include <linux/err.h> #include <linux/falloc.h> +#include <linux/file.h> #include <linux/fs.h> #include <linux/init.h> #include <linux/kernel.h> @@ -25,19 +26,26 @@ #include <linux/list.h> #include <linux/lsm_audit.h> #include <linux/lsm_hooks.h> +#include <linux/memfd.h> +#include <linux/mm.h> #include <linux/mount.h> #include <linux/namei.h> #include <linux/path.h> #include <linux/pid.h> #include <linux/rcupdate.h> #include <linux/sched/signal.h> +#include <linux/shmem_fs.h> #include <linux/spinlock.h> #include <linux/stat.h> +#include <linux/syscalls.h> +#include <linux/mman.h> #include <linux/types.h> #include <linux/wait_bit.h> #include <linux/workqueue.h> +#include <uapi/linux/fcntl.h> #include <uapi/linux/fiemap.h> #include <uapi/linux/landlock.h> +#include <linux/binfmts.h>

#include "access.h" #include "audit.h" @@ -1667,6 +1675,201 @@ get_required_file_open_access(const struct file *const file) return access; }

+/** + * is_memfd_file - Check if file was created via memfd_create() + * @file: File to check + * + * Returns true if @file was created via memfd_create(), false otherwise. + * + * memfd files are shmem-backed files with "memfd:" prefix in their dentry name. + * This is the definitive way to distinguish memfd files from regular shmem + * files. + */ +static bool is_memfd_file(struct file *file) +{ + const struct dentry *dentry; + const unsigned char *name; + size_t name_len; + + /* Fast path: basic validation */ + if (unlikely(!file)) + return false; + + /* Must be shmem-backed first - this is the cheapest definitive check */ + if (!shmem_file(file)) + return false; + +#ifdef CONFIG_MEMFD_CREATE + + /* Validate dentry and get name info */ + dentry = file->f_path.dentry; + if (unlikely(!dentry)) + return false; + + name_len = dentry->d_name.len; + name = dentry->d_name.name; + + /* memfd files always have "memfd:" prefix (6 characters) */ + if (name_len < 6 || unlikely(!name)) + return false; + + /* Check for exact "memfd:" prefix */ + return memcmp(name, "memfd:", 6) == 0; +#else + return false; +#endif +} + +#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST + +/* + * Test that is_memfd_file() returns false for NULL input + */ +static void test_memfd_null_file(struct kunit *test) +{ + KUNIT_EXPECT_FALSE(test, is_memfd_file(NULL)); +} + +/* + * Test that regular shmem files are correctly distinguished from memfd files + */ +static void test_shmem_vs_memfd_detection(struct kunit *test) +{ + struct file *shmem_files[4]; + static const char *const names[] = { + "regular_shmem", "", "large_shmem", + "memfd_fake" /* This should NOT be detected as memfd */ + }; + static const size_t sizes[] = { 4096, 4096, 1024 * 1024, 8192 }; + static const unsigned long vm_flags[] = { VM_NORESERVE, 0, + VM_NORESERVE | VM_ACCOUNT, + VM_NORESERVE }; + int i; + + for (i = 0; i < ARRAY_SIZE(shmem_files); i++) { + shmem_files[i] = + shmem_file_setup(names[i], sizes[i], vm_flags[i]); + KUNIT_ASSERT_FALSE(test, IS_ERR(shmem_files[i])); + + /* All should be shmem-backed but NOT memfd */ + KUNIT_EXPECT_TRUE(test, shmem_file(shmem_files[i])); + KUNIT_EXPECT_FALSE(test, is_memfd_file(shmem_files[i])); + + /* Verify dentry name doesn't have memfd: prefix */ + if (shmem_files[i]->f_path.dentry && + shmem_files[i]->f_path.dentry->d_name.name) { + const char *dentry_name = + shmem_files[i]->f_path.dentry->d_name.name; + KUNIT_EXPECT_TRUE(test, + strlen(dentry_name) < 6 || + memcmp(dentry_name, + "memfd:", 6) != 0); + } + + fput(shmem_files[i]); + } +} + +/* + * Test edge cases and boundary conditions + */ +static void test_memfd_detection_edge_cases(struct kunit *test) +{ + struct file *edge_case_files[3]; + static const char *const tricky_names[] = { + "memf", /* Too short for memfd: prefix */ + "memfd", /* Still too short */ + "memfdx:test" /* Wrong prefix */ + }; + int i; + + for (i = 0; i < ARRAY_SIZE(edge_case_files); i++) { + edge_case_files[i] = + shmem_file_setup(tricky_names[i], 4096, VM_NORESERVE); + KUNIT_ASSERT_FALSE(test, IS_ERR(edge_case_files[i])); + + /* All should be shmem but NOT memfd due to incorrect naming */ + KUNIT_EXPECT_TRUE(test, shmem_file(edge_case_files[i])); + KUNIT_EXPECT_FALSE(test, is_memfd_file(edge_case_files[i])); + + fput(edge_case_files[i]); + } +} + +/* + * Test detection consistency across multiple calls + */ +static void test_memfd_detection_consistency(struct kunit *test) +{ + struct file *file; + bool initial_result, subsequent_result; + int iteration; + + file = shmem_file_setup("consistency_test", 4096, VM_NORESERVE); + KUNIT_ASSERT_FALSE(test, IS_ERR(file)); + + /* Get initial detection result */ + initial_result = is_memfd_file(file); + + /* Multiple calls should return identical results */ + for (iteration = 0; iteration < 10; iteration++) { + subsequent_result = is_memfd_file(file); + KUNIT_EXPECT_EQ(test, initial_result, subsequent_result); + } + + fput(file); +} + +#ifdef CONFIG_MEMFD_CREATE + +/* + * Test performance characteristics (ensure function is reasonably fast) + */ +static void test_memfd_detection_performance(struct kunit *test) +{ + struct file *files[5]; + static const char *const names[] = { "perf1", "perf2", "perf3", "perf4", + "perf5" }; + ktime_t start_time, end_time; + s64 duration_ns; + int i, j; + const int iterations = 1000; + + /* Set up test files */ + for (i = 0; i < ARRAY_SIZE(files); i++) { + files[i] = shmem_file_setup(names[i], 4096, VM_NORESERVE); + KUNIT_ASSERT_FALSE(test, IS_ERR(files[i])); + } + + /* Time the detection function */ + start_time = ktime_get(); + + for (i = 0; i < iterations; i++) { + for (j = 0; j < ARRAY_SIZE(files); j++) { + bool result = is_memfd_file(files[j]); + (void)result; /* Suppress unused variable warning */ + } + } + + end_time = ktime_get(); + duration_ns = ktime_to_ns(ktime_sub(end_time, start_time)); + + /* Cleanup */ + for (i = 0; i < ARRAY_SIZE(files); i++) + fput(files[i]); + + /* Performance check: should complete in reasonable time */ + /* This is a sanity check - actual limits depend on system */ + KUNIT_EXPECT_LT(test, duration_ns, + 1000000000LL); /* Less than 1 second */ + + pr_info("memfd detection performance: %lld ns for %d iterations on %zu files\n", + duration_ns, iterations, ARRAY_SIZE(files)); +} + +#endif /* CONFIG_MEMFD_CREATE */ +#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */ + static int hook_file_alloc_security(struct file *const file) { /* @@ -1949,6 +2152,13 @@ static struct kunit_case test_cases[] = { KUNIT_CASE(test_is_eacces_with_none), KUNIT_CASE(test_is_eacces_with_refer), KUNIT_CASE(test_is_eacces_with_write), + KUNIT_CASE(test_memfd_null_file), + KUNIT_CASE(test_shmem_vs_memfd_detection), + KUNIT_CASE(test_memfd_detection_edge_cases), + KUNIT_CASE(test_memfd_detection_consistency), +#ifdef CONFIG_MEMFD_CREATE + KUNIT_CASE(test_memfd_detection_performance), +#endif {} }; /* clang-format on */ diff --git a/security/landlock/task.c b/security/landlock/task.c index 2385017418ca..559a96a97ab6 100644 --- a/security/landlock/task.c +++ b/security/landlock/task.c @@ -165,73 +165,6 @@ static int hook_ptrace_traceme(struct task_struct *const parent) return err; }

-/** - * domain_is_scoped - Checks if the client domain is scoped in the same - * domain as the server. - * - * @client: IPC sender domain. - * @server: IPC receiver domain. - * @scope: The scope restriction criteria. - * - * Returns: True if the @client domain is scoped to access the @server, - * unless the @server is also scoped in the same domain as @client. - */ -static bool domain_is_scoped(const struct landlock_ruleset *const client, - const struct landlock_ruleset *const server, - access_mask_t scope) -{ - int client_layer, server_layer; - const struct landlock_hierarchy *client_walker, *server_walker; - - /* Quick return if client has no domain */ - if (WARN_ON_ONCE(!client)) - return false; - - client_layer = client->num_layers - 1; - client_walker = client->hierarchy; - /* - * client_layer must be a signed integer with greater capacity - * than client->num_layers to ensure the following loop stops. - */ - BUILD_BUG_ON(sizeof(client_layer) > sizeof(client->num_layers)); - - server_layer = server ? (server->num_layers - 1) : -1; - server_walker = server ? server->hierarchy : NULL; - - /* - * Walks client's parent domains down to the same hierarchy level - * as the server's domain, and checks that none of these client's - * parent domains are scoped. - */ - for (; client_layer > server_layer; client_layer--) { - if (landlock_get_scope_mask(client, client_layer) & scope) - return true; - - client_walker = client_walker->parent; - } - /* - * Walks server's parent domains down to the same hierarchy level as - * the client's domain. - */ - for (; server_layer > client_layer; server_layer--) - server_walker = server_walker->parent; - - for (; client_layer >= 0; client_layer--) { - if (landlock_get_scope_mask(client, client_layer) & scope) { - /* - * Client and server are at the same level in the - * hierarchy. If the client is scoped, the request is - * only allowed if this domain is also a server's - * ancestor. - */ - return server_walker != client_walker; - } - client_walker = client_walker->parent; - server_walker = server_walker->parent; - } - return false; -} - static bool sock_is_scoped(struct sock *const other, const struct landlock_ruleset *const domain) {

Implement LSM hooks to enforce memfd execution restrictions:

- hook_mmap_file: Prevent executable mapping of memfd files - hook_file_mprotect: Block mprotect() adding PROT_EXEC to memfd mappings - hook_bprm_creds_for_exec: Prevent direct execution via execve() family - hook_file_alloc_security: Initialize memfd files with proper access masks

All hooks use domain hierarchy checking to enforce scoped restrictions with proper audit logging. This prevents multiple attack vectors: - Direct mmap(PROT_EXEC) on memfd - Two-stage mmap(PROT_READ) + mprotect(PROT_EXEC) bypass - execve("/proc/self/fd/N") anonymous execution

Implement memfd execution access control in check_memfd_execute_access() using hierarchy-aware domain checking

Signed-off-by: Abhinav Saxena xandfury@gmail.com --- security/landlock/cred.c | 14 ---- security/landlock/fs.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 194 insertions(+), 15 deletions(-)

diff --git a/security/landlock/cred.c b/security/landlock/cred.c index 0cb3edde4d18..356dad0b7e9b 100644 --- a/security/landlock/cred.c +++ b/security/landlock/cred.c @@ -43,25 +43,11 @@ static void hook_cred_free(struct cred *const cred) landlock_put_ruleset_deferred(dom); }

-#ifdef CONFIG_AUDIT - -static int hook_bprm_creds_for_exec(struct linux_binprm *const bprm) -{ - /* Resets for each execution. */ - landlock_cred(bprm->cred)->domain_exec = 0; - return 0; -} - -#endif /* CONFIG_AUDIT */ - static struct security_hook_list landlock_hooks[] __ro_after_init = { LSM_HOOK_INIT(cred_prepare, hook_cred_prepare), LSM_HOOK_INIT(cred_transfer, hook_cred_transfer), LSM_HOOK_INIT(cred_free, hook_cred_free),

-#ifdef CONFIG_AUDIT - LSM_HOOK_INIT(bprm_creds_for_exec, hook_bprm_creds_for_exec), -#endif /* CONFIG_AUDIT */ };

__init void landlock_add_cred_hooks(void) diff --git a/security/landlock/fs.c b/security/landlock/fs.c index d86d21034f4c..e8b58f2fd87e 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -1880,7 +1880,24 @@ static int hook_file_alloc_security(struct file *const file) * without going through the file_open hook, for example when using * memfd_create(2). */ - landlock_file(file)->allowed_access = LANDLOCK_MASK_ACCESS_FS; + access_mask_t allowed_access = LANDLOCK_MASK_ACCESS_FS; + const struct landlock_cred_security *subject; + size_t layer; + static const struct access_masks memfd_scope = { + .scope = LANDLOCK_SCOPE_MEMFD_EXEC, + }; + + /* allow everything by default */ + landlock_file(file)->allowed_access = allowed_access; + + subject = landlock_get_applicable_subject(current_cred(), memfd_scope, + &layer); + if (subject && is_memfd_file(file)) { + /* Creator domain restricts memfd execution */ + allowed_access &= ~LANDLOCK_ACCESS_FS_EXECUTE; + landlock_file(file)->allowed_access = allowed_access; + /* Store creator and audit... */ + } return 0; }

@@ -2107,6 +2124,178 @@ static void hook_file_free_security(struct file *file) landlock_put_ruleset_deferred(landlock_file(file)->fown_subject.domain); }

+static bool +check_memfd_execute_access(const struct file *file, + const struct landlock_cred_security **subject, + size_t *layer_plus_one) +{ + const struct landlock_ruleset *executor_domain, *creator_domain; + const struct landlock_cred_security *creator_subject; + static const struct access_masks memfd_scope = { + .scope = LANDLOCK_SCOPE_MEMFD_EXEC, + }; + size_t creator_layer_plus_one = 0; + bool executor_scoped, creator_scoped, is_scoped; + + *subject = NULL; + *layer_plus_one = 0; + + /* Check scoping status for both executor and creator */ + *subject = landlock_get_applicable_subject(current_cred(), memfd_scope, + layer_plus_one); + creator_subject = landlock_get_applicable_subject( + file->f_cred, memfd_scope, &creator_layer_plus_one); + + executor_scoped = (*subject != NULL); + creator_scoped = (creator_subject != NULL); + + if (!creator_scoped) + return true; /* No scoping enabled, allow execution */ + + /* Get domains for comparison */ + executor_domain = executor_scoped ? (*subject)->domain : NULL; + creator_domain = creator_scoped ? creator_subject->domain : + landlock_cred(file->f_cred)->domain; + + pr_info("MEMFD_DEBUG: executor_domain=%p, creator_domain=%p\n", + executor_domain, creator_domain); + + /* + * Same-domain: deny to prevent read-to-execute bypass + * This prevents processes from bypassing execute restrictions + * by creating memfd in the same domain + */ + if (executor_domain == creator_domain) + return false; + + /* + * Cross-domain: use domain hierarchy checks to see if executor is + * scoped from creator domain_is_scoped() returns true when access + * should be DENIED + */ + if (executor_scoped || creator_scoped) { + is_scoped = domain_is_scoped(executor_domain, creator_domain, + LANDLOCK_SCOPE_MEMFD_EXEC); + pr_info("MEMFD_DEBUG: Cross-domain: is_scoped=%d, returning=%d\n", + is_scoped, !is_scoped); + /* Return true (allow) when NOT scoped, false (deny) when scoped */ + return !is_scoped; + } + + return true; +} + +static int hook_mmap_file(struct file *file, unsigned long reqprot, + unsigned long prot, unsigned long flags) +{ + const struct landlock_cred_security *subject; + size_t layer_plus_one; + + /* Only check executable mappings */ + if (!(prot & PROT_EXEC)) + return 0; + + /* Only restrict memfd files */ + if (!is_memfd_file(file)) + return 0; + + /* Check if memfd execution is allowed */ + if (check_memfd_execute_access(file, &subject, &layer_plus_one)) + return 0; + + /* Log denial for audit */ + if (subject) { + landlock_log_denial(subject, &(struct landlock_request) { + .type = LANDLOCK_REQUEST_SCOPE_MEMFD_EXEC, + .audit = { + .type = LSM_AUDIT_DATA_ANONINODE, + .u.file = file, + }, + .layer_plus_one = layer_plus_one, + }); + } + + return -EACCES; +} + +static int hook_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot, + unsigned long prot) +{ + const struct landlock_cred_security *subject; + size_t layer_plus_one; + + /* Only check when adding execute permission */ + if (!(prot & PROT_EXEC)) + return 0; + + /* Must have a file backing the VMA */ + if (!vma || !vma->vm_file) + return 0; + + /* Only restrict memfd files */ + if (!is_memfd_file(vma->vm_file)) + return 0; + + /* Check if memfd execution is allowed */ + if (check_memfd_execute_access(vma->vm_file, &subject, &layer_plus_one)) + return 0; + + /* Log denial for audit */ + if (subject) { + landlock_log_denial(subject, &(struct landlock_request) { + .type = LANDLOCK_REQUEST_SCOPE_MEMFD_EXEC, + .audit = { + .type = LSM_AUDIT_DATA_ANONINODE, + .u.file = vma->vm_file, + }, + .layer_plus_one = layer_plus_one, + }); + } + + return -EACCES; +} + +static int hook_bprm_creds_for_exec(struct linux_binprm *bprm) +{ +#ifdef CONFIG_AUDIT + /* Resets for each execution. */ + landlock_cred(bprm->cred)->domain_exec = 0; +#endif /* CONFIG_AUDIT */ + + const struct landlock_cred_security *subject; + size_t layer_plus_one; + struct file *file; + + if (!bprm) + return 0; + + file = bprm->file; + if (!file) + return 0; + + /* Only restrict memfd files */ + if (!is_memfd_file(file)) + return 0; + + /* Check if memfd execution is allowed */ + if (check_memfd_execute_access(file, &subject, &layer_plus_one)) + return 0; + + /* Log denial for audit */ + if (subject) { + landlock_log_denial(subject, &(struct landlock_request) { + .type = LANDLOCK_REQUEST_SCOPE_MEMFD_EXEC, + .audit = { + .type = LSM_AUDIT_DATA_ANONINODE, + .u.file = file, + }, + .layer_plus_one = layer_plus_one, + }); + } + + return -EACCES; /* maybe we should return EPERM? */ +} + static struct security_hook_list landlock_hooks[] __ro_after_init = { LSM_HOOK_INIT(inode_free_security_rcu, hook_inode_free_security_rcu),

@@ -2133,6 +2322,10 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = { LSM_HOOK_INIT(file_ioctl_compat, hook_file_ioctl_compat), LSM_HOOK_INIT(file_set_fowner, hook_file_set_fowner), LSM_HOOK_INIT(file_free_security, hook_file_free_security), + + LSM_HOOK_INIT(mmap_file, hook_mmap_file), + LSM_HOOK_INIT(file_mprotect, hook_file_mprotect), + LSM_HOOK_INIT(bprm_creds_for_exec, hook_bprm_creds_for_exec), };

__init void landlock_add_fs_hooks(void)