[PATCH v3 1/2] landlock: Serialize TSYNC thread restriction

[Tingmao Wang]

On 3/3/26 19:50, Günther Noack wrote:
> [...]
> On Tue, Mar 03, 2026 at 11:20:10AM -0500, Justin Suess wrote:
>> On Thu, Feb 26, 2026 at 09:59:02AM +0800, Yihan Ding wrote:
>>> [...]
>>> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
>>> index de01aa899751..xxxxxxxxxxxx 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_trylock(&current->signal->exec_update_lock))
>>> +		return -ERESTARTNOINTR;
>> These two lines above introduced a test failure in tsync_test
>> completing_enablement.
>>
>> The commit that introduced the bug is 3d6327c306b3e1356ab868bf27a0854669295a4f
>> (this patch) and is currently in the mic/next branch.
>>
>> I noticed the test failure while testing an unrelated patch.
>>
>> The bug is because this code never actually yields or restarts the syscall.
>>
>> This is the test output I observed:
>>
>>   [+] Running tsync_test:
>>   TAP version 13
>>   1..4
>>   # Starting 4 tests from 1 test cases.
>>   #  RUN           global.single_threaded_success ...
>>   #            OK  global.single_threaded_success
>>   ok 1 global.single_threaded_success
>>   #  RUN           global.multi_threaded_success ...
>>   #            OK  global.multi_threaded_success
>>   ok 2 global.multi_threaded_success
>>   #  RUN           global.multi_threaded_success_despite_diverging_domains ...
>>   #            OK  global.multi_threaded_success_despite_diverging_domains
>>   ok 3 global.multi_threaded_success_despite_diverging_domains
>>   #  RUN           global.competing_enablement ...
>>   # tsync_test.c:156:competing_enablement:Expected 0 (0) == d[1].result (-1)
> 
> The interesting part here is when you print out the errno that is
> returned from the syscall -- it is 513, the value of ERESTARTNOINTR!
> 
> My understanding so far: Poking around in kernel/entry/common.c, it
> seems that __exit_to_user_mode_loop() calls
> arch_do_signal_or_restart() only when there is a pending signal
> (_TIF_SIGPENDING or _TIF_NOTIFY_SIGNAL).  So it was possible that the
> system call returns with the (normally internal) error code
> ERESTARTNOINTR, in the case where the trylock fails, but where current
> has not received a signal from the other competing TSYNC thread yet.
> 
> So with that in mind, would it work to do this?
> 
>   while (try-to-acquire-the-lock) {
>     if (current-has-task-works-pending)
>       return -ERESTARTNOINTR;
> 
>     cond_resched();
>   }
> 
> Then we could avoid calling task_work_run() directly; (I find it
> difficult to reason about the implications of calling taks_work_run()
> directly, because these task works may make assumptions about the
> context in which they are running.)

I've not caught up with the full discussion so might be missing some context on why RESTARTNOINTR was used here,
but wouldn't

diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index 950b63d23729..f695fe44e2f1 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -490,7 +490,7 @@ int landlock_restrict_sibling_threads(const struct cred *old_cred,
 	 * when multiple threads call landlock_restrict_self() simultaneously.
 	 */
 	if (!down_write_trylock(&current->signal->exec_update_lock))
-		return -ERESTARTNOINTR;
+		return restart_syscall();
 
 	/*
 	 * We schedule a pseudo-signal task_work for each of the calling task's

achieve what the original patch intended?

Kind regards,
Tingmao