[Devel] [PATCH VZ10 v2 2/4] connector: free the per-VE connector state after an RCU grace period
Konstantin Khorenko
khorenko at virtuozzo.com
Tue Aug 25 14:47:20 MSK 2026
On 8/18/26 17:10, Vasileios Almpanis wrote:
> 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();
> +
[Severity: Medium]
Can this crash the netlink receive path while the VE is stopping?
Clearing ve->cn now happens long before the kernel netlink socket is
released in cn_fini_ve(): the window covers this synchronize_rcu(),
free_percpu(), remove_proc_entry() and cn_queue_free_dev(). Within
that window dev->nls is still alive and accepts messages, and a send
to it ends up in:
drivers/connector/connector.c:cn_call_callback() {
...
struct cn_dev *dev = get_cdev(sock_net(skb->sk)->owner_ve);
...
spin_lock_bh(&dev->cbdev->queue_lock);
...
}
Since the previous patch get_cdev() returns NULL once ve->cn is
cleared, so this dereferences a NULL dev.
All in-VE tasks are dead at this point, but the VE netns can still be
entered from the host (nsenter, criu and vzctl network tooling), and a
NETLINK_CONNECTOR socket created there delivers straight into
cn_rx_skb() -> cn_call_callback() in the sender's context.
Before this patch the pointer was cleared only after
netlink_kernel_release(), which quiesces the input path first, so this
window did not exist. The next patch adds a NULL check to the send
side (cn_netlink_send_mult_ve()) but not to the receive side.
Would a NULL check in cn_call_callback(), or clearing ve->cn only
after netlink_kernel_release() in cn_fini_ve() while keeping the
grace period before the frees, close this?
=============================================================
● Scenario 1 - NULL deref (fetch after the clear)
Precondition: the container is stopping - all CT tasks are dead, the container init is finishing zap_pid_ns_processes().
On the host there is a process (criu / vzctl tooling / nsenter) that has opened a NETLINK_CONNECTOR socket in this CT's
netns beforehand (the socket keeps the netns alive).
CPU0: container init (last CT task) CPU1: host process with a socket in the CT netns
========================================= ============================================
do_exit()
exit_notify()
zap_pid_ns_processes() /* pid_ns is empty */
ve_exit_ns()
down_write(&ve->op_sem)
ve_hook_iterate_fini()
cn_fini_ve()
cn = rcu_dereference_protected(ve->cn)
cn->cn_already_initialized = 0
cn_proc_fini_ve()
cn_del_callback_ve()
RCU_INIT_POINTER(ve->cn, NULL) <--- pointer is hidden
synchronize_rcu() <--- sleeps for milliseconds,
... dev->nls is still ALIVE
... sendmsg(nl_sock, msg)
... netlink_sendmsg()
... netlink_unicast()
... netlink_unicast_kernel()
... nlk->netlink_rcv = cn_rx_skb()
... cn_call_callback()
... dev = get_cdev(net->owner_ve)
... rcu_dereference_check(ve->cn, 1)
... /* ve->cn == NULL */
... return NULL; <--- patch 1
... spin_lock_bh(&dev->cbdev->queue_lock)
... /* dev == NULL */
... *** oops: NULL + offsetof(cbdev) ***
free_percpu(cn->local_event)
remove_proc_entry("connector", ...)
cn_queue_free_dev(dev->cbdev)
netlink_kernel_release(dev->nls) <--- only HERE does the input path die,
kfree(cn) but it is too late
The key point: the window between RCU_INIT_POINTER(ve->cn, NULL) and netlink_kernel_release() is now wide (it contains a
whole synchronize_rcu()), the kernel socket keeps accepting messages all that time, and cn_call_callback() on the receive
side has no NULL check. Before patch 2 the order was reversed: netlink_kernel_release() quiesced the input path first, and
only then ve->cn = NULL - within that window CPU1 simply could not reach cn_call_callback().
● Scenario 2 - UAF (fetch before the clear, use after the free)
Same setup, but CPU1 fetched the pointer before the clear and got preempted. synchronize_rcu() does not wait for it -
cn_call_callback() reads ve->cn without rcu_read_lock():
CPU0: container init CPU1: host-side sender
========================================= ============================================
cn_rx_skb()
cn_call_callback()
dev = get_cdev(...) /* cn != NULL, ok */
<-- preempted -->
cn_proc_fini_ve()
RCU_INIT_POINTER(ve->cn, NULL)
synchronize_rcu() /* CPU1 is not in an RCU
section - it will NOT
be waited for */
free_percpu(cn->local_event)
cn_queue_free_dev(dev->cbdev) <--- cbdev is freed
netlink_kernel_release(dev->nls)
kfree(cn) <--- cn is freed
<-- resumes -->
spin_lock_bh(&dev->cbdev->queue_lock)
*** use-after-free: dev = &cn->cdev,
cn and cbdev already kfree()d ***
Scenario 2 is exactly why a bare if (!dev) return in cn_call_callback() is not enough: it only fixes scenario 1. The
proposed patch takes rcu_read_lock() around the fetch plus the queue walk - then in scenario 2 CPU1 is inside an RCU
section, synchronize_rcu() on CPU0 has to wait for it and all the frees move past the end of the section; and in scenario
1 the NULL check kicks in.
=============================================================
suggested fix:
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -154,7 +154,7 @@ static int cn_call_callback(struct sk_buff *skb)
{
struct nlmsghdr *nlh;
struct cn_callback_entry *i, *cbq = NULL;
- struct cn_dev *dev = get_cdev(sock_net(skb->sk)->owner_ve);
+ struct cn_dev *dev;
struct cn_msg *msg = nlmsg_data(nlmsg_hdr(skb));
struct netlink_skb_parms *nsp = &NETLINK_CB(skb);
int err = -ENODEV;
@@ -164,6 +164,19 @@ static int cn_call_callback(struct sk_buff *skb)
if (nlh->nlmsg_len < NLMSG_HDRLEN + sizeof(struct cn_msg) + msg->len)
return -EINVAL;
+ /*
+ * The kernel socket outlives ve->cn on VE stop: cn_proc_fini_ve()
+ * clears the pointer and waits for a grace period long before
+ * cn_fini_ve() releases the socket, so a message sent from the
+ * host into the dying VE netns can still get here. Fetch ve->cn
+ * and walk the callback queue under rcu_read_lock(): once the
+ * pointer is observed, the grace period in cn_proc_fini_ve()
+ * keeps the callback device alive until we are done with it.
+ */
+ rcu_read_lock();
+ dev = get_cdev(sock_net(skb->sk)->owner_ve);
+ if (!dev) {
+ rcu_read_unlock();
+ return -ENODEV;
+ }
+
spin_lock_bh(&dev->cbdev->queue_lock);
list_for_each_entry(i, &dev->cbdev->queue_list, callback_entry) {
if (cn_cb_equal(&i->id.id, &msg->id)) {
@@ -173,6 +186,7 @@ static int cn_call_callback(struct sk_buff *skb)
}
}
spin_unlock_bh(&dev->cbdev->queue_lock);
+ rcu_read_unlock();
if (cbq != NULL) {
cbq->callback(msg, nsp);
> 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);
> }
>
>
More information about the Devel
mailing list