[Devel] [PATCH VZ10 v3 3/4] connector: deliver per-VE proc events under an RCU read lock

[Vasileios Almpanis]

proc_event_connector_ve() dereferences ve->cn several times and
fill_exit_event() dereferences ve->ve_nsproxy assuming the VE cannot
stop in the middle of the delivery. This holds while the reported task
is alive in the VE: a live task keeps the VE pid namespace busy, so
zap_pid_ns_processes() -> ve_exit_ns() cannot start.

The next patch makes proc_exit_connector() deliver the exit event with
a VE reference pinned before exit_notify(). Once the task is reaped,
its pid no longer keeps the pid namespace busy: the container init may
be woken up by free_pid() from release_task(), finish
zap_pid_ns_processes() and run ve_exit_ns() while the exit event is
still being delivered:

    cpu0: exiting task                cpu1: container init
    do_exit()
      exit_notify()
        release_task()
          free_pid()  ------------->  wakes zap_pid_ns_processes()
      proc_exit_connector()           ve_exit_ns()
        proc_event_connector_ve()       cn_fini_ve()      /* ve->cn */
          fill_exit_event()             ve_drop_context() /* ve_nsproxy */
            ve->ve_nsproxy->...

Deliver the event under rcu_read_lock() and recheck the pointers: the
previous patch guarantees everything reachable from ve->cn stays alive
for the whole read-side critical section once observed, and
ve_drop_context() already waits for a grace period before dropping
ve_nsproxy. Bail out if the VE is being stopped: its listeners are
dead anyway, there is nobody to deliver to.

The whole delivery path runs with GFP_NOWAIT and never sleeps, so it
is legal inside an RCU read-side critical section.

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   | 36 +++++++++++++++++++++++++++++++++---
 drivers/connector/connector.c |  4 ++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 6095c7def7ce..608430808548 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -89,6 +89,10 @@ static inline void send_msg_ve(struct ve_struct *ve, struct cn_msg *msg)
 	struct local_event *le_ptr;
 	__u32 filter_data[2];
 
+	/* The VE is being stopped, so are its listeners: nothing to do */
+	if (!cn)
+		return;
+
 	/*
 	 * The following hack with local_event->lock address works only
 	 * till the "lock" is the first field in the local_event struct,
@@ -172,15 +176,27 @@ static void proc_event_connector_ve(struct task_struct *task,
 	struct cn_msg *msg;
 	__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
 
+	/*
+	 * The exit event may be delivered when the reported task no
+	 * longer pins the VE (see proc_exit_connector()), so the VE may
+	 * be stopping concurrently. cn_proc_fini_ve() waits for an RCU
+	 * grace period before the connector state is freed, take the RCU
+	 * read lock to make the state observed here stay valid for the
+	 * whole delivery. The path below never sleeps (GFP_NOWAIT).
+	 */
+	rcu_read_lock();
+
 	if (proc_event_num_listeners(ve) < 1)
-		return;
+		goto out_unlock;
 
 	msg = cn_msg_fill(buffer, ve, task, what, cookie, fill_event);
 	if (!msg)
-		return;
+		goto out_unlock;
 
 	/*  If cn_netlink_send() failed, the data is not sent */
 	send_msg_ve(ve, msg);
+out_unlock:
+	rcu_read_unlock();
 }
 
 static void proc_event_connector(struct task_struct *task,
@@ -350,9 +366,23 @@ void proc_coredump_connector(struct task_struct *task)
 static bool fill_exit_event(struct proc_event *ev, struct ve_struct *ve,
 			    struct task_struct *task, long cookie_pids)
 {
-	struct pid_namespace *pid_ns = ve->ve_nsproxy->pid_ns_for_children;
+	struct pid_namespace *pid_ns;
 	struct task_struct *parent;
 	struct pids *pids = (struct pids *)cookie_pids;
+	struct nsproxy *nsproxy;
+
+	/*
+	 * Unlike all other events, the exit event may be delivered after
+	 * the task was reaped, when nothing keeps the VE pid namespace
+	 * busy anymore and the VE may be stopping concurrently.
+	 * ve_drop_context() clears ve_nsproxy and waits for an RCU grace
+	 * period before dropping it; we are called under rcu_read_lock().
+	 * The VE is dead, so are its listeners: skip the event.
+	 */
+	nsproxy = rcu_dereference(ve->ve_nsproxy);
+	if (!nsproxy)
+		return false;
+	pid_ns = nsproxy->pid_ns_for_children;
 
 	ev->event_data.exit.process_pid = pid_nr_ns(pids->pid, pid_ns);
 	ev->event_data.exit.process_tgid = pid_nr_ns(pids->tgid, pid_ns);
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index 05c281bb321b..5597801c4af2 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -79,6 +79,10 @@ int cn_netlink_send_mult_ve(struct ve_struct *ve, struct cn_msg *msg, u16 len,
 	u32 group = 0;
 	int found = 0;
 
+	/* The VE is being stopped, see proc_event_connector_ve() */
+	if (!dev)
+		return -ENODEV;
+
 	if (portid || __group) {
 		group = __group;
 	} else {

-- 
2.43.0