* [PATCH v4 0/2] landlock: Fix TSYNC deadlock and clean up error path @ 2026-03-04 9:54 Yihan Ding 2026-03-04 9:54 ` [PATCH v4 1/2] landlock: Serialize TSYNC thread restriction Yihan Ding 2026-03-04 9:54 ` [PATCH v4 2/2] landlock: Clean up interrupted thread logic in TSYNC Yihan Ding 0 siblings, 2 replies; 5+ messages in thread From: Yihan Ding @ 2026-03-04 9:54 UTC (permalink / raw) To: gnoack3000 Cc: dingyihan, jannh, linux-kernel, linux-security-module, m, mic, paul, syzbot+7ea2f5e9dfd468201817, utilityemal77 Hello, This patch series fixes a deadlock in the Landlock TSYNC multithreading support, originally reported by syzbot, and cleans up the associated interrupt recovery path. The deadlock occurs when multiple threads concurrently call landlock_restrict_self() with sibling thread restriction enabled, causing them to mutually queue task_works on each other and block indefinitely. * Patch 1 fixes the root cause by serializing the TSYNC operations within the same process using the exec_update_lock. * Patch 2 cleans up the interrupt recovery path by replacing an unnecessary wait_for_completion() with a straightforward loop break, avoiding Use-After-Free while unblocking remaining task_works. Changes in v4: - Patch 1: Use restart_syscall() instead of returning -ERESTARTNOINTR. This ensures the syscall is properly restarted without leaking the internal error code to userspace, fixing a test failure in tsync_test.competing_enablement. (Caught by Justin Suess, suggested by Tingmao Wang). - Patch 1 and 2: Wrap comments to fit in 80 columns Changes in v3: - Patch 1: Changed down_write_killable() to down_write_trylock() and return -ERESTARTNOINTR on failure. This avoids a secondary deadlock where a blocking wait prevents a sibling thread from waking up to execute the requested TSYNC task_work. (Noted by Günther Noack. down_write_interruptible() was also suggested but is not implemented for rw_semaphores in the kernel). - Patch 2: No changes. Changes in v2: - Split the changes into a 2-patch series. - Patch 1: Adopted down_write_killable() instead of down_write(). - Patch 2: Removed wait_for_completion(&shared_ctx.all_prepared) and replaced it with a `break` to prevent UAF. Link to v3: https://lore.kernel.org/all/20260226015903.3158620-1-dingyihan@uniontech.com/ Link to v2: https://lore.kernel.org/all/20260225024734.3024732-1-dingyihan@uniontech.com/ Link to v1: https://lore.kernel.org/all/20260224062729.2908692-1-dingyihan@uniontech.com/ Yihan Ding (2): landlock: Serialize TSYNC thread restriction landlock: Clean up interrupted thread logic in TSYNC security/landlock/tsync.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 1/2] landlock: Serialize TSYNC thread restriction 2026-03-04 9:54 [PATCH v4 0/2] landlock: Fix TSYNC deadlock and clean up error path Yihan Ding @ 2026-03-04 9:54 ` Yihan Ding 2026-03-04 14:15 ` Justin Suess 2026-03-04 19:05 ` Günther Noack 2026-03-04 9:54 ` [PATCH v4 2/2] landlock: Clean up interrupted thread logic in TSYNC Yihan Ding 1 sibling, 2 replies; 5+ messages in thread From: Yihan Ding @ 2026-03-04 9:54 UTC (permalink / raw) To: gnoack3000 Cc: dingyihan, jannh, linux-kernel, linux-security-module, m, mic, paul, syzbot+7ea2f5e9dfd468201817, utilityemal77 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 return -ERESTARTNOINTR 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 <gnoack3000@gmail.com> Signed-off-by: Yihan Ding <dingyihan@uniontech.com> --- Changes in v4: - Use restart_syscall() instead of returning -ERESTARTNOINTR. This ensures the syscall is properly restarted without leaking the internal error code to userspace, fixing a test failure in tsync_test.competing_enablement. (Caught by Justin Suess, suggested by Tingmao Wang). Changes in v3: - Replaced down_write_killable() with down_write_trylock() and returned -ERESTARTNOINTR to avoid a secondary deadlock caused by blocking the execution of task_works. (Caught by Günther Noack). --- security/landlock/tsync.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c index de01aa899751..1f460b9ec833 100644 --- a/security/landlock/tsync.c +++ b/security/landlock/tsync.c @@ -446,6 +446,15 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred, shared_ctx.old_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 @@ -556,6 +565,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); } -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/2] landlock: Serialize TSYNC thread restriction 2026-03-04 9:54 ` [PATCH v4 1/2] landlock: Serialize TSYNC thread restriction Yihan Ding @ 2026-03-04 14:15 ` Justin Suess 2026-03-04 19:05 ` Günther Noack 1 sibling, 0 replies; 5+ messages in thread From: Justin Suess @ 2026-03-04 14:15 UTC (permalink / raw) To: Yihan Ding Cc: gnoack3000, jannh, linux-kernel, linux-security-module, m, mic, paul, syzbot+7ea2f5e9dfd468201817 On Wed, Mar 04, 2026 at 05:54:17PM +0800, Yihan Ding wrote: > 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 return -ERESTARTNOINTR 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 <gnoack3000@gmail.com> > Signed-off-by: Yihan Ding <dingyihan@uniontech.com> > --- > Changes in v4: > - Use restart_syscall() instead of returning -ERESTARTNOINTR. > This ensures the syscall is properly restarted without leaking the > internal error code to userspace, fixing a test failure in > tsync_test.competing_enablement. (Caught by Justin Suess, suggested > by Tingmao Wang). > > Changes in v3: > - Replaced down_write_killable() with down_write_trylock() and > returned -ERESTARTNOINTR to avoid a secondary deadlock caused by > blocking the execution of task_works. (Caught by Günther Noack). > > --- > security/landlock/tsync.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c > index de01aa899751..1f460b9ec833 100644 > --- a/security/landlock/tsync.c > +++ b/security/landlock/tsync.c > @@ -446,6 +446,15 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred, > shared_ctx.old_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 > @@ -556,6 +565,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); > } > -- > 2.20.1 > Tested-by: Justin Suess <utilityemal77@gmail.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/2] landlock: Serialize TSYNC thread restriction 2026-03-04 9:54 ` [PATCH v4 1/2] landlock: Serialize TSYNC thread restriction Yihan Ding 2026-03-04 14:15 ` Justin Suess @ 2026-03-04 19:05 ` Günther Noack 1 sibling, 0 replies; 5+ messages in thread From: Günther Noack @ 2026-03-04 19:05 UTC (permalink / raw) To: Yihan Ding Cc: jannh, linux-kernel, linux-security-module, m, mic, paul, syzbot+7ea2f5e9dfd468201817, utilityemal77 Hello! Thank you for the revised patch. I tested this and it passes the selftests now. I noticed that the comment is unfortunately wrongly indented, which is also caught by clang-format (see below); we should fix that before submit; I remarked on another two less important nits in the commit message as well. On Wed, Mar 04, 2026 at 05:54:17PM +0800, Yihan Ding wrote: > 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 return -ERESTARTNOINTR if the lock ^^^^^^^^^^^^^^^^^^^^^^ Nit: Maybe it would be better to write "restart the syscall" now, as we are not just returning -ERESTARTNOINTR here any more. > 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 <gnoack3000@gmail.com> > Signed-off-by: Yihan Ding <dingyihan@uniontech.com> FYI This would be a good place to add a Reported-by: Justin and Suggested-by: Tingmao as well. > --- > Changes in v4: > - Use restart_syscall() instead of returning -ERESTARTNOINTR. > This ensures the syscall is properly restarted without leaking the > internal error code to userspace, fixing a test failure in > tsync_test.competing_enablement. (Caught by Justin Suess, suggested > by Tingmao Wang). > > Changes in v3: > - Replaced down_write_killable() with down_write_trylock() and > returned -ERESTARTNOINTR to avoid a secondary deadlock caused by > blocking the execution of task_works. (Caught by Günther Noack). > > --- > security/landlock/tsync.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c > index de01aa899751..1f460b9ec833 100644 > --- a/security/landlock/tsync.c > +++ b/security/landlock/tsync.c > @@ -446,6 +446,15 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred, > shared_ctx.old_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. > + */ ^^^^^^^^^^^^^^^^ This comment should be indented with one tab, not with two, the same as the code that follows it. The comment text is good. Remark on the side, the Landlock code is formatted with clang-format; I have a small tool that runs it for me on these files: clang-format -i \ security/landlock/*.[ch] \ include/uapi/linux/landlock.h \ tools/testing/selftests/landlock/*.[ch] clang-format does not fix the indentation perfectly here either, but it discovers the issue and reindents at least the first line. > + 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 > @@ -556,6 +565,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); > } > -- > 2.20.1 > These remarks are all in comments and documentation though; the code looks good to me and the selftests work! Thank you for looking into it! –Günther ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 2/2] landlock: Clean up interrupted thread logic in TSYNC 2026-03-04 9:54 [PATCH v4 0/2] landlock: Fix TSYNC deadlock and clean up error path Yihan Ding 2026-03-04 9:54 ` [PATCH v4 1/2] landlock: Serialize TSYNC thread restriction Yihan Ding @ 2026-03-04 9:54 ` Yihan Ding 1 sibling, 0 replies; 5+ messages in thread From: Yihan Ding @ 2026-03-04 9:54 UTC (permalink / raw) To: gnoack3000 Cc: dingyihan, jannh, linux-kernel, linux-security-module, m, mic, paul, syzbot+7ea2f5e9dfd468201817, utilityemal77 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 <gnoack3000@gmail.com> Signed-off-by: Yihan Ding <dingyihan@uniontech.com> --- Change in v3: -No change in v3 Changes in v2: - Replaced wait_for_completion(&shared_ctx.all_prepared) with a break statement based on the realization that the bottom wait for 'all_finished' already guards against UAF. - Updated comments for clarity. --- security/landlock/tsync.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c index 1f460b9ec833..d52583ee1d93 100644 --- a/security/landlock/tsync.c +++ b/security/landlock/tsync.c @@ -536,24 +536,27 @@ 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); -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-04 19:05 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-04 9:54 [PATCH v4 0/2] landlock: Fix TSYNC deadlock and clean up error path Yihan Ding 2026-03-04 9:54 ` [PATCH v4 1/2] landlock: Serialize TSYNC thread restriction Yihan Ding 2026-03-04 14:15 ` Justin Suess 2026-03-04 19:05 ` Günther Noack 2026-03-04 9:54 ` [PATCH v4 2/2] landlock: Clean up interrupted thread logic in TSYNC Yihan Ding
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox