[Devel] [PATCH vz10 1/5] fixup! cgroup: use proper lockdep condition for ve_nsproxy dereference

[Pavel Tikhomirov]

It's excess and can actually be confusing. Only init of root container
pidns ever updates ve_nsproxy, so we don't need ANY lock to access it
if we are this init of root container pidns.

So actually we can temporarily release op_sem around access to ve_nsproxy
and it would still be safe. That is exactly what unconditional "1" here
means.

Nacked-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

On 6/25/26 20:16, Eva Kurchatova wrote:
> From: Konstantin Khorenko <khorenko at virtuozzo.com>
> 
> 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.
> 
> 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.
> 
> 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 5585545304a3..0bd3aff89dbd 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -2252,7 +2252,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) ||
> @@ -2287,7 +2288,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.