linux-kernel - Re: [PATCH 1/2] nfs: clear SB_RDONLY before getting superblock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAAH4uRAZAii6bL9UPx3dJ9x0jUW_UNrY8Bm07_k5znZT0Q=2jg@mail.gmail.com>
Date: Tue, 4 Mar 2025 11:22:40 -0500
From: Eric Hagberg <ehagberg@...estreet.com>
To: Li Lingfeng <lilingfeng3@...wei.com>
Cc: trondmy@...nel.org, anna@...nel.org, linux-nfs@...r.kernel.org, 
 	linux-kernel@...r.kernel.org, linux-nfs@...ebar.com, yukuai1@...weicloud.com, 
 	houtao1@...wei.com, yi.zhang@...wei.com, yangerkun@...wei.com, 
 	lilingfeng@...weicloud.com
Subject: Re: [PATCH 1/2] nfs: clear SB_RDONLY before getting superblock

BTW - I tested applying both patches and the second one did fix the
issue reported in
https://lore.kernel.org/all/12d7ea53-1202-4e21-a7ef-431c94758ce5@app.fastmail.com/T/


On Tue, Mar 4, 2025 at 7:48 AM Li Lingfeng <lilingfeng3@...wei.com> wrote:
>
> As described in the link, commit 52cb7f8f1778 ("nfs: ignore SB_RDONLY when
> mounting nfs") removed the check for the ro flag when determining whether
> to share the superblock, which caused issues when mounting different
> subdirectories under the same export directory via NFSv3. However, this
> change did not affect NFSv4.
>
> For NFSv3:
> 1) A single superblock is created for the initial mount.
> 2) When mounted read-only, this superblock carries the SB_RDONLY flag.
> 3) Before commit 52cb7f8f1778 ("nfs: ignore SB_RDONLY when mounting nfs"):
> Subsequent rw mounts would not share the existing ro superblock due to
> flag mismatch, creating a new superblock without SB_RDONLY.
> After the commit:
>   The SB_RDONLY flag is ignored during superblock comparison, and this leads
>   to sharing the existing superblock even for rw mounts.
>   Ultimately results in write operations being rejected at the VFS layer.
>
> For NFSv4:
> 1) Multiple superblocks are created and the last one will be kept.
> 2) The actually used superblock for ro mounts doesn't carry SB_RDONLY flag.
> Therefore, commit 52cb7f8f1778 doesn't affect NFSv4 mounts.
>
> Clear SB_RDONLY before getting superblock when NFS_MOUNT_UNSHARED is not
> set to fix it.
>
> Fixes: 52cb7f8f1778 ("nfs: ignore SB_RDONLY when mounting nfs")
> Closes: https://lore.kernel.org/all/12d7ea53-1202-4e21-a7ef-431c94758ce5@app.fastmail.com/T/
> Signed-off-by: Li Lingfeng <lilingfeng3@...wei.com>
> ---
>  fs/nfs/super.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index aeb715b4a690..3e5528c2c822 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1304,8 +1304,17 @@ int nfs_get_tree_common(struct fs_context *fc)
>         if (IS_ERR(server))
>                 return PTR_ERR(server);
>
> +       /*
> +        * When NFS_MOUNT_UNSHARED is not set, NFS forces the sharing of a
> +        * superblock among each filesystem that mounts sub-directories
> +        * belonging to a single exported root path.
> +        * To prevent interference between different filesystems, the
> +        * SB_RDONLY flag should be removed from the superblock.
> +        */
>         if (server->flags & NFS_MOUNT_UNSHARED)
>                 compare_super = NULL;
> +       else
> +               fc->sb_flags &= ~SB_RDONLY;
>
>         /* -o noac implies -o sync */
>         if (server->flags & NFS_MOUNT_NOAC)
> --
> 2.31.1
>