linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][RFC][CFT] kill vfs_submount(), already
@ 2025-05-03 21:29 Al Viro
  2025-05-05 10:55 ` Jan Kara
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Al Viro @ 2025-05-03 21:29 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Steven Rostedt, linux-trace-kernel, Christian Brauner, Jan Kara

The last remaining user of vfs_submount() (tracefs) is easy to convert
to fs_context_for_submount(); do that and bury that thing, along with
SB_SUBMOUNT

If nobody objects, I'm going to throw that into the mount-related pile;
alternatively, that could be split into kernel/trace.c part (in invariant
branch, to be pulled by tracefs folks and into the mount pile before
the rest of the patch).  Preferences?

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/namespace.c b/fs/namespace.c
index 07f636036b86..293e6f925eff 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1297,21 +1297,6 @@ struct vfsmount *vfs_kern_mount(struct file_system_type *type,
 }
 EXPORT_SYMBOL_GPL(vfs_kern_mount);
 
-struct vfsmount *
-vfs_submount(const struct dentry *mountpoint, struct file_system_type *type,
-	     const char *name, void *data)
-{
-	/* Until it is worked out how to pass the user namespace
-	 * through from the parent mount to the submount don't support
-	 * unprivileged mounts with submounts.
-	 */
-	if (mountpoint->d_sb->s_user_ns != &init_user_ns)
-		return ERR_PTR(-EPERM);
-
-	return vfs_kern_mount(type, SB_SUBMOUNT, name, data);
-}
-EXPORT_SYMBOL_GPL(vfs_submount);
-
 static struct mount *clone_mnt(struct mount *old, struct dentry *root,
 					int flag)
 {
diff --git a/fs/super.c b/fs/super.c
index 97a17f9d9023..1886e4c930e0 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -823,13 +823,6 @@ struct super_block *sget(struct file_system_type *type,
 	struct super_block *old;
 	int err;
 
-	/* We don't yet pass the user namespace of the parent
-	 * mount through to here so always use &init_user_ns
-	 * until that changes.
-	 */
-	if (flags & SB_SUBMOUNT)
-		user_ns = &init_user_ns;
-
 retry:
 	spin_lock(&sb_lock);
 	if (test) {
@@ -849,7 +842,7 @@ struct super_block *sget(struct file_system_type *type,
 	}
 	if (!s) {
 		spin_unlock(&sb_lock);
-		s = alloc_super(type, (flags & ~SB_SUBMOUNT), user_ns);
+		s = alloc_super(type, flags, user_ns);
 		if (!s)
 			return ERR_PTR(-ENOMEM);
 		goto retry;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 016b0fe1536e..515e702d98ae 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1240,7 +1240,6 @@ extern int send_sigurg(struct file *file);
 /* These sb flags are internal to the kernel */
 #define SB_DEAD         BIT(21)
 #define SB_DYING        BIT(24)
-#define SB_SUBMOUNT     BIT(26)
 #define SB_FORCE        BIT(27)
 #define SB_NOSEC        BIT(28)
 #define SB_BORN         BIT(29)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index fa488721019f..7b6248ba4428 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -51,6 +51,7 @@
 #include <linux/workqueue.h>
 #include <linux/sort.h>
 #include <linux/io.h> /* vmap_page_range() */
+#include <linux/fs_context.h>
 
 #include <asm/setup.h> /* COMMAND_LINE_SIZE */
 
@@ -10072,6 +10073,8 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
 {
 	struct vfsmount *mnt;
 	struct file_system_type *type;
+	struct fs_context *fc;
+	int ret;
 
 	/*
 	 * To maintain backward compatibility for tools that mount
@@ -10081,10 +10084,20 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
 	type = get_fs_type("tracefs");
 	if (!type)
 		return NULL;
-	mnt = vfs_submount(mntpt, type, "tracefs", NULL);
+
+	fc = fs_context_for_submount(type, mntpt);
+	if (IS_ERR(fc))
+		return ERR_CAST(fc);
+
+	ret = vfs_parse_fs_string(fc, "source",
+				  "tracefs", strlen("tracefs"));
+	if (!ret)
+		mnt = fc_mount(fc);
+	else
+		mnt = ERR_PTR(ret);
+
+	put_fs_context(fc);
 	put_filesystem(type);
-	if (IS_ERR(mnt))
-		return NULL;
 	return mnt;
 }
 

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

* Re: [PATCH][RFC][CFT] kill vfs_submount(), already
  2025-05-03 21:29 [PATCH][RFC][CFT] kill vfs_submount(), already Al Viro
@ 2025-05-05 10:55 ` Jan Kara
  2025-05-05 21:38   ` [PATCH v2] kill vfs_submount() Al Viro
  2025-05-05 13:57 ` [PATCH][RFC][CFT] kill vfs_submount(), already Christian Brauner
  2025-05-05 14:15 ` Steven Rostedt
  2 siblings, 1 reply; 8+ messages in thread
From: Jan Kara @ 2025-05-05 10:55 UTC (permalink / raw)
  To: Al Viro
  Cc: linux-fsdevel, Steven Rostedt, linux-trace-kernel,
	Christian Brauner, Jan Kara

On Sat 03-05-25 22:29:25, Al Viro wrote:
> The last remaining user of vfs_submount() (tracefs) is easy to convert
> to fs_context_for_submount(); do that and bury that thing, along with
> SB_SUBMOUNT
> 
> If nobody objects, I'm going to throw that into the mount-related pile;
> alternatively, that could be split into kernel/trace.c part (in invariant
> branch, to be pulled by tracefs folks and into the mount pile before
> the rest of the patch).  Preferences?
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

...

> @@ -10072,6 +10073,8 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
>  {
>  	struct vfsmount *mnt;
>  	struct file_system_type *type;
> +	struct fs_context *fc;
> +	int ret;
>  
>  	/*
>  	 * To maintain backward compatibility for tools that mount
> @@ -10081,10 +10084,20 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
>  	type = get_fs_type("tracefs");
>  	if (!type)
>  		return NULL;
> -	mnt = vfs_submount(mntpt, type, "tracefs", NULL);
> +
> +	fc = fs_context_for_submount(type, mntpt);
> +	if (IS_ERR(fc))
> +		return ERR_CAST(fc);

Missing put_filesystem() here?

> +
> +	ret = vfs_parse_fs_string(fc, "source",
> +				  "tracefs", strlen("tracefs"));
> +	if (!ret)
> +		mnt = fc_mount(fc);
> +	else
> +		mnt = ERR_PTR(ret);
> +
> +	put_fs_context(fc);
>  	put_filesystem(type);
> -	if (IS_ERR(mnt))
> -		return NULL;
>  	return mnt;
>  }

Otherwise looks good so with the fixup feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>
								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

* Re: [PATCH][RFC][CFT] kill vfs_submount(), already
  2025-05-03 21:29 [PATCH][RFC][CFT] kill vfs_submount(), already Al Viro
  2025-05-05 10:55 ` Jan Kara
@ 2025-05-05 13:57 ` Christian Brauner
  2025-05-05 14:15 ` Steven Rostedt
  2 siblings, 0 replies; 8+ messages in thread
From: Christian Brauner @ 2025-05-05 13:57 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, Steven Rostedt, linux-trace-kernel, Jan Kara

On Sat, May 03, 2025 at 10:29:25PM +0100, Al Viro wrote:
> The last remaining user of vfs_submount() (tracefs) is easy to convert
> to fs_context_for_submount(); do that and bury that thing, along with
> SB_SUBMOUNT
> 
> If nobody objects, I'm going to throw that into the mount-related pile;
> alternatively, that could be split into kernel/trace.c part (in invariant
> branch, to be pulled by tracefs folks and into the mount pile before
> the rest of the patch).  Preferences?
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Looks good,
Reviewed-by: Christian Brauner <brauner@kernel.org>

> diff --git a/fs/namespace.c b/fs/namespace.c
> index 07f636036b86..293e6f925eff 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1297,21 +1297,6 @@ struct vfsmount *vfs_kern_mount(struct file_system_type *type,
>  }
>  EXPORT_SYMBOL_GPL(vfs_kern_mount);
>  
> -struct vfsmount *
> -vfs_submount(const struct dentry *mountpoint, struct file_system_type *type,
> -	     const char *name, void *data)
> -{
> -	/* Until it is worked out how to pass the user namespace
> -	 * through from the parent mount to the submount don't support
> -	 * unprivileged mounts with submounts.
> -	 */
> -	if (mountpoint->d_sb->s_user_ns != &init_user_ns)
> -		return ERR_PTR(-EPERM);
> -
> -	return vfs_kern_mount(type, SB_SUBMOUNT, name, data);
> -}
> -EXPORT_SYMBOL_GPL(vfs_submount);
> -
>  static struct mount *clone_mnt(struct mount *old, struct dentry *root,
>  					int flag)
>  {
> diff --git a/fs/super.c b/fs/super.c
> index 97a17f9d9023..1886e4c930e0 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -823,13 +823,6 @@ struct super_block *sget(struct file_system_type *type,
>  	struct super_block *old;
>  	int err;
>  
> -	/* We don't yet pass the user namespace of the parent
> -	 * mount through to here so always use &init_user_ns
> -	 * until that changes.
> -	 */
> -	if (flags & SB_SUBMOUNT)
> -		user_ns = &init_user_ns;

Oh thank god this disgusting hack is finally gone.

> -
>  retry:
>  	spin_lock(&sb_lock);
>  	if (test) {
> @@ -849,7 +842,7 @@ struct super_block *sget(struct file_system_type *type,
>  	}
>  	if (!s) {
>  		spin_unlock(&sb_lock);
> -		s = alloc_super(type, (flags & ~SB_SUBMOUNT), user_ns);
> +		s = alloc_super(type, flags, user_ns);
>  		if (!s)
>  			return ERR_PTR(-ENOMEM);
>  		goto retry;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 016b0fe1536e..515e702d98ae 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1240,7 +1240,6 @@ extern int send_sigurg(struct file *file);
>  /* These sb flags are internal to the kernel */
>  #define SB_DEAD         BIT(21)
>  #define SB_DYING        BIT(24)
> -#define SB_SUBMOUNT     BIT(26)
>  #define SB_FORCE        BIT(27)
>  #define SB_NOSEC        BIT(28)
>  #define SB_BORN         BIT(29)
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index fa488721019f..7b6248ba4428 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -51,6 +51,7 @@
>  #include <linux/workqueue.h>
>  #include <linux/sort.h>
>  #include <linux/io.h> /* vmap_page_range() */
> +#include <linux/fs_context.h>
>  
>  #include <asm/setup.h> /* COMMAND_LINE_SIZE */
>  
> @@ -10072,6 +10073,8 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
>  {
>  	struct vfsmount *mnt;
>  	struct file_system_type *type;
> +	struct fs_context *fc;
> +	int ret;
>  
>  	/*
>  	 * To maintain backward compatibility for tools that mount
> @@ -10081,10 +10084,20 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
>  	type = get_fs_type("tracefs");
>  	if (!type)
>  		return NULL;
> -	mnt = vfs_submount(mntpt, type, "tracefs", NULL);
> +
> +	fc = fs_context_for_submount(type, mntpt);
> +	if (IS_ERR(fc))
> +		return ERR_CAST(fc);
> +
> +	ret = vfs_parse_fs_string(fc, "source",
> +				  "tracefs", strlen("tracefs"));
> +	if (!ret)
> +		mnt = fc_mount(fc);
> +	else
> +		mnt = ERR_PTR(ret);
> +
> +	put_fs_context(fc);
>  	put_filesystem(type);
> -	if (IS_ERR(mnt))
> -		return NULL;
>  	return mnt;
>  }
>  

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

* Re: [PATCH][RFC][CFT] kill vfs_submount(), already
  2025-05-03 21:29 [PATCH][RFC][CFT] kill vfs_submount(), already Al Viro
  2025-05-05 10:55 ` Jan Kara
  2025-05-05 13:57 ` [PATCH][RFC][CFT] kill vfs_submount(), already Christian Brauner
@ 2025-05-05 14:15 ` Steven Rostedt
  2 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2025-05-05 14:15 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, linux-trace-kernel, Christian Brauner, Jan Kara

On Sat, 3 May 2025 22:29:25 +0100
Al Viro <viro@zeniv.linux.org.uk> wrote:

> @@ -10081,10 +10084,20 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
>  	type = get_fs_type("tracefs");
>  	if (!type)
>  		return NULL;
> -	mnt = vfs_submount(mntpt, type, "tracefs", NULL);
> +
> +	fc = fs_context_for_submount(type, mntpt);
> +	if (IS_ERR(fc))
> +		return ERR_CAST(fc);

As Jan mentioned, the put_filesystem(type) is need in the error path.

> +
> +	ret = vfs_parse_fs_string(fc, "source",
> +				  "tracefs", strlen("tracefs"));
> +	if (!ret)
> +		mnt = fc_mount(fc);
> +	else
> +		mnt = ERR_PTR(ret);
> +
> +	put_fs_context(fc);
>  	put_filesystem(type);
> -	if (IS_ERR(mnt))
> -		return NULL;

This didn't apply cleanly to Linus's tree, nor linux-next, due to missing:

	mntget(mnt);

Was that supposed to be deleted too?

Anyway, I applied this (still keeping the mntget()) and it appears to work.

-- Steve


>  	return mnt;
>  }

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

* [PATCH v2] kill vfs_submount()
  2025-05-05 10:55 ` Jan Kara
@ 2025-05-05 21:38   ` Al Viro
  2025-05-05 22:00     ` Al Viro
                       ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Al Viro @ 2025-05-05 21:38 UTC (permalink / raw)
  To: Jan Kara
  Cc: linux-fsdevel, Steven Rostedt, linux-trace-kernel,
	Christian Brauner

On Mon, May 05, 2025 at 12:55:34PM +0200, Jan Kara wrote:
> >  	if (!type)
> >  		return NULL;
> > -	mnt = vfs_submount(mntpt, type, "tracefs", NULL);
> > +
> > +	fc = fs_context_for_submount(type, mntpt);
> > +	if (IS_ERR(fc))
> > +		return ERR_CAST(fc);
> 
> Missing put_filesystem() here?

Actually, I'd rather have it done unconditionally right after
fc_context_for_submount() - fs_context allocation grabs
a reference and it's held until put_fs_context, so...

[PATCH] kill vfs_submount()
    
The last remaining user of vfs_submount() (tracefs) is easy to convert
to fs_context_for_submount(); do that and bury that thing, along with
SB_SUBMOUNT

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/namespace.c b/fs/namespace.c
index 018e95fe5459..0577a9fb6050 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1329,21 +1329,6 @@ struct vfsmount *vfs_kern_mount(struct file_system_type *type,
 }
 EXPORT_SYMBOL_GPL(vfs_kern_mount);
 
-struct vfsmount *
-vfs_submount(const struct dentry *mountpoint, struct file_system_type *type,
-	     const char *name, void *data)
-{
-	/* Until it is worked out how to pass the user namespace
-	 * through from the parent mount to the submount don't support
-	 * unprivileged mounts with submounts.
-	 */
-	if (mountpoint->d_sb->s_user_ns != &init_user_ns)
-		return ERR_PTR(-EPERM);
-
-	return vfs_kern_mount(type, SB_SUBMOUNT, name, data);
-}
-EXPORT_SYMBOL_GPL(vfs_submount);
-
 static struct mount *clone_mnt(struct mount *old, struct dentry *root,
 					int flag)
 {
diff --git a/fs/super.c b/fs/super.c
index 97a17f9d9023..1886e4c930e0 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -823,13 +823,6 @@ struct super_block *sget(struct file_system_type *type,
 	struct super_block *old;
 	int err;
 
-	/* We don't yet pass the user namespace of the parent
-	 * mount through to here so always use &init_user_ns
-	 * until that changes.
-	 */
-	if (flags & SB_SUBMOUNT)
-		user_ns = &init_user_ns;
-
 retry:
 	spin_lock(&sb_lock);
 	if (test) {
@@ -849,7 +842,7 @@ struct super_block *sget(struct file_system_type *type,
 	}
 	if (!s) {
 		spin_unlock(&sb_lock);
-		s = alloc_super(type, (flags & ~SB_SUBMOUNT), user_ns);
+		s = alloc_super(type, flags, user_ns);
 		if (!s)
 			return ERR_PTR(-ENOMEM);
 		goto retry;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 016b0fe1536e..515e702d98ae 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1240,7 +1240,6 @@ extern int send_sigurg(struct file *file);
 /* These sb flags are internal to the kernel */
 #define SB_DEAD         BIT(21)
 #define SB_DYING        BIT(24)
-#define SB_SUBMOUNT     BIT(26)
 #define SB_FORCE        BIT(27)
 #define SB_NOSEC        BIT(28)
 #define SB_BORN         BIT(29)
diff --git a/include/linux/mount.h b/include/linux/mount.h
index dcc17ce8a959..d4eb90a367af 100644
--- a/include/linux/mount.h
+++ b/include/linux/mount.h
@@ -98,9 +98,6 @@ extern struct vfsmount *vfs_create_mount(struct fs_context *fc);
 extern struct vfsmount *vfs_kern_mount(struct file_system_type *type,
 				      int flags, const char *name,
 				      void *data);
-extern struct vfsmount *vfs_submount(const struct dentry *mountpoint,
-				     struct file_system_type *type,
-				     const char *name, void *data);
 
 extern void mnt_set_expiry(struct vfsmount *mnt, struct list_head *expiry_list);
 extern void mark_mounts_for_expiry(struct list_head *mounts);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index e22aacb0028a..936a615e8c56 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -51,6 +51,7 @@
 #include <linux/workqueue.h>
 #include <linux/sort.h>
 #include <linux/io.h> /* vmap_page_range() */
+#include <linux/fs_context.h>
 
 #include <asm/setup.h> /* COMMAND_LINE_SIZE */
 
@@ -10075,6 +10076,8 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
 {
 	struct vfsmount *mnt;
 	struct file_system_type *type;
+	struct fs_context *fc;
+	int ret;
 
 	/*
 	 * To maintain backward compatibility for tools that mount
@@ -10084,10 +10087,20 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
 	type = get_fs_type("tracefs");
 	if (!type)
 		return NULL;
-	mnt = vfs_submount(mntpt, type, "tracefs", NULL);
+
+	fc = fs_context_for_submount(type, mntpt);
 	put_filesystem(type);
-	if (IS_ERR(mnt))
-		return NULL;
+	if (IS_ERR(fc))
+		return ERR_CAST(fc);
+
+	ret = vfs_parse_fs_string(fc, "source",
+				  "tracefs", strlen("tracefs"));
+	if (!ret)
+		mnt = fc_mount(fc);
+	else
+		mnt = ERR_PTR(ret);
+
+	put_fs_context(fc);
 	return mnt;
 }
 

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

* Re: [PATCH v2] kill vfs_submount()
  2025-05-05 21:38   ` [PATCH v2] kill vfs_submount() Al Viro
@ 2025-05-05 22:00     ` Al Viro
  2025-05-06  0:19     ` Steven Rostedt
  2025-05-06 10:57     ` Jan Kara
  2 siblings, 0 replies; 8+ messages in thread
From: Al Viro @ 2025-05-05 22:00 UTC (permalink / raw)
  To: Jan Kara
  Cc: linux-fsdevel, Steven Rostedt, linux-trace-kernel,
	Christian Brauner

On Mon, May 05, 2025 at 10:38:29PM +0100, Al Viro wrote:
> On Mon, May 05, 2025 at 12:55:34PM +0200, Jan Kara wrote:
> > >  	if (!type)
> > >  		return NULL;
> > > -	mnt = vfs_submount(mntpt, type, "tracefs", NULL);
> > > +
> > > +	fc = fs_context_for_submount(type, mntpt);
> > > +	if (IS_ERR(fc))
> > > +		return ERR_CAST(fc);
> > 
> > Missing put_filesystem() here?
> 
> Actually, I'd rather have it done unconditionally right after
> fc_context_for_submount() - fs_context allocation grabs
> a reference and it's held until put_fs_context, so...

Just in case - that stuff is still on top of ->d_automount() calling
conventions change; see viro/vfs.git#work.automount for both.

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

* Re: [PATCH v2] kill vfs_submount()
  2025-05-05 21:38   ` [PATCH v2] kill vfs_submount() Al Viro
  2025-05-05 22:00     ` Al Viro
@ 2025-05-06  0:19     ` Steven Rostedt
  2025-05-06 10:57     ` Jan Kara
  2 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2025-05-06  0:19 UTC (permalink / raw)
  To: Al Viro; +Cc: Jan Kara, linux-fsdevel, linux-trace-kernel, Christian Brauner

On Mon, 5 May 2025 22:38:29 +0100
Al Viro <viro@zeniv.linux.org.uk> wrote:

> @@ -10084,10 +10087,20 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
>  	type = get_fs_type("tracefs");
>  	if (!type)
>  		return NULL;
> -	mnt = vfs_submount(mntpt, type, "tracefs", NULL);
> +
> +	fc = fs_context_for_submount(type, mntpt);
>  	put_filesystem(type);
> -	if (IS_ERR(mnt))
> -		return NULL;
> +	if (IS_ERR(fc))
> +		return ERR_CAST(fc);
> +
> +	ret = vfs_parse_fs_string(fc, "source",
> +				  "tracefs", strlen("tracefs"));
> +	if (!ret)
> +		mnt = fc_mount(fc);
> +	else
> +		mnt = ERR_PTR(ret);
> +
> +	put_fs_context(fc);
>  	return mnt;
>  }
>  

In case others try to apply this on Linus or linux-next, this is based on top of:

  https://lore.kernel.org/all/20250424060845.GG2023217@ZenIV/

Otherwise it doesn't apply cleanly.

Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Tested-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve

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

* Re: [PATCH v2] kill vfs_submount()
  2025-05-05 21:38   ` [PATCH v2] kill vfs_submount() Al Viro
  2025-05-05 22:00     ` Al Viro
  2025-05-06  0:19     ` Steven Rostedt
@ 2025-05-06 10:57     ` Jan Kara
  2 siblings, 0 replies; 8+ messages in thread
From: Jan Kara @ 2025-05-06 10:57 UTC (permalink / raw)
  To: Al Viro
  Cc: Jan Kara, linux-fsdevel, Steven Rostedt, linux-trace-kernel,
	Christian Brauner

On Mon 05-05-25 22:38:29, Al Viro wrote:
> On Mon, May 05, 2025 at 12:55:34PM +0200, Jan Kara wrote:
> > >  	if (!type)
> > >  		return NULL;
> > > -	mnt = vfs_submount(mntpt, type, "tracefs", NULL);
> > > +
> > > +	fc = fs_context_for_submount(type, mntpt);
> > > +	if (IS_ERR(fc))
> > > +		return ERR_CAST(fc);
> > 
> > Missing put_filesystem() here?
> 
> Actually, I'd rather have it done unconditionally right after
> fc_context_for_submount() - fs_context allocation grabs
> a reference and it's held until put_fs_context, so...
> 
> [PATCH] kill vfs_submount()
>     
> The last remaining user of vfs_submount() (tracefs) is easy to convert
> to fs_context_for_submount(); do that and bury that thing, along with
> SB_SUBMOUNT
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Indeed this works as well. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 018e95fe5459..0577a9fb6050 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -1329,21 +1329,6 @@ struct vfsmount *vfs_kern_mount(struct file_system_type *type,
>  }
>  EXPORT_SYMBOL_GPL(vfs_kern_mount);
>  
> -struct vfsmount *
> -vfs_submount(const struct dentry *mountpoint, struct file_system_type *type,
> -	     const char *name, void *data)
> -{
> -	/* Until it is worked out how to pass the user namespace
> -	 * through from the parent mount to the submount don't support
> -	 * unprivileged mounts with submounts.
> -	 */
> -	if (mountpoint->d_sb->s_user_ns != &init_user_ns)
> -		return ERR_PTR(-EPERM);
> -
> -	return vfs_kern_mount(type, SB_SUBMOUNT, name, data);
> -}
> -EXPORT_SYMBOL_GPL(vfs_submount);
> -
>  static struct mount *clone_mnt(struct mount *old, struct dentry *root,
>  					int flag)
>  {
> diff --git a/fs/super.c b/fs/super.c
> index 97a17f9d9023..1886e4c930e0 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -823,13 +823,6 @@ struct super_block *sget(struct file_system_type *type,
>  	struct super_block *old;
>  	int err;
>  
> -	/* We don't yet pass the user namespace of the parent
> -	 * mount through to here so always use &init_user_ns
> -	 * until that changes.
> -	 */
> -	if (flags & SB_SUBMOUNT)
> -		user_ns = &init_user_ns;
> -
>  retry:
>  	spin_lock(&sb_lock);
>  	if (test) {
> @@ -849,7 +842,7 @@ struct super_block *sget(struct file_system_type *type,
>  	}
>  	if (!s) {
>  		spin_unlock(&sb_lock);
> -		s = alloc_super(type, (flags & ~SB_SUBMOUNT), user_ns);
> +		s = alloc_super(type, flags, user_ns);
>  		if (!s)
>  			return ERR_PTR(-ENOMEM);
>  		goto retry;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 016b0fe1536e..515e702d98ae 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1240,7 +1240,6 @@ extern int send_sigurg(struct file *file);
>  /* These sb flags are internal to the kernel */
>  #define SB_DEAD         BIT(21)
>  #define SB_DYING        BIT(24)
> -#define SB_SUBMOUNT     BIT(26)
>  #define SB_FORCE        BIT(27)
>  #define SB_NOSEC        BIT(28)
>  #define SB_BORN         BIT(29)
> diff --git a/include/linux/mount.h b/include/linux/mount.h
> index dcc17ce8a959..d4eb90a367af 100644
> --- a/include/linux/mount.h
> +++ b/include/linux/mount.h
> @@ -98,9 +98,6 @@ extern struct vfsmount *vfs_create_mount(struct fs_context *fc);
>  extern struct vfsmount *vfs_kern_mount(struct file_system_type *type,
>  				      int flags, const char *name,
>  				      void *data);
> -extern struct vfsmount *vfs_submount(const struct dentry *mountpoint,
> -				     struct file_system_type *type,
> -				     const char *name, void *data);
>  
>  extern void mnt_set_expiry(struct vfsmount *mnt, struct list_head *expiry_list);
>  extern void mark_mounts_for_expiry(struct list_head *mounts);
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index e22aacb0028a..936a615e8c56 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -51,6 +51,7 @@
>  #include <linux/workqueue.h>
>  #include <linux/sort.h>
>  #include <linux/io.h> /* vmap_page_range() */
> +#include <linux/fs_context.h>
>  
>  #include <asm/setup.h> /* COMMAND_LINE_SIZE */
>  
> @@ -10075,6 +10076,8 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
>  {
>  	struct vfsmount *mnt;
>  	struct file_system_type *type;
> +	struct fs_context *fc;
> +	int ret;
>  
>  	/*
>  	 * To maintain backward compatibility for tools that mount
> @@ -10084,10 +10087,20 @@ static struct vfsmount *trace_automount(struct dentry *mntpt, void *ingore)
>  	type = get_fs_type("tracefs");
>  	if (!type)
>  		return NULL;
> -	mnt = vfs_submount(mntpt, type, "tracefs", NULL);
> +
> +	fc = fs_context_for_submount(type, mntpt);
>  	put_filesystem(type);
> -	if (IS_ERR(mnt))
> -		return NULL;
> +	if (IS_ERR(fc))
> +		return ERR_CAST(fc);
> +
> +	ret = vfs_parse_fs_string(fc, "source",
> +				  "tracefs", strlen("tracefs"));
> +	if (!ret)
> +		mnt = fc_mount(fc);
> +	else
> +		mnt = ERR_PTR(ret);
> +
> +	put_fs_context(fc);
>  	return mnt;
>  }
>  
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

end of thread, other threads:[~2025-05-06 10:57 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-03 21:29 [PATCH][RFC][CFT] kill vfs_submount(), already Al Viro
2025-05-05 10:55 ` Jan Kara
2025-05-05 21:38   ` [PATCH v2] kill vfs_submount() Al Viro
2025-05-05 22:00     ` Al Viro
2025-05-06  0:19     ` Steven Rostedt
2025-05-06 10:57     ` Jan Kara
2025-05-05 13:57 ` [PATCH][RFC][CFT] kill vfs_submount(), already Christian Brauner
2025-05-05 14:15 ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).