[Devel] [PATCH vz10 v2 1/2] fs/kernfs, ve: hide entries from a VE without invalidating the dentry

[Konstantin Khorenko]

On 8/26/26 13:04, Mirian Shilakadze wrote:
...
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index be680eb98ed4..4a5ee299a94e 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -1199,8 +1199,21 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
>  	    kernfs_info(dentry->d_sb)->ns != kn->ns)
>  		goto out_bad;
>  
> -	if (!kernfs_d_visible(kn, kernfs_info(dentry->d_sb)))
> -		goto out_bad;
> +	if (!kernfs_d_visible(kn, kernfs_info(dentry->d_sb))) {
> +		/*
> +		 * On an instance this VE created, drop the dentry as before.
> +		 * Anywhere else the node is fine and is only outside this
> +		 * VE's view: returning 0 would tell the VFS that the dentry
> +		 * is stale, and it answers that with d_invalidate(), which
> +		 * detaches every mount on that dentry in every mount
> +		 * namespace.  Report the name as missing to this caller
> +		 * instead.
> +		 */
> +		if (kernfs_info(dentry->d_sb)->ve == get_exec_env())

kernfs of a VE may outlive the VE (and ve_struct) - if kernefs of a CT is pinned somehow while CT is stopped,
there is no get_ve() or any other blocker for a Container destruction before VE kernfs is unmounted.

And in case the VE1 died and VE2 is started and occasionally ve_struct got VE2 gets the same pointer,
then the comparison will tell us not truth.

=======
  What is stored in info->ve

  When some VE mounts its own kernfs instance (for example, a container mounts its own sysfs), kernfs_get_tree() creates a
  new superblock and a pointer to the mounting task's VE is written into its kernfs_super_info (fs/kernfs/mount.c:365):

  info->ve = get_exec_env();

  The key point: this is a raw pointer. There is no get_ve(), no taking of any other reference there. That is, the
  superblock does not in any way extend the lifetime of the ve_struct it points to. This was the case before the patch too -
  patch 1 merely added a second place where this pointer is used (the comparison in kernfs_dop_revalidate()).

  How a superblock can outlive its VE

  Normally a container's sysfs is unmounted when the container stops, and the sb dies together with it. But the mount can be
  "pinned" from outside: for example, the host bind-mounted the container's sysfs somewhere into its own mount namespace,
  or some process holds an open fd inside that mount. Then, after the container is stopped and destroyed, its ve_struct is
  freed (kfree), while the superblock, with info->ve pointing at already freed memory, lives on.

  By itself this is not a use-after-free: the pointer is never dereferenced anywhere, it is only compared with
  get_exec_env(). Comparing addresses with a freed object is legal - nobody tries to read through the address.

  Where the trap appears: address reuse

  kfree() returns the memory to the allocator, and the next kmalloc() of the same size may well hand out the very same
  address. A new container starts - its ve_struct may end up at the address where the dead container's ve_struct used to
  live.

  Now the scenario in full:

  1. Container A mounts its own sysfs -> an sb with info->ve = <address X>.
  2. The host pinned that mount, container A is destroyed, the ve_struct at address X is freed. The sb is alive, info->ve =
     X (dangling).
  3. Container B starts, its ve_struct is allocated at the same address X.
  4. A task of container B, by some path (through that host-pinned mount), does a lookup of an entry hidden from it on this
     old sb.
  5. The check kernfs_info(dentry->d_sb)->ve == get_exec_env() gives X == X -> true, even though container B did not create
     this instance. The code wrongly decides "this is this VE's own instance" and takes the old behavior: goto out_bad ->
     return 0 -> d_invalidate() instead of the new -ENOENT.

  Why I don't consider this a blocker

  The consequence of the false match is merely a fallback to today's (pre-patch) behavior: the dentry is invalidated and the
  mounts on that dentry are detached. But the dentry belongs to the sysfs instance of dead container A, that is, the only
  things that can suffer are mounts placed on top of entries of that dead instance - not the host's /sys and not container
  B's sysfs. Plus the scenario itself requires exotics: the host is for some reason holding a dead container's sysfs, the
  new VE landed at exactly the same address, and its task is walking that foreign mount.

  The reverse direction is safe automatically: "didn't match although it should have" is impossible, because the VE that
  actually created the instance is dead - it has no tasks left, there is nothing to compare against. The only possible
  failure is a false match, and, as shown above, it degrades into the old behavior, not into a new hole.

  That is why the wording was "worth being aware": the patch makes a long-existing weak spot (a pointer not backed by a
  reference) matter for the logic (load-bearing) for the first time, but no real harm can be extracted from it. If one
  wanted to close the question nicely, one could store not the pointer but, say, ve->veid, or take a get_ve() reference when
  setting info->ve and drop it in kernfs_free_fs_context/on sb destruction. But I would not demand that from this series.


> +			goto out_bad;
> +		up_read(&root->kernfs_rwsem);
> +		return -ENOENT;
> +	}
>  
>  	up_read(&root->kernfs_rwsem);
>  	return 1;