[Devel] [PATCH vz10] cgroup: use proper lockdep condition for ve_nsproxy dereference

[Pavel Tikhomirov]

On 4/9/26 09:37, Konstantin Khorenko wrote:
> cgroup_join_vz_slice() and cgroup_leave_vz_slice() dereference
> ve->ve_nsproxy with rcu_dereference_protected(..., 1), which
> unconditionally silences lockdep without naming the actual lock that
> makes the access safe.
> 
> Both functions are only called from ve_start_container(), which runs
> under ve->op_sem write-lock. The pointer is stable because
> ve_grab_context() publishes it via rcu_assign_pointer() earlier in the
> same function, and ve_drop_context() - the only path that clears it -
> cannot run concurrently since it also requires op_sem.

My original logic was that ve->ve_nsproxy remains stable no matter the lock.

It's because in ve_start_container->{cgroup_join_vz_slice,cgroup_leave_vz_slice} we are the init of container, so we are the only process which can really now change ve->ve_nsproxy and we know we don't.

> 
> Replace the unconditional "1" with lockdep_is_held(&ve->op_sem) so
> that lockdep can actually verify the locking at runtime. This is
> consistent with how ve_grab_context(), ve_drop_context(), and
> ve_stop_ns() already annotate the same pointer.

In this case we are actually already protected by ve_grab_context/ve_drop_context no matter the lock.

But yes, adding lockdep does not brake anything since we also have the lock held, and is a more simple logic. We can maybe add a comment to indicate that lockdep is not strictly required.

> 
> Fixes: f74e12f9f249a ("ve/cgroup: add vz.slice cgroup to put kernel threads to")
> https://virtuozzo.atlassian.net/browse/VSTOR-128317
> 
> Feature: ve: ve generic structures
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>  kernel/cgroup/cgroup.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index 376c9bbd1c41e..57e86306e9cb3 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -2242,7 +2242,8 @@ int cgroup_join_vz_slice(struct ve_struct *ve)
>  	struct cgroup *cgrp;
>  	int ret;
>  
> -	cset = rcu_dereference_protected(ve->ve_nsproxy, 1)->cgroup_ns->root_cset;
> +	cset = rcu_dereference_protected(ve->ve_nsproxy,
> +				lockdep_is_held(&ve->op_sem))->cgroup_ns->root_cset;
>  	cgrp = cset_cgroup_from_root(cset, &cgrp_dfl_root);
>  
>  	if (!is_virtualized_cgroup(cgrp) ||
> @@ -2277,7 +2278,8 @@ int cgroup_leave_vz_slice(struct ve_struct *ve)
>  	struct css_set *cset;
>  	struct cgroup *cgrp;
>  
> -	cset = rcu_dereference_protected(ve->ve_nsproxy, 1)->cgroup_ns->root_cset;
> +	cset = rcu_dereference_protected(ve->ve_nsproxy,
> +				lockdep_is_held(&ve->op_sem))->cgroup_ns->root_cset;
>  	cgrp = cset_cgroup_from_root(cset, &cgrp_dfl_root);
>  
>  	if (!is_virtualized_cgroup(cgrp) ||

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.