[PATCH RFC 0/2] NFSv4: set sb_flags to second superblock

[PATCH RFC 0/2] NFSv4: set sb_flags to second superblock

[Li Lingfeng]

Added sb_flags parameter to d_automount callback function and
fs_context_for_submount().
NFSv4 uses this parameter to set the second superblock.

Li Lingfeng (2):
  fs: pass sb_flags to submount
  NFSv4: set sb_flags to second superblock

 fs/afs/internal.h          | 2 +-
 fs/afs/mntpt.c             | 4 ++--
 fs/autofs/root.c           | 4 ++--
 fs/debugfs/inode.c         | 2 +-
 fs/fs_context.c            | 5 +++--
 fs/fuse/dir.c              | 4 ++--
 fs/namei.c                 | 3 ++-
 fs/nfs/internal.h          | 2 +-
 fs/nfs/namespace.c         | 4 ++--
 fs/smb/client/cifsfs.h     | 2 +-
 fs/smb/client/namespace.c  | 2 +-
 include/linux/dcache.h     | 2 +-
 include/linux/fs_context.h | 3 ++-
 13 files changed, 21 insertions(+), 18 deletions(-)

-- 
2.39.2

[PATCH RFC 1/2] fs: pass sb_flags to submount

[Li Lingfeng]

From: Li Lingfeng <lilingfeng3@huawei.com>

This commit has no functional change.
Get sb_flags by nameidata, and pass it to submount.

Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
 fs/afs/internal.h          | 2 +-
 fs/afs/mntpt.c             | 4 ++--
 fs/autofs/root.c           | 4 ++--
 fs/debugfs/inode.c         | 2 +-
 fs/fs_context.c            | 5 +++--
 fs/fuse/dir.c              | 4 ++--
 fs/namei.c                 | 3 ++-
 fs/nfs/internal.h          | 2 +-
 fs/nfs/namespace.c         | 4 ++--
 fs/smb/client/cifsfs.h     | 2 +-
 fs/smb/client/namespace.c  | 2 +-
 include/linux/dcache.h     | 2 +-
 include/linux/fs_context.h | 3 ++-
 13 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 6e1d3c4daf72..dc07446e6378 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -1295,7 +1295,7 @@ extern const struct inode_operations afs_mntpt_inode_operations;
 extern const struct inode_operations afs_autocell_inode_operations;
 extern const struct file_operations afs_mntpt_file_operations;
 
-extern struct vfsmount *afs_d_automount(struct path *);
+extern struct vfsmount *afs_d_automount(struct path *, unsigned int);
 extern void afs_mntpt_kill_timer(void);
 
 /*
diff --git a/fs/afs/mntpt.c b/fs/afs/mntpt.c
index 297487ee8323..3519deab514f 100644
--- a/fs/afs/mntpt.c
+++ b/fs/afs/mntpt.c
@@ -161,7 +161,7 @@ static struct vfsmount *afs_mntpt_do_automount(struct dentry *mntpt)
 
 	BUG_ON(!d_inode(mntpt));
 
-	fc = fs_context_for_submount(&afs_fs_type, mntpt);
+	fc = fs_context_for_submount(&afs_fs_type, mntpt, 0);
 	if (IS_ERR(fc))
 		return ERR_CAST(fc);
 
@@ -178,7 +178,7 @@ static struct vfsmount *afs_mntpt_do_automount(struct dentry *mntpt)
 /*
  * handle an automount point
  */
-struct vfsmount *afs_d_automount(struct path *path)
+struct vfsmount *afs_d_automount(struct path *path, unsigned int sb_flags)
 {
 	struct vfsmount *newmnt;
 
diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 530d18827e35..f7294d3a089f 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -25,7 +25,7 @@ static long autofs_root_compat_ioctl(struct file *,
 static int autofs_dir_open(struct inode *inode, struct file *file);
 static struct dentry *autofs_lookup(struct inode *,
 				    struct dentry *, unsigned int);
-static struct vfsmount *autofs_d_automount(struct path *);
+static struct vfsmount *autofs_d_automount(struct path *, unsigned int);
 static int autofs_d_manage(const struct path *, bool);
 static void autofs_dentry_release(struct dentry *);
 
@@ -328,7 +328,7 @@ static struct dentry *autofs_mountpoint_changed(struct path *path)
 	return path->dentry;
 }
 
-static struct vfsmount *autofs_d_automount(struct path *path)
+static struct vfsmount *autofs_d_automount(struct path *path, unsigned int sb_flags)
 {
 	struct dentry *dentry = path->dentry;
 	struct autofs_sb_info *sbi = autofs_sbi(dentry->d_sb);
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index dc51df0b118d..a2cdab95d12a 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -229,7 +229,7 @@ static void debugfs_release_dentry(struct dentry *dentry)
 	kfree(fsd);
 }
 
-static struct vfsmount *debugfs_automount(struct path *path)
+static struct vfsmount *debugfs_automount(struct path *path, unsigned int sb_flags)
 {
 	struct debugfs_fsdata *fsd = path->dentry->d_fsdata;
 
diff --git a/fs/fs_context.c b/fs/fs_context.c
index 98589aae5208..95367dc7dc40 100644
--- a/fs/fs_context.c
+++ b/fs/fs_context.c
@@ -352,12 +352,13 @@ EXPORT_SYMBOL(fs_context_for_reconfigure);
  * the fc->security object is inherited from @reference (if needed).
  */
 struct fs_context *fs_context_for_submount(struct file_system_type *type,
-					   struct dentry *reference)
+					   struct dentry *reference,
+					   unsigned int sb_flags)
 {
 	struct fs_context *fc;
 	int ret;
 
-	fc = alloc_fs_context(type, reference, 0, 0, FS_CONTEXT_FOR_SUBMOUNT);
+	fc = alloc_fs_context(type, reference, sb_flags, 0, FS_CONTEXT_FOR_SUBMOUNT);
 	if (IS_ERR(fc))
 		return fc;
 
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 2b0d4781f394..88bd5aec11e7 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -305,13 +305,13 @@ static int fuse_dentry_delete(const struct dentry *dentry)
  * as the root), and return that mount so it can be auto-mounted on
  * @path.
  */
-static struct vfsmount *fuse_dentry_automount(struct path *path)
+static struct vfsmount *fuse_dentry_automount(struct path *path, unsigned int sb_flags)
 {
 	struct fs_context *fsc;
 	struct vfsmount *mnt;
 	struct fuse_inode *mp_fi = get_fuse_inode(d_inode(path->dentry));
 
-	fsc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry);
+	fsc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry, 0);
 	if (IS_ERR(fsc))
 		return ERR_CAST(fsc);
 
diff --git a/fs/namei.c b/fs/namei.c
index 37fb0a8aa09a..445de9fcef38 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1345,6 +1345,7 @@ static bool choose_mountpoint(struct mount *m, const struct path *root,
 static int follow_automount(struct path *path, int *count, unsigned lookup_flags)
 {
 	struct dentry *dentry = path->dentry;
+	struct nameidata *nd = container_of(count, struct nameidata, total_link_count);
 
 	/* We don't want to mount if someone's just doing a stat -
 	 * unless they're stat'ing a directory and appended a '/' to
@@ -1365,7 +1366,7 @@ static int follow_automount(struct path *path, int *count, unsigned lookup_flags
 	if (count && (*count)++ >= MAXSYMLINKS)
 		return -ELOOP;
 
-	return finish_automount(dentry->d_op->d_automount(path), path);
+	return finish_automount(dentry->d_op->d_automount(path, nd->root.mnt->mnt_sb->s_flags), path);
 }
 
 /*
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 9f0f4534744b..f0e35e0d05c9 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -484,7 +484,7 @@ static inline bool nfs_file_io_is_buffered(struct nfs_inode *nfsi)
 #define NFS_PATH_CANONICAL 1
 extern char *nfs_path(char **p, struct dentry *dentry,
 		      char *buffer, ssize_t buflen, unsigned flags);
-extern struct vfsmount *nfs_d_automount(struct path *path);
+extern struct vfsmount *nfs_d_automount(struct path *path, unsigned int);
 int nfs_submount(struct fs_context *, struct nfs_server *);
 int nfs_do_submount(struct fs_context *);
 
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index e7494cdd957e..887aeacedebd 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -142,7 +142,7 @@ EXPORT_SYMBOL_GPL(nfs_path);
  * situation, and that different filesystems may want to use
  * different security flavours.
  */
-struct vfsmount *nfs_d_automount(struct path *path)
+struct vfsmount *nfs_d_automount(struct path *path, unsigned int sb_flags)
 {
 	struct nfs_fs_context *ctx;
 	struct fs_context *fc;
@@ -158,7 +158,7 @@ struct vfsmount *nfs_d_automount(struct path *path)
 	/* Open a new filesystem context, transferring parameters from the
 	 * parent superblock, including the network namespace.
 	 */
-	fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry);
+	fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry, 0);
 	if (IS_ERR(fc))
 		return ERR_CAST(fc);
 
diff --git a/fs/smb/client/cifsfs.h b/fs/smb/client/cifsfs.h
index 62d5fee3e5eb..eec5d5fa42a5 100644
--- a/fs/smb/client/cifsfs.h
+++ b/fs/smb/client/cifsfs.h
@@ -113,7 +113,7 @@ extern int cifs_readdir(struct file *file, struct dir_context *ctx);
 extern const struct dentry_operations cifs_dentry_ops;
 extern const struct dentry_operations cifs_ci_dentry_ops;
 
-extern struct vfsmount *cifs_d_automount(struct path *path);
+extern struct vfsmount *cifs_d_automount(struct path *path, unsigned int sb_flags);
 
 /* Functions related to symlinks */
 extern const char *cifs_get_link(struct dentry *, struct inode *,
diff --git a/fs/smb/client/namespace.c b/fs/smb/client/namespace.c
index 4a517b280f2b..81640e6b2d3f 100644
--- a/fs/smb/client/namespace.c
+++ b/fs/smb/client/namespace.c
@@ -254,7 +254,7 @@ static struct vfsmount *cifs_do_automount(struct path *path)
 /*
  * Attempt to automount the referral
  */
-struct vfsmount *cifs_d_automount(struct path *path)
+struct vfsmount *cifs_d_automount(struct path *path, unsigned int sb_flags)
 {
 	struct vfsmount *newmnt;
 
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index bf53e3894aae..864b0cd1c0c9 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -142,7 +142,7 @@ struct dentry_operations {
 	void (*d_prune)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
 	char *(*d_dname)(struct dentry *, char *, int);
-	struct vfsmount *(*d_automount)(struct path *);
+	struct vfsmount *(*d_automount)(struct path *, unsigned int sb_flags);
 	int (*d_manage)(const struct path *, bool);
 	struct dentry *(*d_real)(struct dentry *, enum d_real_type type);
 } ____cacheline_aligned;
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index c13e99cbbf81..920bcbfaff2e 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -130,7 +130,8 @@ extern struct fs_context *fs_context_for_reconfigure(struct dentry *dentry,
 						unsigned int sb_flags,
 						unsigned int sb_flags_mask);
 extern struct fs_context *fs_context_for_submount(struct file_system_type *fs_type,
-						struct dentry *reference);
+						struct dentry *reference,
+						unsigned int sb_flags);
 
 extern struct fs_context *vfs_dup_fs_context(struct fs_context *fc);
 extern int vfs_parse_fs_param(struct fs_context *fc, struct fs_parameter *param);
-- 
2.39.2

[PATCH RFC 2/2] NFSv4: set sb_flags to second superblock

[Li Lingfeng]

From: Li Lingfeng <lilingfeng3@huawei.com>

During the process of mounting an NFSv4 client, two superblocks will be
created in sequence. The first superblock corresponds to the root
directory exported by the server, and the second superblock corresponds to
the directory that will be actually mounted. The first superblock will
eventually be destroyed.
The flag passed from user mode will only be passed to the first
superblock, resulting in the actual used superblock not carrying the flag
passed from user mode(fs_context_for_submount() will set sb_flags as 0).

If the 'ro' parameter is used in two consecutive mount commands, only the
first execution will create a new vfsmount, and the kernel will return
EBUSY on the second execution. However, if a remount command with the 'ro'
parameter is executed between the two mount commands, both mount commands
will create new vfsmounts.

The superblock generated after the first mount command does not have the
'ro' flag, and the read-only status of the file system is implemented by
checking the read-only flag of the vfsmount. After executing the remount
command, the 'ro' flag will be added to the superblock. When the second
mount command is executed, the comparison result between the superblock
with the 'ro' flag and the fs_context without the flag in the
nfs_compare_mount_options() function will be different, resulting in the
creation of a new vfsmount.

This problem can be reproduced by performing the following operations:
mount -t nfs -o ro,vers=4.0 192.168.240.250:/sdb /mnt/sdb
mount -t nfs -o remount,ro,vers=4.0 192.168.240.250:/sdb /mnt/sdb
mount -t nfs -o ro,vers=4.0 192.168.240.250:/sdb /mnt/sdb
Two vfsmounts are generated:
[root@localhost ~]# mount | grep nfs
192.168.240.250:/sdb on /mnt/sdb type nfs4 (ro,relatime,vers=4.0,
rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,timeo=600,retrans=2,
sec=sys,clientaddr=192.168.240.251,local_lock=none,addr=192.168.240.250)
192.168.240.250:/sdb on /mnt/sdb type nfs4 (ro,relatime,vers=4.0,
rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,timeo=600,retrans=2,
sec=sys,clientaddr=192.168.240.251,local_lock=none,addr=192.168.240.250)

Fix this by setting sb_flags to second superblock.

Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
 fs/nfs/namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 887aeacedebd..8b3d75af60d4 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -158,7 +158,7 @@ struct vfsmount *nfs_d_automount(struct path *path, unsigned int sb_flags)
 	/* Open a new filesystem context, transferring parameters from the
 	 * parent superblock, including the network namespace.
 	 */
-	fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry, 0);
+	fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry, sb_flags);
 	if (IS_ERR(fc))
 		return ERR_CAST(fc);
 
-- 
2.39.2

Re: [PATCH RFC 2/2] NFSv4: set sb_flags to second superblock

[Li Lingfeng]

I think this may be a problem, but I'm unable to come up with a suitable 
solution. Would you mind providing some suggestions?

在 2024/6/4 19:26, Li Lingfeng 写道:
> From: Li Lingfeng <lilingfeng3@huawei.com>
>
> During the process of mounting an NFSv4 client, two superblocks will be
> created in sequence. The first superblock corresponds to the root
> directory exported by the server, and the second superblock corresponds to
> the directory that will be actually mounted. The first superblock will
> eventually be destroyed.
> The flag passed from user mode will only be passed to the first
> superblock, resulting in the actual used superblock not carrying the flag
> passed from user mode(fs_context_for_submount() will set sb_flags as 0).
>
> If the 'ro' parameter is used in two consecutive mount commands, only the
> first execution will create a new vfsmount, and the kernel will return
> EBUSY on the second execution. However, if a remount command with the 'ro'
> parameter is executed between the two mount commands, both mount commands
> will create new vfsmounts.
>
> The superblock generated after the first mount command does not have the
> 'ro' flag, and the read-only status of the file system is implemented by
> checking the read-only flag of the vfsmount. After executing the remount
> command, the 'ro' flag will be added to the superblock. When the second
> mount command is executed, the comparison result between the superblock
> with the 'ro' flag and the fs_context without the flag in the
> nfs_compare_mount_options() function will be different, resulting in the
> creation of a new vfsmount.
>
> This problem can be reproduced by performing the following operations:
> mount -t nfs -o ro,vers=4.0 192.168.240.250:/sdb /mnt/sdb
> mount -t nfs -o remount,ro,vers=4.0 192.168.240.250:/sdb /mnt/sdb
> mount -t nfs -o ro,vers=4.0 192.168.240.250:/sdb /mnt/sdb
> Two vfsmounts are generated:
> [root@localhost ~]# mount | grep nfs
> 192.168.240.250:/sdb on /mnt/sdb type nfs4 (ro,relatime,vers=4.0,
> rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,timeo=600,retrans=2,
> sec=sys,clientaddr=192.168.240.251,local_lock=none,addr=192.168.240.250)
> 192.168.240.250:/sdb on /mnt/sdb type nfs4 (ro,relatime,vers=4.0,
> rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,timeo=600,retrans=2,
> sec=sys,clientaddr=192.168.240.251,local_lock=none,addr=192.168.240.250)
>
> Fix this by setting sb_flags to second superblock.
>
> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
> ---
>   fs/nfs/namespace.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
> index 887aeacedebd..8b3d75af60d4 100644
> --- a/fs/nfs/namespace.c
> +++ b/fs/nfs/namespace.c
> @@ -158,7 +158,7 @@ struct vfsmount *nfs_d_automount(struct path *path, unsigned int sb_flags)
>   	/* Open a new filesystem context, transferring parameters from the
>   	 * parent superblock, including the network namespace.
>   	 */
> -	fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry, 0);
> +	fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, path->dentry, sb_flags);
>   	if (IS_ERR(fc))
>   		return ERR_CAST(fc);
>   

Re: [PATCH RFC 2/2] NFSv4: set sb_flags to second superblock

[Li Lingfeng]

Friendly ping ...

Thanks

在 2024/6/14 11:14, Li Lingfeng 写道:
> I think this may be a problem, but I'm unable to come up with a 
> suitable solution. Would you mind providing some suggestions?
>
> 在 2024/6/4 19:26, Li Lingfeng 写道:
>> From: Li Lingfeng <lilingfeng3@huawei.com>
>>
>> During the process of mounting an NFSv4 client, two superblocks will be
>> created in sequence. The first superblock corresponds to the root
>> directory exported by the server, and the second superblock 
>> corresponds to
>> the directory that will be actually mounted. The first superblock will
>> eventually be destroyed.
>> The flag passed from user mode will only be passed to the first
>> superblock, resulting in the actual used superblock not carrying the 
>> flag
>> passed from user mode(fs_context_for_submount() will set sb_flags as 0).
>>
>> If the 'ro' parameter is used in two consecutive mount commands, only 
>> the
>> first execution will create a new vfsmount, and the kernel will return
>> EBUSY on the second execution. However, if a remount command with the 
>> 'ro'
>> parameter is executed between the two mount commands, both mount 
>> commands
>> will create new vfsmounts.
>>
>> The superblock generated after the first mount command does not have the
>> 'ro' flag, and the read-only status of the file system is implemented by
>> checking the read-only flag of the vfsmount. After executing the remount
>> command, the 'ro' flag will be added to the superblock. When the second
>> mount command is executed, the comparison result between the superblock
>> with the 'ro' flag and the fs_context without the flag in the
>> nfs_compare_mount_options() function will be different, resulting in the
>> creation of a new vfsmount.
>>
>> This problem can be reproduced by performing the following operations:
>> mount -t nfs -o ro,vers=4.0 192.168.240.250:/sdb /mnt/sdb
>> mount -t nfs -o remount,ro,vers=4.0 192.168.240.250:/sdb /mnt/sdb
>> mount -t nfs -o ro,vers=4.0 192.168.240.250:/sdb /mnt/sdb
>> Two vfsmounts are generated:
>> [root@localhost ~]# mount | grep nfs
>> 192.168.240.250:/sdb on /mnt/sdb type nfs4 (ro,relatime,vers=4.0,
>> rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,timeo=600,retrans=2, 
>>
>> sec=sys,clientaddr=192.168.240.251,local_lock=none,addr=192.168.240.250)
>> 192.168.240.250:/sdb on /mnt/sdb type nfs4 (ro,relatime,vers=4.0,
>> rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,timeo=600,retrans=2, 
>>
>> sec=sys,clientaddr=192.168.240.251,local_lock=none,addr=192.168.240.250)
>>
>> Fix this by setting sb_flags to second superblock.
>>
>> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
>> ---
>>   fs/nfs/namespace.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
>> index 887aeacedebd..8b3d75af60d4 100644
>> --- a/fs/nfs/namespace.c
>> +++ b/fs/nfs/namespace.c
>> @@ -158,7 +158,7 @@ struct vfsmount *nfs_d_automount(struct path 
>> *path, unsigned int sb_flags)
>>       /* Open a new filesystem context, transferring parameters from the
>>        * parent superblock, including the network namespace.
>>        */
>> -    fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, 
>> path->dentry, 0);
>> +    fc = fs_context_for_submount(path->mnt->mnt_sb->s_type, 
>> path->dentry, sb_flags);
>>       if (IS_ERR(fc))
>>           return ERR_CAST(fc);