diff mbox series

nfs: remove SB_RDONLY when remounting nfs

Message ID 20250221082613.2674633-1-lilingfeng3@huawei.com (mailing list archive)
State Handled Elsewhere
Headers show
Series nfs: remove SB_RDONLY when remounting nfs | expand

Commit Message

Li Lingfeng Feb. 21, 2025, 8:26 a.m. UTC
In some scenarios, when mounting NFS, more than one superblock may be
created. The final superblock used is the last one created, but only the
first superblock carries the ro flag passed from user space. If a ro flag
is added to the superblock via remount, it will trigger the issue
described in Link[1].

Link[2] attempted to address this by marking the superblock as ro during
the initial mount. However, this introduced a new problem in scenarios
where multiple mount points share the same superblock:
[root@a ~]# mount /dev/sdb /mnt/sdb
[root@a ~]# echo "/mnt/sdb *(rw,no_root_squash)" > /etc/exports
[root@a ~]# echo "/mnt/sdb/test_dir2 *(ro,no_root_squash)" >> /etc/exports
[root@a ~]# systemctl restart nfs-server
[root@a ~]# mount -t nfs -o rw 127.0.0.1:/mnt/sdb/test_dir1 /mnt/test_mp1
[root@a ~]# mount | grep nfs4
127.0.0.1:/mnt/sdb/test_dir1 on /mnt/test_mp1 type nfs4 (rw,relatime,...
[root@a ~]# mount -t nfs -o ro 127.0.0.1:/mnt/sdb/test_dir2 /mnt/test_mp2
[root@a ~]# mount | grep nfs4
127.0.0.1:/mnt/sdb/test_dir1 on /mnt/test_mp1 type nfs4 (ro,relatime,...
127.0.0.1:/mnt/sdb/test_dir2 on /mnt/test_mp2 type nfs4 (ro,relatime,...
[root@a ~]#

When mounting the second NFS, the shared superblock is marked as ro,
causing the previous NFS mount to become read-only.

To resolve both issues, the ro flag is no longer applied to the superblock
during remount. Instead, the ro flag on the mount is used to control
whether the mount point is read-only.

Fixes: 281cad46b34d ("NFS: Create a submount rpc_op")
Link[1]: https://lore.kernel.org/all/20240604112636.236517-3-lilingfeng@huaweicloud.com/
Link[2]: https://lore.kernel.org/all/20241130035818.1459775-1-lilingfeng3@huawei.com/
Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
 fs/nfs/super.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Li Lingfeng March 3, 2025, 1:08 a.m. UTC | #1
Friendly ping.

Thanks.

在 2025/2/21 16:26, Li Lingfeng 写道:
> In some scenarios, when mounting NFS, more than one superblock may be
> created. The final superblock used is the last one created, but only the
> first superblock carries the ro flag passed from user space. If a ro flag
> is added to the superblock via remount, it will trigger the issue
> described in Link[1].
>
> Link[2] attempted to address this by marking the superblock as ro during
> the initial mount. However, this introduced a new problem in scenarios
> where multiple mount points share the same superblock:
> [root@a ~]# mount /dev/sdb /mnt/sdb
> [root@a ~]# echo "/mnt/sdb *(rw,no_root_squash)" > /etc/exports
> [root@a ~]# echo "/mnt/sdb/test_dir2 *(ro,no_root_squash)" >> /etc/exports
> [root@a ~]# systemctl restart nfs-server
> [root@a ~]# mount -t nfs -o rw 127.0.0.1:/mnt/sdb/test_dir1 /mnt/test_mp1
> [root@a ~]# mount | grep nfs4
> 127.0.0.1:/mnt/sdb/test_dir1 on /mnt/test_mp1 type nfs4 (rw,relatime,...
> [root@a ~]# mount -t nfs -o ro 127.0.0.1:/mnt/sdb/test_dir2 /mnt/test_mp2
> [root@a ~]# mount | grep nfs4
> 127.0.0.1:/mnt/sdb/test_dir1 on /mnt/test_mp1 type nfs4 (ro,relatime,...
> 127.0.0.1:/mnt/sdb/test_dir2 on /mnt/test_mp2 type nfs4 (ro,relatime,...
> [root@a ~]#
>
> When mounting the second NFS, the shared superblock is marked as ro,
> causing the previous NFS mount to become read-only.
>
> To resolve both issues, the ro flag is no longer applied to the superblock
> during remount. Instead, the ro flag on the mount is used to control
> whether the mount point is read-only.
>
> Fixes: 281cad46b34d ("NFS: Create a submount rpc_op")
> Link[1]: https://lore.kernel.org/all/20240604112636.236517-3-lilingfeng@huaweicloud.com/
> Link[2]: https://lore.kernel.org/all/20241130035818.1459775-1-lilingfeng3@huawei.com/
> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
> ---
>   fs/nfs/super.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index aeb715b4a690..f08e1d7fb179 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1047,6 +1047,7 @@ int nfs_reconfigure(struct fs_context *fc)
>   
>   	sync_filesystem(sb);
>   
> +	fc->sb_flags &= ~SB_RDONLY;
>   	/*
>   	 * Userspace mount programs that send binary options generally send
>   	 * them populated with default values. We have no way to know which
Zhang Yi March 3, 2025, 3:33 a.m. UTC | #2
On 2025/2/21 16:26, Li Lingfeng wrote:
> In some scenarios, when mounting NFS, more than one superblock may be
> created. The final superblock used is the last one created, but only the
> first superblock carries the ro flag passed from user space. If a ro flag
> is added to the superblock via remount, it will trigger the issue
> described in Link[1].
> 
> Link[2] attempted to address this by marking the superblock as ro during
> the initial mount. However, this introduced a new problem in scenarios
> where multiple mount points share the same superblock:
> [root@a ~]# mount /dev/sdb /mnt/sdb
> [root@a ~]# echo "/mnt/sdb *(rw,no_root_squash)" > /etc/exports
> [root@a ~]# echo "/mnt/sdb/test_dir2 *(ro,no_root_squash)" >> /etc/exports
> [root@a ~]# systemctl restart nfs-server
> [root@a ~]# mount -t nfs -o rw 127.0.0.1:/mnt/sdb/test_dir1 /mnt/test_mp1
> [root@a ~]# mount | grep nfs4
> 127.0.0.1:/mnt/sdb/test_dir1 on /mnt/test_mp1 type nfs4 (rw,relatime,...
> [root@a ~]# mount -t nfs -o ro 127.0.0.1:/mnt/sdb/test_dir2 /mnt/test_mp2
> [root@a ~]# mount | grep nfs4
> 127.0.0.1:/mnt/sdb/test_dir1 on /mnt/test_mp1 type nfs4 (ro,relatime,...
> 127.0.0.1:/mnt/sdb/test_dir2 on /mnt/test_mp2 type nfs4 (ro,relatime,...
> [root@a ~]#
> 
> When mounting the second NFS, the shared superblock is marked as ro,
> causing the previous NFS mount to become read-only.
> 
> To resolve both issues, the ro flag is no longer applied to the superblock
> during remount. Instead, the ro flag on the mount is used to control
> whether the mount point is read-only.
> 
> Fixes: 281cad46b34d ("NFS: Create a submount rpc_op")
> Link[1]: https://lore.kernel.org/all/20240604112636.236517-3-lilingfeng@huaweicloud.com/
> Link[2]: https://lore.kernel.org/all/20241130035818.1459775-1-lilingfeng3@huawei.com/
> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
> ---
>  fs/nfs/super.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index aeb715b4a690..f08e1d7fb179 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1047,6 +1047,7 @@ int nfs_reconfigure(struct fs_context *fc)
>  
>  	sync_filesystem(sb);
>  
> +	fc->sb_flags &= ~SB_RDONLY;

What about change sb_flags_mask instead? Something like below,

	fc->sb_flags_mask &= ~SB_RDONLY;

and I'd also suggested to add a comment to explain the reason in detail.

Thanks,
Yi.
Li Lingfeng March 3, 2025, 12:58 p.m. UTC | #3
在 2025/3/3 11:33, Zhang Yi 写道:
> On 2025/2/21 16:26, Li Lingfeng wrote:
>> In some scenarios, when mounting NFS, more than one superblock may be
>> created. The final superblock used is the last one created, but only the
>> first superblock carries the ro flag passed from user space. If a ro flag
>> is added to the superblock via remount, it will trigger the issue
>> described in Link[1].
>>
>> Link[2] attempted to address this by marking the superblock as ro during
>> the initial mount. However, this introduced a new problem in scenarios
>> where multiple mount points share the same superblock:
>> [root@a ~]# mount /dev/sdb /mnt/sdb
>> [root@a ~]# echo "/mnt/sdb *(rw,no_root_squash)" > /etc/exports
>> [root@a ~]# echo "/mnt/sdb/test_dir2 *(ro,no_root_squash)" >> /etc/exports
>> [root@a ~]# systemctl restart nfs-server
>> [root@a ~]# mount -t nfs -o rw 127.0.0.1:/mnt/sdb/test_dir1 /mnt/test_mp1
>> [root@a ~]# mount | grep nfs4
>> 127.0.0.1:/mnt/sdb/test_dir1 on /mnt/test_mp1 type nfs4 (rw,relatime,...
>> [root@a ~]# mount -t nfs -o ro 127.0.0.1:/mnt/sdb/test_dir2 /mnt/test_mp2
>> [root@a ~]# mount | grep nfs4
>> 127.0.0.1:/mnt/sdb/test_dir1 on /mnt/test_mp1 type nfs4 (ro,relatime,...
>> 127.0.0.1:/mnt/sdb/test_dir2 on /mnt/test_mp2 type nfs4 (ro,relatime,...
>> [root@a ~]#
>>
>> When mounting the second NFS, the shared superblock is marked as ro,
>> causing the previous NFS mount to become read-only.
>>
>> To resolve both issues, the ro flag is no longer applied to the superblock
>> during remount. Instead, the ro flag on the mount is used to control
>> whether the mount point is read-only.
>>
>> Fixes: 281cad46b34d ("NFS: Create a submount rpc_op")
>> Link[1]: https://lore.kernel.org/all/20240604112636.236517-3-lilingfeng@huaweicloud.com/
>> Link[2]: https://lore.kernel.org/all/20241130035818.1459775-1-lilingfeng3@huawei.com/
>> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
>> ---
>>   fs/nfs/super.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index aeb715b4a690..f08e1d7fb179 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -1047,6 +1047,7 @@ int nfs_reconfigure(struct fs_context *fc)
>>   
>>   	sync_filesystem(sb);
>>   
>> +	fc->sb_flags &= ~SB_RDONLY;
> What about change sb_flags_mask instead? Something like below,
>
> 	fc->sb_flags_mask &= ~SB_RDONLY;
>
> and I'd also suggested to add a comment to explain the reason in detail.
>
> Thanks,
> Yi.

Thanks for your advice.  I will send v2 soon.
diff mbox series

Patch

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index aeb715b4a690..f08e1d7fb179 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1047,6 +1047,7 @@  int nfs_reconfigure(struct fs_context *fc)
 
 	sync_filesystem(sb);
 
+	fc->sb_flags &= ~SB_RDONLY;
 	/*
 	 * Userspace mount programs that send binary options generally send
 	 * them populated with default values. We have no way to know which