[Devel] [PATCH vz10 2/2] ve: do not wait for user-mode helpers under ve->op_sem in ve_stop_ns()

[Konstantin Khorenko]

merged into
e766b39ed060 ve/kthread: fix race when work can be added to stopped kthread worker #VSTOR-106887

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 6/12/26 11:56, Pavel Tikhomirov wrote:
> Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> 
> On 6/12/26 08:57, Konstantin Khorenko wrote:
>> ve_stop_ns() switches the VE to VE_STATE_STOPPING and then waits for all
>> in-flight per-VE user-mode helpers to drain via wait_khelpers(). After
>> the per-VE workqueue / cgroup release_agent infrastructure was removed,
>> this wait ended up running under ve->op_sem held for write.
>>
>> That is a deadlock: a call_usermodehelper_exec_ve() payload waited for
>> by wait_khelpers() may run arbitrary user code that accesses ve.* cgroup
>> files, and those take ve->op_sem (for read). ve_stop_ns() would then
>> block forever holding op_sem for write while the helper blocks trying to
>> acquire it.
>>
>> Drop ve->op_sem around wait_khelpers() and re-acquire it afterwards, the
>> way the code did before the release_agent removal (it used to drop the
>> lock around wait_khelpers() + ve_workqueue_stop()). The VE is already in
>> VE_STATE_STOPPING before the lock is dropped, so no new helper can be
>> queued and all op_sem entry points bail out on the state check.
>>
>> Fixes: 9b103188a9b2 ("ve/kthread: fix race when work can be added to stopped kthread worker")
>> Reported-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>> https://virtuozzo.atlassian.net/browse/VSTOR-132310
>> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
>> ---
>>  kernel/ve/ve.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
>> index 65723f28dbad..231c0300e929 100644
>> --- a/kernel/ve/ve.c
>> +++ b/kernel/ve/ve.c
>> @@ -575,7 +575,19 @@ void ve_stop_ns(struct pid_namespace *pid_ns)
>>  	ve_set_state(ve, VE_STATE_STOPPING);
>>  	synchronize_rcu();
>>  
>> +	/*
>> +	 * Drop the lock before waiting for in-flight user-mode helpers.
>> +	 * A call_usermodehelper_exec_ve() payload may access ve.* cgroup
>> +	 * files, which take ve->op_sem, so waiting for it via wait_khelpers()
>> +	 * while holding op_sem would deadlock. The state is already
>> +	 * VE_STATE_STOPPING, so no new helper can be queued; all entry points
>> +	 * must check the state before proceeding.
>> +	 */
>> +	up_write(&ve->op_sem);
>> +
>>  	wait_khelpers(ve);
>> +
>> +	down_write(&ve->op_sem);
>>  	/*
>>  	 * Neither it can be in pseudosuper state
>>  	 * anymore, setup it again if needed.
>