[Devel] [PATCH VZ10 v2 2/4] connector: free the per-VE connector state after an RCU grace period

[Vasileios Almpanis]

cn_fini_ve() tears down everything the proc event delivery path uses
and only clears ve->cn at the very end. A reader that fetched ve->cn
right before the teardown dereferences freed memory afterwards:
the only guard on the delivery path is the ve->cn check in
proc_event_num_listeners() and nothing keeps the state alive once the
check has passed.

Today the window is not reachable: the per-VE delivery path is only
entered for a task alive in this VE, a live task keeps the VE pid
namespace busy, so zap_pid_ns_processes() -> ve_exit_ns() ->
cn_fini_ve() cannot run in parallel. A subsequent patch will make
proc_exit_connector() deliver the exit event with a VE reference
pinned before exit_notify(), i.e. possibly after the task was reaped
and stopped pinning the pid namespace. This will make the teardown able
to run in parallel with the delivery.

Clear ve->cn and wait for an RCU grace period before freeing anything
reachable from it, so that the delivery path can safely use the state
it observed within a single RCU read-side critical section. The clearing
is done in cn_proc_fini_ve(): it has to happen before the first thing
the delivery path uses (local_event) is freed and everything else is
freed later in cn_fini_ve().

https://virtuozzo.atlassian.net/browse/VSTOR-140421
Signed-off-by: Vasileios Almpanis <vasileios.almpanis at virtuozzo.com>

Feature: ve: ve generic structures
---
 drivers/connector/cn_proc.c   | 11 +++++++++++
 drivers/connector/connector.c |  2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index d4ce1697dd0b..6095c7def7ce 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -535,5 +535,16 @@ void cn_proc_fini_ve(struct ve_struct *ve)
 				       ve_is_super(ve));
 
 	cn_del_callback_ve(ve, &cn_proc_event_id);
+
+	/*
+	 * Hide the connector state from the proc event delivery path,
+	 * which dereferences ve->cn under rcu_read_lock(), and wait for
+	 * the readers to finish before anything reachable from it is
+	 * freed: the percpu local_event here, the callback device, the
+	 * netlink socket and the state itself in cn_fini_ve().
+	 */
+	RCU_INIT_POINTER(ve->cn, NULL);
+	synchronize_rcu();
+
 	free_percpu(cn->local_event);
 }
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index bf4a83f3f870..6483e15b888e 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -394,7 +394,7 @@ static void cn_fini_ve(void *data)
 	cn_queue_free_dev(dev->cbdev);
 	netlink_kernel_release(dev->nls);
 
-	RCU_INIT_POINTER(ve->cn, NULL);
+	/* ve->cn was cleared by cn_proc_fini_ve() before the grace period */
 	kfree(cn);
 }
 

-- 
2.43.0