Forwarded: Re: [syzbot] [kernel?] INFO: task hung in restrict_one_thread_callback

syzbot posted 1 patch 1 month, 1 week ago
There is a newer version of this series
Forwarded: Re: [syzbot] [kernel?] INFO: task hung in restrict_one_thread_callback
Posted by syzbot 1 month, 1 week ago
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: Re: [syzbot] [kernel?] INFO: task hung in restrict_one_thread_callback
Author: dingyihan@uniontech.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master

--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -447,6 +447,12 @@ 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.
+	 */
+	down_write(&current->signal->exec_update_lock);
+
 	/*
 	 * We schedule a pseudo-signal task_work for each of the calling task's
 	 * sibling threads.  In the task work, each thread:
@@ -527,14 +533,17 @@ 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.
+				 * We must wait for the remaining task works to finish to
+				 * prevent a use-after-free of the local shared_ctx.
 				 */
 				wait_for_completion(&shared_ctx.all_prepared);
 			}
@@ -557,5 +566,7 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
 
 	tsync_works_release(&works);
 
+	up_write(&current->signal->exec_update_lock);
+
 	return atomic_read(&shared_ctx.preparation_error);
 }

在 2026/2/24 11:03, syzbot 写道:
>> Hi Günther,
>>
>> Thank you for the detailed analysis! I completely agree that serializing the TSYNC 
>> operations is the right way to prevent this deadlock. I have drafted a patch using 
>> `exec_update_lock` (similar to how seccomp uses `cred_guard_mutex`).
>>
>> Regarding your proposal to split this into two patches (one for the cleanup 
>> path and one for the lock): Maybe combining them into a single patch is a better choice. Here is why:
>>
>> We actually *cannot* remove `wait_for_completion(&shared_ctx.all_prepared)` 
>> in the interrupt recovery path. Since `shared_ctx` is allocated on the local 
>> stack of the caller, removing the wait would cause a severe Use-After-Free (UAF) if the 
>> thread returns to userspace while sibling task_works are still executing and dereferencing `ctx`. 
>>
>> By adding the lock, we inherently resolve the deadlock, meaning the sibling task_works 
>> will never get stuck. Thus, `wait_for_completion` becomes perfectly safe to keep, 
>> and it remains strictly necessary to protect the stack memory. Therefore, the "fix" for the 
>> cleanup path is simply updating the comments to reflect this reality, which is tightly coupled with the locking fix. 
>> It felt more cohesive as a single patch.
>>
>> I have test the patch on my laptop,and it will not trigger the issue.Let's have syzbot test this combined logic:
>>
>> #syz test: 
> 
> "---" does not look like a valid git repo address.
>