* [PATCH v2 0/2] landlock: Fix TSYNC deadlock and clean up error path @ 2026-02-25 2:47 Yihan Ding 2026-02-25 2:47 ` [PATCH v2 1/2] landlock: Serialize TSYNC thread restriction Yihan Ding 2026-02-25 2:47 ` [PATCH v2 2/2] landlock: Clean up interrupted thread logic in TSYNC Yihan Ding 0 siblings, 2 replies; 7+ messages in thread From: Yihan Ding @ 2026-02-25 2:47 UTC (permalink / raw) To: Mickaël Salaün, Günther Noack Cc: Paul Moore, Jann Horn, linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817, Yihan Ding 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 v2: - Split the changes into a 2-patch series for clearer logical separation. - Patch 1: Adopted down_write_killable() instead of down_write() to ensure responsiveness to fatal signals (suggested by Günther Noack). - Patch 2: Removed wait_for_completion(&shared_ctx.all_prepared) and replaced it with a `break`. The function's bottom wait for 'all_finished' already provides the necessary UAF protection (suggested by Günther Noack). 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 | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) -- 2.51.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] landlock: Serialize TSYNC thread restriction 2026-02-25 2:47 [PATCH v2 0/2] landlock: Fix TSYNC deadlock and clean up error path Yihan Ding @ 2026-02-25 2:47 ` Yihan Ding 2026-02-25 12:07 ` Günther Noack 2026-02-25 2:47 ` [PATCH v2 2/2] landlock: Clean up interrupted thread logic in TSYNC Yihan Ding 1 sibling, 1 reply; 7+ messages in thread From: Yihan Ding @ 2026-02-25 2:47 UTC (permalink / raw) To: Mickaël Salaün, Günther Noack Cc: Paul Moore, Jann Horn, linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817, Yihan Ding 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_killable() to ensure the thread remains responsive to fatal signals while waiting for the lock. 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> --- security/landlock/tsync.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c index de01aa899751..420fcfc2fe9a 100644 --- a/security/landlock/tsync.c +++ b/security/landlock/tsync.c @@ -447,6 +447,13 @@ 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 (down_write_killable(¤t->signal->exec_update_lock)) + return -EINTR; + /* * We schedule a pseudo-signal task_work for each of the calling task's * sibling threads. In the task work, each thread: @@ -556,6 +563,7 @@ 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.51.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] landlock: Serialize TSYNC thread restriction 2026-02-25 2:47 ` [PATCH v2 1/2] landlock: Serialize TSYNC thread restriction Yihan Ding @ 2026-02-25 12:07 ` Günther Noack 2026-02-25 22:33 ` Günther Noack 0 siblings, 1 reply; 7+ messages in thread From: Günther Noack @ 2026-02-25 12:07 UTC (permalink / raw) To: Yihan Ding Cc: Mickaël Salaün, Günther Noack, Paul Moore, Jann Horn, linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817 On Wed, Feb 25, 2026 at 10:47:33AM +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_killable() to ensure the thread > remains responsive to fatal signals while waiting for the lock. > > 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> > --- > security/landlock/tsync.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c > index de01aa899751..420fcfc2fe9a 100644 > --- a/security/landlock/tsync.c > +++ b/security/landlock/tsync.c > @@ -447,6 +447,13 @@ 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 (down_write_killable(¤t->signal->exec_update_lock)) > + return -EINTR; > + > /* > * We schedule a pseudo-signal task_work for each of the calling task's > * sibling threads. In the task work, each thread: > @@ -556,6 +563,7 @@ 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.51.0 > Thank you! Reviewed-by: Günther Noack <gnoack@google.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] landlock: Serialize TSYNC thread restriction 2026-02-25 12:07 ` Günther Noack @ 2026-02-25 22:33 ` Günther Noack 2026-02-26 1:43 ` Ding Yihan 0 siblings, 1 reply; 7+ messages in thread From: Günther Noack @ 2026-02-25 22:33 UTC (permalink / raw) To: Günther Noack Cc: Yihan Ding, Mickaël Salaün, Paul Moore, Jann Horn, linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817 On Wed, Feb 25, 2026 at 01:07:26PM +0100, Günther Noack wrote: > On Wed, Feb 25, 2026 at 10:47:33AM +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_killable() to ensure the thread > > remains responsive to fatal signals while waiting for the lock. > > > > 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> > > --- > > security/landlock/tsync.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c > > index de01aa899751..420fcfc2fe9a 100644 > > --- a/security/landlock/tsync.c > > +++ b/security/landlock/tsync.c > > @@ -447,6 +447,13 @@ 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 (down_write_killable(¤t->signal->exec_update_lock)) > > + return -EINTR; > > + > > /* > > * We schedule a pseudo-signal task_work for each of the calling task's > > * sibling threads. In the task work, each thread: > > @@ -556,6 +563,7 @@ 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.51.0 > > > > Thank you! > > Reviewed-by: Günther Noack <gnoack@google.com> Hello Yihan Ding! Apologies, I have to take this back -- applying the patch in this form would be a mistake. When I tried this out with the Syzkaller test case, I noticed that the tests started taking multiple seconds per run. The way I reproduced it was by running the Syzkaller reproducer under Qemu and looking for the frequency of the "executing program" lines that it prints for each test run. When I looked deeper, what was happening was actually that we got ourselves into a deadlock again, which, in hindsight should have been obvious: When two threads call landlock_restrict_self() roughly at the same time, then the one that grabs the lock first will (a) keep the other (killably) blocked on the lock acquisition, and (b) later ask the other thread to run a task work. But in order to run a task work, the blocked thread must first return from the syscall. However, down_write_killable() only returns when either the lock is available or when the thread was killed. To resolve this, we need to actually use a lock acquisition that respects other ways of interruption as well; we can either use a down_write_trylock() and return -ERESTARTNOINTR, or we can use down_write_interruptible(). Sorry for the poor advice to use the _killable variant earlier. Could I ask you to please send another revision using _trylock() or _interruptible()? Thanks, –Günther ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] landlock: Serialize TSYNC thread restriction 2026-02-25 22:33 ` Günther Noack @ 2026-02-26 1:43 ` Ding Yihan 0 siblings, 0 replies; 7+ messages in thread From: Ding Yihan @ 2026-02-26 1:43 UTC (permalink / raw) To: Günther Noack, Günther Noack Cc: Mickaël Salaün, Paul Moore, Jann Horn, linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817 Hi Günther, great catch with the QEMU test! Returning -ERESTARTNOINTR via down_write_trylock() is indeed the perfect way to allow the blocked thread to process the TWA_SIGNAL and retry. I have sent v3 with this update. 在 2026/2/26 06:33, Günther Noack 写道: > On Wed, Feb 25, 2026 at 01:07:26PM +0100, Günther Noack wrote: >> On Wed, Feb 25, 2026 at 10:47:33AM +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_killable() to ensure the thread >>> remains responsive to fatal signals while waiting for the lock. >>> >>> 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> >>> --- >>> security/landlock/tsync.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c >>> index de01aa899751..420fcfc2fe9a 100644 >>> --- a/security/landlock/tsync.c >>> +++ b/security/landlock/tsync.c >>> @@ -447,6 +447,13 @@ 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 (down_write_killable(¤t->signal->exec_update_lock)) >>> + return -EINTR; >>> + >>> /* >>> * We schedule a pseudo-signal task_work for each of the calling task's >>> * sibling threads. In the task work, each thread: >>> @@ -556,6 +563,7 @@ 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.51.0 >>> >> >> Thank you! >> >> Reviewed-by: Günther Noack <gnoack@google.com> > > Hello Yihan Ding! > > Apologies, I have to take this back -- applying the patch in this form > would be a mistake. When I tried this out with the Syzkaller test > case, I noticed that the tests started taking multiple seconds per > run. The way I reproduced it was by running the Syzkaller reproducer > under Qemu and looking for the frequency of the "executing program" > lines that it prints for each test run. > > When I looked deeper, what was happening was actually that we got > ourselves into a deadlock again, which, in hindsight should have been > obvious: When two threads call landlock_restrict_self() roughly at the > same time, then the one that grabs the lock first will (a) keep the > other (killably) blocked on the lock acquisition, and (b) later ask > the other thread to run a task work. But in order to run a task work, > the blocked thread must first return from the syscall. However, > down_write_killable() only returns when either the lock is available > or when the thread was killed. > > To resolve this, we need to actually use a lock acquisition that > respects other ways of interruption as well; we can either use a > down_write_trylock() and return -ERESTARTNOINTR, or we can use > down_write_interruptible(). > > Sorry for the poor advice to use the _killable variant earlier. Could > I ask you to please send another revision using _trylock() or > _interruptible()? > > Thanks, > –Günther > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] landlock: Clean up interrupted thread logic in TSYNC 2026-02-25 2:47 [PATCH v2 0/2] landlock: Fix TSYNC deadlock and clean up error path Yihan Ding 2026-02-25 2:47 ` [PATCH v2 1/2] landlock: Serialize TSYNC thread restriction Yihan Ding @ 2026-02-25 2:47 ` Yihan Ding 2026-02-25 12:09 ` Günther Noack 1 sibling, 1 reply; 7+ messages in thread From: Yihan Ding @ 2026-02-25 2:47 UTC (permalink / raw) To: Mickaël Salaün, Günther Noack Cc: Paul Moore, Jann Horn, linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817, Yihan Ding 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> --- 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 | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c index 420fcfc2fe9a..9731ec7f329a 100644 --- a/security/landlock/tsync.c +++ b/security/landlock/tsync.c @@ -534,24 +534,28 @@ 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.51.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] landlock: Clean up interrupted thread logic in TSYNC 2026-02-25 2:47 ` [PATCH v2 2/2] landlock: Clean up interrupted thread logic in TSYNC Yihan Ding @ 2026-02-25 12:09 ` Günther Noack 0 siblings, 0 replies; 7+ messages in thread From: Günther Noack @ 2026-02-25 12:09 UTC (permalink / raw) To: Yihan Ding Cc: Mickaël Salaün, Günther Noack, Paul Moore, Jann Horn, linux-security-module, linux-kernel, syzbot+7ea2f5e9dfd468201817 On Wed, Feb 25, 2026 at 10:47:34AM +0800, Yihan Ding wrote: > 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> > --- > 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 | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c > index 420fcfc2fe9a..9731ec7f329a 100644 > --- a/security/landlock/tsync.c > +++ b/security/landlock/tsync.c > @@ -534,24 +534,28 @@ 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.51.0 > > Thank you! Reviewed-by: Günther Noack <gnoack@google.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-02-26 1:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-25 2:47 [PATCH v2 0/2] landlock: Fix TSYNC deadlock and clean up error path Yihan Ding 2026-02-25 2:47 ` [PATCH v2 1/2] landlock: Serialize TSYNC thread restriction Yihan Ding 2026-02-25 12:07 ` Günther Noack 2026-02-25 22:33 ` Günther Noack 2026-02-26 1:43 ` Ding Yihan 2026-02-25 2:47 ` [PATCH v2 2/2] landlock: Clean up interrupted thread logic in TSYNC Yihan Ding 2026-02-25 12:09 ` Günther Noack
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox