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 <[email protected]>
https://virtuozzo.atlassian.net/browse/VSTOR-132310
Signed-off-by: Konstantin Khorenko <[email protected]>
---
 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.
-- 
2.47.1

_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to