OpenVZ / Virtuozzo kernel development (devel@openvz.org)
 help / color / mirror / Atom feed
* [Devel] [PATCH vz10 v2] fs/kernfs, ve: pin the VE that created a kernfs instance
@ 2026-09-29 16:49 Konstantin Khorenko
  2026-09-29 16:51 ` Konstantin Khorenko
  2026-09-29 16:54 ` no-reply
  0 siblings, 2 replies; 3+ messages in thread
From: Konstantin Khorenko @ 2026-09-29 16:49 UTC (permalink / raw)
  To: devel, Konstantin Khorenko

kernfs_get_tree() stores a bare get_exec_env() pointer in
kernfs_super_info, and since commit 8d1abf587552 ("fs/kernfs, ve:
hide entries from a VE without invalidating the dentry")
kernfs_dop_revalidate() compares it against the caller's VE to decide
between dropping the dentry and answering -ENOENT.

Nothing backs that pointer with a reference: if the mount is pinned from
outside - the host bind-mounted a Container's sysfs, or holds an fd
inside it - the superblock outlives the Container, info->ve dangles, and
a later Container whose ve_struct is allocated at the same address is
mistaken for the creator of the instance. Its lookups then take the
d_invalidate() path on the dead instance and detach the mounts placed on
top of it.

Take a css reference on the VE for the lifetime of the superblock, so
the comparison always names the VE that really created the instance.

The assignment in kernfs_set_ve_perms() is dropped instead of
converted: it runs right after kernfs_get_tree() in the same mount
context, so it always rewrites the field with the same value.

Review notes:
 * Pasha: the stale match is hard to hit in practice, since a new VE
   gets its own netns and with it its own sysfs superblock and never
   looks at the old instance;
 * Mirian: the comparison in kernfs_dop_revalidate() (8d1abf587552) is
   the only reader of info->ve, dropping it would also close this.

Fixes: 8d1abf587552 ("fs/kernfs, ve: hide entries from a VE without invalidating the dentry")
https://virtuozzo.atlassian.net/browse/VSTOR-142552

Feature: kernfs: per-CT entries visibility and permissions configuration
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Reviewed-by: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
---
Changes in v2:
- commit message updated
- reviewed-by lines added
- Link to v1: http://lore.virtuozzo.com/20260928-prep-kh-vstor-142552-kernfs-pin-ve-v1-1-7297e159e813@virtuozzo.com
---
 fs/kernfs/mount.c | 13 ++++++++++---
 fs/kernfs/ve.c    |  1 -
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index 14a08ba6ae57d..d4bf659b5f8fe 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -18,6 +18,7 @@
 #include <linux/exportfs.h>
 #include <linux/uuid.h>
 #include <linux/statfs.h>
+#include <linux/ve.h>
 
 #include "kernfs-internal.h"
 #include "kernfs-ve.h"
@@ -362,7 +363,7 @@ int kernfs_get_tree(struct fs_context *fc)
 	info->root = kfc->root;
 	info->ns = kfc->ns_tag;
 	INIT_LIST_HEAD(&info->node);
-	info->ve = get_exec_env();
+	info->ve = get_ve(get_exec_env());
 
 	fc->s_fs_info = info;
 	sb = sget_fc(fc, kernfs_test_super, kernfs_set_super);
@@ -397,9 +398,14 @@ int kernfs_get_tree(struct fs_context *fc)
 
 void kernfs_free_fs_context(struct fs_context *fc)
 {
+	struct kernfs_super_info *info = fc->s_fs_info;
+
 	/* Note that we don't deal with kfc->ns_tag here. */
-	kfree(fc->s_fs_info);
-	fc->s_fs_info = NULL;
+	if (info) {
+		put_ve(info->ve);
+		kfree(info);
+		fc->s_fs_info = NULL;
+	}
 }
 
 /**
@@ -424,6 +430,7 @@ void kernfs_kill_sb(struct super_block *sb)
 	 * so we can't find it, before freeing kernfs_super_info.
 	 */
 	kill_anon_super(sb);
+	put_ve(info->ve);
 	kfree(info);
 }
 
diff --git a/fs/kernfs/ve.c b/fs/kernfs/ve.c
index 630ea34d1ccab..1097fd30598e3 100644
--- a/fs/kernfs/ve.c
+++ b/fs/kernfs/ve.c
@@ -28,7 +28,6 @@ void kernfs_set_ve_perms(struct dentry *root, off_t key_off)
 	struct kernfs_super_info *info = kernfs_info(root->d_sb);
 
 	info->ve_perms_off = key_off;
-	info->ve = get_exec_env();
 }
 
 int kernfs_init_ve_perms(struct kernfs_root *root,

---
base-commit: 0679f0d70523972eec3a0643565521d93e26eb9c
change-id: 20260928-prep-kh-vstor-142552-kernfs-pin-ve-db8e7eb89201

Best regards,
--  
Konstantin Khorenko <khorenko@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Devel] [PATCH vz10 v2] fs/kernfs, ve: pin the VE that created a kernfs instance
  2026-09-29 16:49 [Devel] [PATCH vz10 v2] fs/kernfs, ve: pin the VE that created a kernfs instance Konstantin Khorenko
@ 2026-09-29 16:51 ` Konstantin Khorenko
  2026-09-29 16:54 ` no-reply
  1 sibling, 0 replies; 3+ messages in thread
From: Konstantin Khorenko @ 2026-09-29 16:51 UTC (permalink / raw)
  To: devel, Konstantin Khorenko

On Tue, 29 Sep 2026 18:49:03 +0200, Konstantin Khorenko wrote:
> fs/kernfs, ve: pin the VE that created a kernfs instance

Applied to branch-rh10-6.12.0-211.52.1.22.x.vz10-ovz, will be released in rh10-6.12.0-211.52.1.22.5.vz10, thanks!

[1/1] fs/kernfs, ve: pin the VE that created a kernfs instance
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/c2eff611f078

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Devel] [PATCH vz10 v2] fs/kernfs, ve: pin the VE that created a kernfs instance
  2026-09-29 16:49 [Devel] [PATCH vz10 v2] fs/kernfs, ve: pin the VE that created a kernfs instance Konstantin Khorenko
  2026-09-29 16:51 ` Konstantin Khorenko
@ 2026-09-29 16:54 ` no-reply
  1 sibling, 0 replies; 3+ messages in thread
From: no-reply @ 2026-09-29 16:54 UTC (permalink / raw)
  To: devel, Konstantin Khorenko

> kernfs_get_tree() stores a bare get_exec_env() pointer in
> kernfs_super_info, and since commit 8d1abf587552 ("fs/kernfs, ve:
> hide entries from a VE without invalidating the dentry")
> kernfs_dop_revalidate() compares it against the caller's VE to decide
> between dropping the dentry and answering -ENOENT.
> 
> Nothing backs that pointer with a reference: if the mount is pinned from
> outside - the host bind-mounted a Container's sysfs, or holds an fd
> inside it - the superblock outlives the Container, info->ve dangles, and
> a later Container whose ve_struct is allocated at the same address is
> mistaken for the creator of the instance. Its lookups then take the
> d_invalidate() path on the dead instance and detach the mounts placed on
> top of it.
> 
> Take a css reference on the VE for the lifetime of the superblock, so
> [ ... ]
> Fixes: 8d1abf587552 ("fs/kernfs, ve: hide entries from a VE without invalidating the dentry")
> https://virtuozzo.atlassian.net/browse/VSTOR-142552
> 
> Feature: kernfs: per-CT entries visibility and permissions configuration
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-prep-kh-vstor-142552-kernfs-pin-ve-v2-1-0795e3aee679@virtuozzo.com?part=1


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-29 16:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-29 16:49 [Devel] [PATCH vz10 v2] fs/kernfs, ve: pin the VE that created a kernfs instance Konstantin Khorenko
2026-09-29 16:51 ` Konstantin Khorenko
2026-09-29 16:54 ` no-reply

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox