* [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac
@ 2011-07-29 13:50 Sachin Prabhu
2011-07-30 18:33 ` Trond Myklebust
2011-08-01 11:10 ` Sachin Prabhu
0 siblings, 2 replies; 6+ messages in thread
From: Sachin Prabhu @ 2011-07-29 13:50 UTC (permalink / raw)
To: NFS
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 <sprabhu@redhat.com>
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)) {
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac
2011-07-29 13:50 [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac Sachin Prabhu
@ 2011-07-30 18:33 ` Trond Myklebust
2011-08-01 11:13 ` Sachin Prabhu
2011-08-01 11:10 ` Sachin Prabhu
1 sibling, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2011-07-30 18:33 UTC (permalink / raw)
To: Sachin Prabhu; +Cc: NFS
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 <sprabhu@redhat.com>
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
Trond.Myklebust@netapp.com
www.netapp.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac
2011-07-30 18:33 ` Trond Myklebust
@ 2011-08-01 11:13 ` Sachin Prabhu
0 siblings, 0 replies; 6+ messages in thread
From: Sachin Prabhu @ 2011-08-01 11:13 UTC (permalink / raw)
To: Trond Myklebust; +Cc: NFS
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 <sprabhu@redhat.com>
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac
2011-07-29 13:50 [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac Sachin Prabhu
2011-07-30 18:33 ` Trond Myklebust
@ 2011-08-01 11:10 ` Sachin Prabhu
2011-08-01 11:19 ` Christoph Hellwig
1 sibling, 1 reply; 6+ messages in thread
From: Sachin Prabhu @ 2011-08-01 11:10 UTC (permalink / raw)
To: NFS
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 <sprabhu@redhat.com>
Reviewed-by: Jeff Layton <jlayton@redhat.com>
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)) {
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac
2011-08-01 11:10 ` Sachin Prabhu
@ 2011-08-01 11:19 ` Christoph Hellwig
2011-08-01 13:29 ` Sachin Prabhu
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2011-08-01 11:19 UTC (permalink / raw)
To: Sachin Prabhu; +Cc: NFS
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac
2011-08-01 11:19 ` Christoph Hellwig
@ 2011-08-01 13:29 ` Sachin Prabhu
0 siblings, 0 replies; 6+ messages in thread
From: Sachin Prabhu @ 2011-08-01 13:29 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: NFS
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-08-01 13:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-29 13:50 [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac Sachin Prabhu
2011-07-30 18:33 ` Trond Myklebust
2011-08-01 11:13 ` Sachin Prabhu
2011-08-01 11:10 ` Sachin Prabhu
2011-08-01 11:19 ` Christoph Hellwig
2011-08-01 13:29 ` Sachin Prabhu
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).