[Devel] [PATCH vz10 3/7] fs/kernfs, ve: lock the walked tree rwsem in kernfs_perms_start

[Pavel Tikhomirov]

On 6/28/26 11:26, Mirian Shilakadze wrote:
> kernfs_perms_start() takes kernfs_root(of->kn)->kernfs_rwsem to protect its
> walk, but of->kn is the cgroup file the seq sits on (ve.sysfs_permissions
> is a cgroup cftype), so that is the cgroup hierarchy rwsem, while the loop
> walks the sysfs tree (sysfs_root_kn), a different kernfs root. The sysfs
> tree walk and the ve_perms_map reads run under the wrong root rwsem and
> race concurrent sysfs node add and remove, a use-after-free.
> 
> Lock kernfs_root(root)->kernfs_rwsem, the tree actually being walked, and
> release the same lock in kernfs_perms_stop(), which now takes the root.

That totally makes sense to lock the sysfs lock, not cgroupfs one.

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

See small suggestion inline.

> 
> https://virtuozzo.atlassian.net/browse/VSTOR-136541
> Fixes: d0cb91d2dec2 ("ve/kernfs: fix kernfs locking in the control per-VE nodes visibility code")
> Signed-off-by: Mirian Shilakadze <mirian.shilakadze at virtuozzo.com>
> ---
>  fs/kernfs/ve.c            | 13 ++++---------
>  fs/sysfs/ve.c             |  2 +-
>  include/linux/kernfs-ve.h |  2 +-
>  3 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/kernfs/ve.c b/fs/kernfs/ve.c
> index f357ebb907f1..2858b7e97f6e 100644
> --- a/fs/kernfs/ve.c
> +++ b/fs/kernfs/ve.c
> @@ -157,9 +157,8 @@ void *kernfs_perms_start(struct seq_file *m, loff_t *ppos,
>  			 struct kernfs_node *root, struct kmapset_key *key)
>  {
>  	struct ve_struct *ve = css_to_ve(seq_css(m));
> -	struct kernfs_open_file *of = m->private;
> -	struct kernfs_node *kn = of->kn;
> -	struct kernfs_root *root_for_sem = kernfs_root(kn);
> +	struct kernfs_node *kn;
> +	struct kernfs_root *root_for_sem = kernfs_root(root);
>  	loff_t pos = *ppos;
>  
>  	down_read(&root_for_sem->kernfs_rwsem);

Maybe just:

down_read(&kernfs_root(root)->kernfs_rwsem);

Variable root_for_sem seems excess.

> @@ -184,13 +183,9 @@ void *kernfs_perms_next(struct seq_file *m, void *v, loff_t *ppos,
>  	return kn;
>  }
>  
> -void kernfs_perms_stop(struct seq_file *m, void *v)
> +void kernfs_perms_stop(struct seq_file *m, void *v, struct kernfs_node *root)
>  {
> -	struct kernfs_open_file *of = m->private;
> -	struct kernfs_node *kn = of->kn;
> -	struct kernfs_root *root = kernfs_root(kn);
> -
> -	up_read(&root->kernfs_rwsem);
> +	up_read(&kernfs_root(root)->kernfs_rwsem);
>  }
>  
>  int kernfs_perms_show(struct seq_file *m, void *v, struct kmapset_key *key)
> diff --git a/fs/sysfs/ve.c b/fs/sysfs/ve.c
> index eb941a1d5dd5..144a901547c0 100644
> --- a/fs/sysfs/ve.c
> +++ b/fs/sysfs/ve.c
> @@ -55,7 +55,7 @@ static void *sysfs_perms_next(struct seq_file *m, void *v, loff_t *ppos)
>  
>  static void sysfs_perms_stop(struct seq_file *m, void *v)
>  {
> -	kernfs_perms_stop(m, v);
> +	kernfs_perms_stop(m, v, sysfs_root_kn);
>  	mutex_unlock(&sysfs_perms_mutex);
>  }
>  
> diff --git a/include/linux/kernfs-ve.h b/include/linux/kernfs-ve.h
> index 2cb905918393..25837ff79e5b 100644
> --- a/include/linux/kernfs-ve.h
> +++ b/include/linux/kernfs-ve.h
> @@ -24,7 +24,7 @@ void *kernfs_perms_start(struct seq_file *m, loff_t *ppos,
>  			 struct kernfs_node *root, struct kmapset_key *key);
>  void *kernfs_perms_next(struct seq_file *m, void *v, loff_t *ppos,
>  			      struct kmapset_key *key);
> -void kernfs_perms_stop(struct seq_file *m, void *v);
> +void kernfs_perms_stop(struct seq_file *m, void *v, struct kernfs_node *root);
>  
>  int kernfs_perms_show(struct seq_file *m, void *v, struct kmapset_key *key);
>  

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