When you normally attempt to mount a share twice on the same mountpoint,
a check in do_add_mount causes it to return an error
# mount localhost:/nfsv3 /mnt
# mount localhost:/nfsv3 /mnt
mount.nfs: /mnt is already mounted or busy
However when using the option 'noac', the user is able to mount the same
share on the same mountpoint multiple times. This happens because a
share mounted with the noac option is automatically assigned the 'sync'
flag MS_SYNCHRONOUS in nfs_initialise_sb(). This flag is set after the
check for already existing superblocks is done in sget(). The check for
the mount flags in nfs_compare_mount_options() does not take into
account the 'sync' flag applied later on in the code path. This means
that when using 'noac', a new superblock structure is assigned for every
new mount of the same share and multiple shares on the same mountpoint
are allowed.
ie.
# mount -onoac localhost:/nfsv3 /mnt
can be run multiple times.
The patch checks for noac and assigns the sync flag before sget() is
called to obtain an already existing superblock structure.
Signed-off-by: Sachin Prabhu <[email protected]>
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index b961cea..9b7dd70 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2035,9 +2035,6 @@ static inline void nfs_initialise_sb(struct
super_block *sb)
sb->s_blocksize = nfs_block_bits(server->wsize,
&sb->s_blocksize_bits);
- if (server->flags & NFS_MOUNT_NOAC)
- sb->s_flags |= MS_SYNCHRONOUS;
-
sb->s_bdi = &server->backing_dev_info;
nfs_super_set_maxbytes(sb, server->maxfilesize);
@@ -2249,6 +2246,10 @@ static struct dentry *nfs_fs_mount(struct
file_system_type *fs_type,
if (server->flags & NFS_MOUNT_UNSHARED)
compare_super = NULL;
+ /* -o noac implies -o sync */
+ if (server->flags & NFS_MOUNT_NOAC)
+ sb_mntdata.mntflags |= MS_SYNCHRONOUS;
+
/* Get a superblock - note that we may end up sharing one that already
exists */
s = sget(fs_type, compare_super, nfs_set_super, &sb_mntdata);
if (IS_ERR(s)) {
@@ -2361,6 +2362,10 @@ nfs_xdev_mount(struct file_system_type *fs_type,
int flags,
if (server->flags & NFS_MOUNT_UNSHARED)
compare_super = NULL;
+ /* -o noac implies -o sync */
+ if (server->flags & NFS_MOUNT_NOAC)
+ sb_mntdata.mntflags |= MS_SYNCHRONOUS;
+
/* Get a superblock - note that we may end up sharing one that already
exists */
s = sget(&nfs_fs_type, compare_super, nfs_set_super, &sb_mntdata);
if (IS_ERR(s)) {
@@ -2628,6 +2633,10 @@ nfs4_remote_mount(struct file_system_type
*fs_type, int flags,
if (server->flags & NFS4_MOUNT_UNSHARED)
compare_super = NULL;
+ /* -o noac implies -o sync */
+ if (server->flags & NFS_MOUNT_NOAC)
+ sb_mntdata.mntflags |= MS_SYNCHRONOUS;
+
/* Get a superblock - note that we may end up sharing one that already
exists */
s = sget(&nfs4_fs_type, compare_super, nfs_set_super, &sb_mntdata);
if (IS_ERR(s)) {
@@ -2916,6 +2925,10 @@ nfs4_xdev_mount(struct file_system_type *fs_type,
int flags,
if (server->flags & NFS4_MOUNT_UNSHARED)
compare_super = NULL;
+ /* -o noac implies -o sync */
+ if (server->flags & NFS_MOUNT_NOAC)
+ sb_mntdata.mntflags |= MS_SYNCHRONOUS;
+
/* Get a superblock - note that we may end up sharing one that already
exists */
s = sget(&nfs4_fs_type, compare_super, nfs_set_super, &sb_mntdata);
if (IS_ERR(s)) {
@@ -3003,6 +3016,10 @@ nfs4_remote_referral_mount(struct
file_system_type *fs_type, int flags,
if (server->flags & NFS4_MOUNT_UNSHARED)
compare_super = NULL;
+ /* -o noac implies -o sync */
+ if (server->flags & NFS_MOUNT_NOAC)
+ sb_mntdata.mntflags |= MS_SYNCHRONOUS;
+
/* Get a superblock - note that we may end up sharing one that already
exists */
s = sget(&nfs4_fs_type, compare_super, nfs_set_super, &sb_mntdata);
if (IS_ERR(s)) {
On Fri, 2011-07-29 at 14:50 +0100, Sachin Prabhu wrote:
> When you normally attempt to mount a share twice on the same mountpoint,
> a check in do_add_mount causes it to return an error
>
> # mount localhost:/nfsv3 /mnt
> # mount localhost:/nfsv3 /mnt
> mount.nfs: /mnt is already mounted or busy
>
> However when using the option 'noac', the user is able to mount the same
> share on the same mountpoint multiple times. This happens because a
> share mounted with the noac option is automatically assigned the 'sync'
> flag MS_SYNCHRONOUS in nfs_initialise_sb(). This flag is set after the
> check for already existing superblocks is done in sget(). The check for
> the mount flags in nfs_compare_mount_options() does not take into
> account the 'sync' flag applied later on in the code path. This means
> that when using 'noac', a new superblock structure is assigned for every
> new mount of the same share and multiple shares on the same mountpoint
> are allowed.
>
> ie.
> # mount -onoac localhost:/nfsv3 /mnt
> can be run multiple times.
>
> The patch checks for noac and assigns the sync flag before sget() is
> called to obtain an already existing superblock structure.
>
>
> Signed-off-by: Sachin Prabhu <[email protected]>
Hi Sachin,
This patch fails to apply with the error 'corrupt patch at line 6'. It
looks to me as if your mailer has reformatted it for you.
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Mon, 2011-08-01 at 07:19 -0400, Christoph Hellwig wrote:
> On Mon, Aug 01, 2011 at 12:10:12PM +0100, Sachin Prabhu wrote:
> > Do not allow multiple mounts on same mountpoint when using -o noac
>
> The patch content really doesn't seem to match the subject line and
> most of the content.
>
> > However when using the option 'noac', the user is able to mount the same
> > share on the same mountpoint multiple times. This happens because a
> > share mounted with the noac option is automatically assigned the 'sync'
> > flag MS_SYNCHRONOUS in nfs_initialise_sb(). This flag is set after the
> > check for already existing superblocks is done in sget(). The check for
> > the mount flags in nfs_compare_mount_options() does not take into
> > account the 'sync' flag applied later on in the code path. This means
> > that when using 'noac', a new superblock structure is assigned for every
> > new mount of the same share and multiple shares on the same mountpoint
> > are allowed.
> >
> > ie.
> > # mount -onoac localhost:/nfsv3 /mnt
> > can be run multiple times.
>
> > The patch checks for noac and assigns the sync flag before sget() is
> > called to obtain an already existing superblock structure.
>
> That's a fine fix, but the the whole patch subject and description
> focusses on a side effect rather than the underlying real problem.
>
> The underlying issue is that youwant to share superblocks when
> having multiple mounts with -o noac, which requires assigning the
> sync flag ealier.
>
> The check in do_add_mount simply prevents mounts with the same
> superblock and root dentry at the same mountpoint, so it will fail for
> any two nfs mounts that might be from the same server but do not share
> the superblock due to different options. I'm actually not sure we want
> to prevent that in general, and I can't see how it could be done easily.
>
Yes. That is essentially what the problem is.
The problem was reported by a user who noticed that passing the 'noac'
mount option allowed them to repeatedly mount the same share on the same
mount point. I just used the same perspective when reporting the issue
upstream.
Sachin Prabhu
On Mon, Aug 01, 2011 at 12:10:12PM +0100, Sachin Prabhu wrote:
> Do not allow multiple mounts on same mountpoint when using -o noac
The patch content really doesn't seem to match the subject line and
most of the content.
> However when using the option 'noac', the user is able to mount the same
> share on the same mountpoint multiple times. This happens because a
> share mounted with the noac option is automatically assigned the 'sync'
> flag MS_SYNCHRONOUS in nfs_initialise_sb(). This flag is set after the
> check for already existing superblocks is done in sget(). The check for
> the mount flags in nfs_compare_mount_options() does not take into
> account the 'sync' flag applied later on in the code path. This means
> that when using 'noac', a new superblock structure is assigned for every
> new mount of the same share and multiple shares on the same mountpoint
> are allowed.
>
> ie.
> # mount -onoac localhost:/nfsv3 /mnt
> can be run multiple times.
> The patch checks for noac and assigns the sync flag before sget() is
> called to obtain an already existing superblock structure.
That's a fine fix, but the the whole patch subject and description
focusses on a side effect rather than the underlying real problem.
The underlying issue is that youwant to share superblocks when
having multiple mounts with -o noac, which requires assigning the
sync flag ealier.
The check in do_add_mount simply prevents mounts with the same
superblock and root dentry at the same mountpoint, so it will fail for
any two nfs mounts that might be from the same server but do not share
the superblock due to different options. I'm actually not sure we want
to prevent that in general, and I can't see how it could be done easily.
On Sat, 2011-07-30 at 14:33 -0400, Trond Myklebust wrote:
> On Fri, 2011-07-29 at 14:50 +0100, Sachin Prabhu wrote:
> > When you normally attempt to mount a share twice on the same mountpoint,
> > a check in do_add_mount causes it to return an error
> >
> > # mount localhost:/nfsv3 /mnt
> > # mount localhost:/nfsv3 /mnt
> > mount.nfs: /mnt is already mounted or busy
> >
> > However when using the option 'noac', the user is able to mount the same
> > share on the same mountpoint multiple times. This happens because a
> > share mounted with the noac option is automatically assigned the 'sync'
> > flag MS_SYNCHRONOUS in nfs_initialise_sb(). This flag is set after the
> > check for already existing superblocks is done in sget(). The check for
> > the mount flags in nfs_compare_mount_options() does not take into
> > account the 'sync' flag applied later on in the code path. This means
> > that when using 'noac', a new superblock structure is assigned for every
> > new mount of the same share and multiple shares on the same mountpoint
> > are allowed.
> >
> > ie.
> > # mount -onoac localhost:/nfsv3 /mnt
> > can be run multiple times.
> >
> > The patch checks for noac and assigns the sync flag before sget() is
> > called to obtain an already existing superblock structure.
> >
> >
> > Signed-off-by: Sachin Prabhu <[email protected]>
>
> Hi Sachin,
>
> This patch fails to apply with the error 'corrupt patch at line 6'. It
> looks to me as if your mailer has reformatted it for you.
>
> Cheers
> Trond
Hello Trond,
Sorry about that. I have re-posted the patch.
Sachin Prabhu
Do not allow multiple mounts on same mountpoint when using -o noac
When you normally attempt to mount a share twice on the same mountpoint,
a check in do_add_mount causes it to return an error
# mount localhost:/nfsv3 /mnt
# mount localhost:/nfsv3 /mnt
mount.nfs: /mnt is already mounted or busy
However when using the option 'noac', the user is able to mount the same
share on the same mountpoint multiple times. This happens because a
share mounted with the noac option is automatically assigned the 'sync'
flag MS_SYNCHRONOUS in nfs_initialise_sb(). This flag is set after the
check for already existing superblocks is done in sget(). The check for
the mount flags in nfs_compare_mount_options() does not take into
account the 'sync' flag applied later on in the code path. This means
that when using 'noac', a new superblock structure is assigned for every
new mount of the same share and multiple shares on the same mountpoint
are allowed.
ie.
# mount -onoac localhost:/nfsv3 /mnt
can be run multiple times.
The patch checks for noac and assigns the sync flag before sget() is
called to obtain an already existing superblock structure.
Signed-off-by: Sachin Prabhu <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index b961cea..9b7dd70 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2035,9 +2035,6 @@ static inline void nfs_initialise_sb(struct super_block *sb)
sb->s_blocksize = nfs_block_bits(server->wsize,
&sb->s_blocksize_bits);
- if (server->flags & NFS_MOUNT_NOAC)
- sb->s_flags |= MS_SYNCHRONOUS;
-
sb->s_bdi = &server->backing_dev_info;
nfs_super_set_maxbytes(sb, server->maxfilesize);
@@ -2249,6 +2246,10 @@ static struct dentry *nfs_fs_mount(struct file_system_type *fs_type,
if (server->flags & NFS_MOUNT_UNSHARED)
compare_super = NULL;
+ /* -o noac implies -o sync */
+ if (server->flags & NFS_MOUNT_NOAC)
+ sb_mntdata.mntflags |= MS_SYNCHRONOUS;
+
/* Get a superblock - note that we may end up sharing one that already exists */
s = sget(fs_type, compare_super, nfs_set_super, &sb_mntdata);
if (IS_ERR(s)) {
@@ -2361,6 +2362,10 @@ nfs_xdev_mount(struct file_system_type *fs_type, int flags,
if (server->flags & NFS_MOUNT_UNSHARED)
compare_super = NULL;
+ /* -o noac implies -o sync */
+ if (server->flags & NFS_MOUNT_NOAC)
+ sb_mntdata.mntflags |= MS_SYNCHRONOUS;
+
/* Get a superblock - note that we may end up sharing one that already exists */
s = sget(&nfs_fs_type, compare_super, nfs_set_super, &sb_mntdata);
if (IS_ERR(s)) {
@@ -2628,6 +2633,10 @@ nfs4_remote_mount(struct file_system_type *fs_type, int flags,
if (server->flags & NFS4_MOUNT_UNSHARED)
compare_super = NULL;
+ /* -o noac implies -o sync */
+ if (server->flags & NFS_MOUNT_NOAC)
+ sb_mntdata.mntflags |= MS_SYNCHRONOUS;
+
/* Get a superblock - note that we may end up sharing one that already exists */
s = sget(&nfs4_fs_type, compare_super, nfs_set_super, &sb_mntdata);
if (IS_ERR(s)) {
@@ -2916,6 +2925,10 @@ nfs4_xdev_mount(struct file_system_type *fs_type, int flags,
if (server->flags & NFS4_MOUNT_UNSHARED)
compare_super = NULL;
+ /* -o noac implies -o sync */
+ if (server->flags & NFS_MOUNT_NOAC)
+ sb_mntdata.mntflags |= MS_SYNCHRONOUS;
+
/* Get a superblock - note that we may end up sharing one that already exists */
s = sget(&nfs4_fs_type, compare_super, nfs_set_super, &sb_mntdata);
if (IS_ERR(s)) {
@@ -3003,6 +3016,10 @@ nfs4_remote_referral_mount(struct file_system_type *fs_type, int flags,
if (server->flags & NFS4_MOUNT_UNSHARED)
compare_super = NULL;
+ /* -o noac implies -o sync */
+ if (server->flags & NFS_MOUNT_NOAC)
+ sb_mntdata.mntflags |= MS_SYNCHRONOUS;
+
/* Get a superblock - note that we may end up sharing one that already exists */
s = sget(&nfs4_fs_type, compare_super, nfs_set_super, &sb_mntdata);
if (IS_ERR(s)) {