From 405ca72dc589dd746e5ee5378bb9d9ee7f844010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Tue, 3 Mar 2026 18:36:31 +0100 Subject: landlock: Fix formatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Auto-format with clang-format -i security/landlock/*.[ch] Cc: Günther Noack Cc: Kees Cook Fixes: 69050f8d6d07 ("treewide: Replace kmalloc with kmalloc_obj for non-scalar types") Reviewed-by: Günther Noack Link: https://lore.kernel.org/r/20260303173632.88040-1-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/domain.c | 3 +-- security/landlock/ruleset.c | 9 ++++----- 2 files changed, 5 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/landlock/domain.c b/security/landlock/domain.c index f5b78d4766cd..f0d83f43afa1 100644 --- a/security/landlock/domain.c +++ b/security/landlock/domain.c @@ -94,8 +94,7 @@ static struct landlock_details *get_current_details(void) * allocate with GFP_KERNEL_ACCOUNT because it is independent from the * caller. */ - details = - kzalloc_flex(*details, exe_path, path_size); + details = kzalloc_flex(*details, exe_path, path_size); if (!details) return ERR_PTR(-ENOMEM); diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c index 319873586385..73018dc8d6c7 100644 --- a/security/landlock/ruleset.c +++ b/security/landlock/ruleset.c @@ -32,9 +32,8 @@ static struct landlock_ruleset *create_ruleset(const u32 num_layers) { struct landlock_ruleset *new_ruleset; - new_ruleset = - kzalloc_flex(*new_ruleset, access_masks, num_layers, - GFP_KERNEL_ACCOUNT); + new_ruleset = kzalloc_flex(*new_ruleset, access_masks, num_layers, + GFP_KERNEL_ACCOUNT); if (!new_ruleset) return ERR_PTR(-ENOMEM); refcount_set(&new_ruleset->usage, 1); @@ -559,8 +558,8 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent, if (IS_ERR(new_dom)) return new_dom; - new_dom->hierarchy = kzalloc_obj(*new_dom->hierarchy, - GFP_KERNEL_ACCOUNT); + new_dom->hierarchy = + kzalloc_obj(*new_dom->hierarchy, GFP_KERNEL_ACCOUNT); if (!new_dom->hierarchy) return ERR_PTR(-ENOMEM); -- cgit v1.2.3 From 929553bbb4cdda9be22175e1adb4d5814b770855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Tue, 17 Feb 2026 13:23:39 +0100 Subject: landlock: Fully release unused TSYNC work entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If task_work_add() failed, ctx->task is put but the tsync_works struct is not reset to its previous state. The first consequence is that the kernel allocates memory for dying threads, which could lead to user-accounted memory exhaustion (not very useful nor specific to this case). The second consequence is that task_work_cancel(), called by cancel_tsync_works(), can dereference a NULL task pointer. Fix this issues by keeping a consistent works->size wrt the added task work. This is done in a new tsync_works_trim() helper which also cleans up the shared_ctx and work fields. As a safeguard, add a pointer check to cancel_tsync_works() and update tsync_works_release() accordingly. Cc: Jann Horn Reviewed-by: Günther Noack Link: https://lore.kernel.org/r/20260217122341.2359582-1-mic@digikod.net [mic: Replace memset() with compound literal] Signed-off-by: Mickaël Salaün --- security/landlock/tsync.c | 47 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c index de01aa899751..3e44be4f66e3 100644 --- a/security/landlock/tsync.c +++ b/security/landlock/tsync.c @@ -203,6 +203,40 @@ static struct tsync_work *tsync_works_provide(struct tsync_works *s, return ctx; } +/** + * tsync_works_trim - Put the last tsync_work element + * + * @s: TSYNC works to trim. + * + * Put the last task and decrement the size of @s. + * + * This helper does not cancel a running task, but just reset the last element + * to zero. + */ +static void tsync_works_trim(struct tsync_works *s) +{ + struct tsync_work *ctx; + + if (WARN_ON_ONCE(s->size <= 0)) + return; + + ctx = s->works[s->size - 1]; + + /* + * For consistency, remove the task from ctx so that it does not look like + * we handed it a task_work. + */ + put_task_struct(ctx->task); + *ctx = (typeof(*ctx)){}; + + /* + * Cancel the tsync_works_provide() change to recycle the reserved memory + * for the next thread, if any. This also ensures that cancel_tsync_works() + * and tsync_works_release() do not see any NULL task pointers. + */ + s->size--; +} + /* * tsync_works_grow_by - preallocates space for n more contexts in s * @@ -276,7 +310,7 @@ static void tsync_works_release(struct tsync_works *s) size_t i; for (i = 0; i < s->size; i++) { - if (!s->works[i]->task) + if (WARN_ON_ONCE(!s->works[i]->task)) continue; put_task_struct(s->works[i]->task); @@ -379,16 +413,14 @@ static bool schedule_task_work(struct tsync_works *works, init_task_work(&ctx->work, restrict_one_thread_callback); err = task_work_add(thread, &ctx->work, TWA_SIGNAL); - if (err) { + if (unlikely(err)) { /* * task_work_add() only fails if the task is about to exit. We * checked that earlier, but it can happen as a race. Resume * without setting an error, as the task is probably gone in the - * next loop iteration. For consistency, remove the task from ctx - * so that it does not look like we handed it a task_work. + * next loop iteration. */ - put_task_struct(ctx->task); - ctx->task = NULL; + tsync_works_trim(works); atomic_dec(&shared_ctx->num_preparing); atomic_dec(&shared_ctx->num_unfinished); @@ -412,6 +444,9 @@ static void cancel_tsync_works(struct tsync_works *works, int i; for (i = 0; i < works->size; i++) { + if (WARN_ON_ONCE(!works->works[i]->task)) + continue; + if (!task_work_cancel(works->works[i]->task, &works->works[i]->work)) continue; -- cgit v1.2.3 From bb8369ead40771b9550e5dbc287d6b707dd6c2b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Tue, 17 Feb 2026 13:23:40 +0100 Subject: landlock: Improve TSYNC types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constify pointers when it makes sense. Consistently use size_t for loops, especially to match works->size type. Add new lines to improve readability. Cc: Jann Horn Reviewed-by: Günther Noack Link: https://lore.kernel.org/r/20260217122341.2359582-2-mic@digikod.net Signed-off-by: Mickaël Salaün --- security/landlock/tsync.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'security') diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c index 3e44be4f66e3..1e738ef2cff5 100644 --- a/security/landlock/tsync.c +++ b/security/landlock/tsync.c @@ -290,13 +290,14 @@ static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags) * tsync_works_contains - checks for presence of task in s */ static bool tsync_works_contains_task(const struct tsync_works *s, - struct task_struct *task) + const struct task_struct *task) { size_t i; for (i = 0; i < s->size; i++) if (s->works[i]->task == task) return true; + return false; } @@ -318,6 +319,7 @@ static void tsync_works_release(struct tsync_works *s) for (i = 0; i < s->capacity; i++) kfree(s->works[i]); + kfree(s->works); s->works = NULL; s->size = 0; @@ -329,7 +331,7 @@ static void tsync_works_release(struct tsync_works *s) */ static size_t count_additional_threads(const struct tsync_works *works) { - struct task_struct *thread, *caller; + const struct task_struct *caller, *thread; size_t n = 0; caller = current; @@ -368,7 +370,8 @@ static bool schedule_task_work(struct tsync_works *works, struct tsync_shared_context *shared_ctx) { int err; - struct task_struct *thread, *caller; + const struct task_struct *caller; + struct task_struct *thread; struct tsync_work *ctx; bool found_more_threads = false; @@ -438,10 +441,10 @@ static bool schedule_task_work(struct tsync_works *works, * shared_ctx->num_preparing and shared_ctx->num_unfished and mark the two * completions if needed, as if the task was never scheduled. */ -static void cancel_tsync_works(struct tsync_works *works, +static void cancel_tsync_works(const struct tsync_works *works, struct tsync_shared_context *shared_ctx) { - int i; + size_t i; for (i = 0; i < works->size; i++) { if (WARN_ON_ONCE(!works->works[i]->task)) -- cgit v1.2.3 From ff88df67dbf78b5eb909f8a3da4115b1cfd998ab Mon Sep 17 00:00:00 2001 From: Yihan Ding Date: Fri, 6 Mar 2026 10:16:50 +0800 Subject: landlock: Serialize TSYNC thread restriction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit syzbot found a deadlock in landlock_restrict_sibling_threads(). When multiple threads concurrently call landlock_restrict_self() with sibling thread restriction enabled, they can deadlock by mutually queueing task_works on each other and then blocking in kernel space (waiting for the other to finish). Fix this by serializing the TSYNC operations within the same process using the exec_update_lock. This prevents concurrent invocations from deadlocking. We use down_write_trylock() and restart the syscall if the lock cannot be acquired immediately. This ensures that if a thread fails to get the lock, it will return to userspace, allowing it to process any pending TSYNC task_works from the lock holder, and then transparently restart the syscall. Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()") Reported-by: syzbot+7ea2f5e9dfd468201817@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=7ea2f5e9dfd468201817 Suggested-by: Günther Noack Suggested-by: Tingmao Wang Tested-by: Justin Suess Signed-off-by: Yihan Ding Tested-by: Günther Noack Reviewed-by: Günther Noack Link: https://lore.kernel.org/r/20260306021651.744723-2-dingyihan@uniontech.com Signed-off-by: Mickaël Salaün --- security/landlock/tsync.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c index 1e738ef2cff5..f0e5f8102001 100644 --- a/security/landlock/tsync.c +++ b/security/landlock/tsync.c @@ -485,6 +485,16 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred, shared_ctx.new_cred = new_cred; shared_ctx.set_no_new_privs = task_no_new_privs(current); + /* + * Serialize concurrent TSYNC operations to prevent deadlocks when + * multiple threads call landlock_restrict_self() simultaneously. + * If the lock is already held, we gracefully yield by restarting the + * syscall. This allows the current thread to process pending + * task_works before retrying. + */ + if (!down_write_trylock(¤t->signal->exec_update_lock)) + return restart_syscall(); + /* * We schedule a pseudo-signal task_work for each of the calling task's * sibling threads. In the task work, each thread: @@ -594,6 +604,6 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred, wait_for_completion(&shared_ctx.all_finished); tsync_works_release(&works); - + up_write(¤t->signal->exec_update_lock); return atomic_read(&shared_ctx.preparation_error); } -- cgit v1.2.3 From 697f514ad9dbe600a808326d80b02caab03b7f90 Mon Sep 17 00:00:00 2001 From: Yihan Ding Date: Fri, 6 Mar 2026 10:16:51 +0800 Subject: landlock: Clean up interrupted thread logic in TSYNC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In landlock_restrict_sibling_threads(), when the calling thread is interrupted while waiting for sibling threads to prepare, it executes a recovery path. Previously, this path included a wait_for_completion() call on all_prepared to prevent a Use-After-Free of the local shared_ctx. However, this wait is redundant. Exiting the main do-while loop already leads to a bottom cleanup section that unconditionally waits for all_finished. Therefore, replacing the wait with a simple break is safe, prevents UAF, and correctly unblocks the remaining task_works. Clean up the error path by breaking the loop and updating the surrounding comments to accurately reflect the state machine. Suggested-by: Günther Noack Signed-off-by: Yihan Ding Tested-by: Günther Noack Reviewed-by: Günther Noack Link: https://lore.kernel.org/r/20260306021651.744723-3-dingyihan@uniontech.com Signed-off-by: Mickaël Salaün --- security/landlock/tsync.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c index f0e5f8102001..4d4427ba8d93 100644 --- a/security/landlock/tsync.c +++ b/security/landlock/tsync.c @@ -575,24 +575,30 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred, -ERESTARTNOINTR); /* - * Cancel task works for tasks that did not start running yet, - * and decrement all_prepared and num_unfinished accordingly. + * Opportunistic improvement: try to cancel task + * works for tasks that did not start running + * yet. We do not have a guarantee that it + * cancels any of the enqueued task works + * because task_work_run() might already have + * dequeued them. */ cancel_tsync_works(&works, &shared_ctx); /* - * The remaining task works have started running, so waiting for - * their completion will finish. + * Break the loop with error. The cleanup code + * after the loop unblocks the remaining + * task_works. */ - wait_for_completion(&shared_ctx.all_prepared); + break; } } } while (found_more_threads && !atomic_read(&shared_ctx.preparation_error)); /* - * We now have all sibling threads blocking and in "prepared" state in the - * task work. Ask all threads to commit. + * We now have either (a) all sibling threads blocking and in "prepared" + * state in the task work, or (b) the preparation error is set. Ask all + * threads to commit (or abort). */ complete_all(&shared_ctx.ready_to_commit); -- cgit v1.2.3