[PATCH] vfs: parse sloppy mount option in correct order

[PATCH] vfs: parse sloppy mount option in correct order

[Roberto Bergantinos Corpas]

With addition of fs_context support, options string is parsed
sequentially, if 'sloppy' option is not leftmost one, we may
return ENOPARAM to userland if a non-valid option preceeds sloopy
and mount will fail :

host# mount -o quota,sloppy 172.23.1.225:/share /mnt
mount.nfs: an incorrect mount option was specified
host# mount -o sloppy,quota 172.23.1.225:/share /mnt
host#

This patch correct that behaviour so that sloppy takes precedence
if specified anywhere on the string

Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
---
 fs/fs_context.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/fs_context.c b/fs/fs_context.c
index de1985eae535..f930808e9db8 100644
--- a/fs/fs_context.c
+++ b/fs/fs_context.c
@@ -210,8 +210,12 @@ int generic_parse_monolithic(struct fs_context *fc, void *data)
 	if (ret)
 		return ret;
 
+	/* 'sloppy' should be parsed first */
+	if (strstr((const char *)data, "sloppy") != NULL)
+		vfs_parse_fs_string(fc, "sloppy", NULL, 0);
+
 	while ((key = strsep(&options, ",")) != NULL) {
-		if (*key) {
+		if (*key && strcmp(key, "sloppy")) {
 			size_t v_len = 0;
 			char *value = strchr(key, '=');
 
-- 
2.31.1

Re: [PATCH] vfs: parse sloppy mount option in correct order

[David Howells]

Roberto Bergantinos Corpas <rbergant@redhat.com> wrote:

> With addition of fs_context support, options string is parsed
> sequentially, if 'sloppy' option is not leftmost one, we may
> return ENOPARAM to userland if a non-valid option preceeds sloopy
> and mount will fail :
> 
> host# mount -o quota,sloppy 172.23.1.225:/share /mnt
> mount.nfs: an incorrect mount option was specified
> host# mount -o sloppy,quota 172.23.1.225:/share /mnt
> host#
> 
> This patch correct that behaviour so that sloppy takes precedence
> if specified anywhere on the string

It's slightly overcorrected, but that probably doesn't matter.

I wonder if we should put a "bool sloppy" in struct fs_context, put the
handling in vfs_parse_fs_param() (ie. skip the error message).

David

Re: [PATCH] vfs: parse sloppy mount option in correct order

[Roberto Bergantinos Corpas]

David,

  i came up with the following approach, i drop the sloppy booleans
from cifs/nfs and leave it at fs_context level

, let me know what you think and ill send as new version :

diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index 7ec35f3f0a5f..5a8c074df74a 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -866,7 +866,7 @@ static int smb3_fs_context_parse_param(struct
fs_context *fc,
     if (!skip_parsing) {
         opt = fs_parse(fc, smb3_fs_parameters, param, &result);
         if (opt < 0)
-            return ctx->sloppy ? 1 : opt;
+            return fc->sloppy ? 1 : opt;
     }

     switch (opt) {
@@ -1412,7 +1412,7 @@ static int smb3_fs_context_parse_param(struct
fs_context *fc,
         ctx->multiuser = true;
         break;
     case Opt_sloppy:
-        ctx->sloppy = true;
+        fc->sloppy = true;
         break;
     case Opt_nosharesock:
         ctx->nosharesock = true;
diff --git a/fs/cifs/fs_context.h b/fs/cifs/fs_context.h
index e54090d9ef36..52a67a96fb67 100644
--- a/fs/cifs/fs_context.h
+++ b/fs/cifs/fs_context.h
@@ -155,7 +155,6 @@ struct smb3_fs_context {
     bool uid_specified;
     bool cruid_specified;
     bool gid_specified;
-    bool sloppy;
     bool got_ip;
     bool got_version;
     bool got_rsize;
diff --git a/fs/fs_context.c b/fs/fs_context.c
index 24ce12f0db32..3c1c999fc1fb 100644
--- a/fs/fs_context.c
+++ b/fs/fs_context.c
@@ -155,8 +155,14 @@ int vfs_parse_fs_param(struct fs_context *fc,
struct fs_parameter *param)
     if (ret != -ENOPARAM)
         return ret;

-    return invalf(fc, "%s: Unknown parameter '%s'",
-              fc->fs_type->name, param->key);
+    /* We got an invalid parameter, but sloppy may have been specified
+     * later on param string. Let's wait to process whole params to return
+     * EINVAL */
+
+    fc->param_inval = true;
+    errorf(fc, "%s: Unknown parameter '%s'",fc->fs_type->name, param->key);
+
+    return 0;
 }
 EXPORT_SYMBOL(vfs_parse_fs_param);

@@ -227,6 +233,9 @@ int generic_parse_monolithic(struct fs_context
*fc, void *data)
         }
     }

+    if (!fc->sloppy && fc->param_inval)
+        ret = -EINVAL;
+
     return ret;
 }
 EXPORT_SYMBOL(generic_parse_monolithic);
diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index ea17fa1f31ec..c9ff68e17b68 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -482,7 +482,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,

     opt = fs_parse(fc, nfs_fs_parameters, param, &result);
     if (opt < 0)
-        return ctx->sloppy ? 1 : opt;
+        return fc->sloppy ? 1 : opt;

     if (fc->security)
         ctx->has_sec_mnt_opts = 1;
@@ -837,7 +837,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
          * Special options
          */
     case Opt_sloppy:
-        ctx->sloppy = true;
+        fc->sloppy = true;
         dfprintk(MOUNT, "NFS:   relaxing parsing rules\n");
         break;
     }
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 12f6acb483bb..9febdc95b4d0 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -80,7 +80,6 @@ struct nfs_fs_context {
     bool            internal;
     bool            skip_reconfig_option_check;
     bool            need_mount;
-    bool            sloppy;
     unsigned int        flags;        /* NFS{,4}_MOUNT_* flags */
     unsigned int        rsize, wsize;
     unsigned int        timeo, retrans;
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index 13fa6f3df8e4..06a4b72a0f98 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -110,6 +110,8 @@ struct fs_context {
     bool            need_free:1;    /* Need to call ops->free() */
     bool            global:1;    /* Goes into &init_user_ns */
     bool            oldapi:1;    /* Coming from mount(2) */
+    bool                    sloppy:1;       /* If fs support it and
was specified */
+    bool                    param_inval:1;  /* If set, check sloppy value */
 };

 struct fs_context_operations {

On Thu, Aug 26, 2021 at 2:29 PM David Howells <dhowells@redhat.com> wrote:
>
> Roberto Bergantinos Corpas <rbergant@redhat.com> wrote:
>
> > With addition of fs_context support, options string is parsed
> > sequentially, if 'sloppy' option is not leftmost one, we may
> > return ENOPARAM to userland if a non-valid option preceeds sloopy
> > and mount will fail :
> >
> > host# mount -o quota,sloppy 172.23.1.225:/share /mnt
> > mount.nfs: an incorrect mount option was specified
> > host# mount -o sloppy,quota 172.23.1.225:/share /mnt
> > host#
> >
> > This patch correct that behaviour so that sloppy takes precedence
> > if specified anywhere on the string
>
> It's slightly overcorrected, but that probably doesn't matter.
>
> I wonder if we should put a "bool sloppy" in struct fs_context, put the
> handling in vfs_parse_fs_param() (ie. skip the error message).
>
> David
>

Re: [PATCH] vfs: parse sloppy mount option in correct order

[Matthew Wilcox]

On Wed, Jul 21, 2021 at 01:30:57PM +0200, Roberto Bergantinos Corpas wrote:
> With addition of fs_context support, options string is parsed
> sequentially, if 'sloppy' option is not leftmost one, we may
> return ENOPARAM to userland if a non-valid option preceeds sloopy
> and mount will fail :
> 
> host# mount -o quota,sloppy 172.23.1.225:/share /mnt
> mount.nfs: an incorrect mount option was specified
> host# mount -o sloppy,quota 172.23.1.225:/share /mnt

It isn't clear to me that this is incorrect behaviour.  Perhaps the user
actually wants the options to the left parsed strictly and the options
to the right parsed sloppily?

Re: [PATCH] vfs: parse sloppy mount option in correct order

[Eric Sandeen]

On 8/26/21 7:46 AM, Matthew Wilcox wrote:
> On Wed, Jul 21, 2021 at 01:30:57PM +0200, Roberto Bergantinos Corpas wrote:
>> With addition of fs_context support, options string is parsed
>> sequentially, if 'sloppy' option is not leftmost one, we may
>> return ENOPARAM to userland if a non-valid option preceeds sloopy
>> and mount will fail :
>>
>> host# mount -o quota,sloppy 172.23.1.225:/share /mnt
>> mount.nfs: an incorrect mount option was specified
>> host# mount -o sloppy,quota 172.23.1.225:/share /mnt
> 
> It isn't clear to me that this is incorrect behaviour.  Perhaps the user
> actually wants the options to the left parsed strictly and the options
> to the right parsed sloppily?

I don't think mount options have ever been order-dependent, at least not
intentionally so, have they?

And what matters most here is surely "how did it work before the mount
API change?"

And it seems to me that previously, invalid options were noted, and whether the
mount would fail was left to the end, depending on whether sloppy was seen
anywhere in the mount options string.  This is the old option parsing:

         while ((p = strsep(&raw, ",")) != NULL) {
...
                 switch (token) {
...
                 case Opt_sloppy:
                         sloppy = 1;
                         dfprintk(MOUNT, "NFS:   relaxing parsing rules\n");
                         break;
...
                 default:
                         invalid_option = 1;
                         dfprintk(MOUNT, "NFS:   unrecognized mount option "
                                         "'%s'\n", p);
                 }
         }

         if (!sloppy && invalid_option)
                 return 0;

-Eric

Re: [PATCH] vfs: parse sloppy mount option in correct order

[Roberto Bergantinos Corpas]

On Thu, Aug 26, 2021 at 3:13 PM Eric Sandeen <sandeen@sandeen.net> wrote:
>
> On 8/26/21 7:46 AM, Matthew Wilcox wrote:
> > On Wed, Jul 21, 2021 at 01:30:57PM +0200, Roberto Bergantinos Corpas wrote:
> >> With addition of fs_context support, options string is parsed
> >> sequentially, if 'sloppy' option is not leftmost one, we may
> >> return ENOPARAM to userland if a non-valid option preceeds sloopy
> >> and mount will fail :
> >>
> >> host# mount -o quota,sloppy 172.23.1.225:/share /mnt
> >> mount.nfs: an incorrect mount option was specified
> >> host# mount -o sloppy,quota 172.23.1.225:/share /mnt
> >
> > It isn't clear to me that this is incorrect behaviour.  Perhaps the user
> > actually wants the options to the left parsed strictly and the options
> > to the right parsed sloppily?
>
> I don't think mount options have ever been order-dependent, at least not
> intentionally so, have they?
>
> And what matters most here is surely "how did it work before the mount
> API change?"
>> And it seems to me that previously, invalid options were noted, and whether the
> mount would fail was left to the end, depending on whether sloppy was seen
> anywhere in the mount options string.  This is the old option parsing:
>
>          while ((p = strsep(&raw, ",")) != NULL) {
> ...
>                  switch (token) {
> ...
>                  case Opt_sloppy:
>                          sloppy = 1;
>                          dfprintk(MOUNT, "NFS:   relaxing parsing rules\n");
>                          break;
> ...
>                  default:
>                          invalid_option = 1;
>                          dfprintk(MOUNT, "NFS:   unrecognized mount option "
>                                          "'%s'\n", p);
>                  }
>          }
>
>          if (!sloppy && invalid_option)
>                  return 0;

 Agree, that's my main point. I think that breaks from previous
behaviour and indeed causes issues on the field.
My understanding too is that there's no order-dependency.

roberto

>
> -Eric
>

Re: [PATCH] vfs: parse sloppy mount option in correct order

[David Wysochanski]

On Thu, Aug 26, 2021 at 9:32 AM Roberto Bergantinos Corpas
<rbergant@redhat.com> wrote:
>
> On Thu, Aug 26, 2021 at 3:13 PM Eric Sandeen <sandeen@sandeen.net> wrote:
> >
> > On 8/26/21 7:46 AM, Matthew Wilcox wrote:
> > > On Wed, Jul 21, 2021 at 01:30:57PM +0200, Roberto Bergantinos Corpas wrote:
> > >> With addition of fs_context support, options string is parsed
> > >> sequentially, if 'sloppy' option is not leftmost one, we may
> > >> return ENOPARAM to userland if a non-valid option preceeds sloopy
> > >> and mount will fail :
> > >>
> > >> host# mount -o quota,sloppy 172.23.1.225:/share /mnt
> > >> mount.nfs: an incorrect mount option was specified
> > >> host# mount -o sloppy,quota 172.23.1.225:/share /mnt
> > >
> > > It isn't clear to me that this is incorrect behaviour.  Perhaps the user
> > > actually wants the options to the left parsed strictly and the options
> > > to the right parsed sloppily?
> >
> > I don't think mount options have ever been order-dependent, at least not
> > intentionally so, have they?
> >
> > And what matters most here is surely "how did it work before the mount
> > API change?"
> >> And it seems to me that previously, invalid options were noted, and whether the
> > mount would fail was left to the end, depending on whether sloppy was seen
> > anywhere in the mount options string.  This is the old option parsing:
> >
> >          while ((p = strsep(&raw, ",")) != NULL) {
> > ...
> >                  switch (token) {
> > ...
> >                  case Opt_sloppy:
> >                          sloppy = 1;
> >                          dfprintk(MOUNT, "NFS:   relaxing parsing rules\n");
> >                          break;
> > ...
> >                  default:
> >                          invalid_option = 1;
> >                          dfprintk(MOUNT, "NFS:   unrecognized mount option "
> >                                          "'%s'\n", p);
> >                  }
> >          }
> >
> >          if (!sloppy && invalid_option)
> >                  return 0;
>
>  Agree, that's my main point. I think that breaks from previous
> behaviour and indeed causes issues on the field.
> My understanding too is that there's no order-dependency.
>
> roberto
>
Hi Roberto, Eric and David H, this seems to have stalled.  What are
the next steps here?

Roberto, it sounded like David H was suggesting maybe an alternative
approach.  Did you look into that?

Then there was a discussion about whether this change was intended
behavior or not, and the main point made was it was a change in
behavior, which I don't think was intended.  I guess one could make
the argument order was never guaranteed, but then why make behavior
order dependent now if it was not intentional kernel change?  I agree
if possible we should retain the order independent behavior in the
kernel.

Re: [PATCH] vfs: parse sloppy mount option in correct order

[Roberto Bergantinos Corpas]

> Roberto, it sounded like David H was suggesting maybe an alternative
> approach.  Did you look into that?

No, sorry about that, it is in my todo list. i'll take a look at it

[PATCH] vfs: parse sloppy mount option in correct order

[Ian Kent]

From: Roberto Bergantinos Corpas <rbergant@redhat.com>

With addition of fs_context support, options string is parsed
sequentially, if 'sloppy' option is not leftmost one, we may
return ENOPARAM to userland if a non-valid option preceeds sloopy
and mount will fail :

host# mount -o quota,sloppy 172.23.1.225:/share /mnt
mount.nfs: an incorrect mount option was specified
host# mount -o sloppy,quota 172.23.1.225:/share /mnt
host#

This patch correct that behaviour so that sloppy takes precedence
if specified anywhere on the string

changes since v1:
- add a boolean to fs context and postpone error reporting until
  parsing is done.

Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
---
 fs/cifs/fs_context.c       |    4 ++--
 fs/cifs/fs_context.h       |    1 -
 fs/fs_context.c            |   14 ++++++++++++--
 fs/nfs/fs_context.c        |    5 +++--
 fs/nfs/internal.h          |    1 -
 include/linux/fs_context.h |    2 ++
 6 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index 0e13dec86b25..32c3fdd7d27a 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -864,7 +864,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 	if (!skip_parsing) {
 		opt = fs_parse(fc, smb3_fs_parameters, param, &result);
 		if (opt < 0)
-			return ctx->sloppy ? 1 : opt;
+			return fc->sloppy ? 1 : opt;
 	}
 
 	switch (opt) {
@@ -1420,7 +1420,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 		ctx->multiuser = true;
 		break;
 	case Opt_sloppy:
-		ctx->sloppy = true;
+		fc->sloppy = true;
 		break;
 	case Opt_nosharesock:
 		ctx->nosharesock = true;
diff --git a/fs/cifs/fs_context.h b/fs/cifs/fs_context.h
index bbaee4c2281f..75e4c41466fa 100644
--- a/fs/cifs/fs_context.h
+++ b/fs/cifs/fs_context.h
@@ -157,7 +157,6 @@ struct smb3_fs_context {
 	bool uid_specified;
 	bool cruid_specified;
 	bool gid_specified;
-	bool sloppy;
 	bool got_ip;
 	bool got_version;
 	bool got_rsize;
diff --git a/fs/fs_context.c b/fs/fs_context.c
index df04e5fc6d66..911a36bf2226 100644
--- a/fs/fs_context.c
+++ b/fs/fs_context.c
@@ -157,8 +157,15 @@ int vfs_parse_fs_param(struct fs_context *fc, struct fs_parameter *param)
 	if (ret != -ENOPARAM)
 		return ret;
 
-	return invalf(fc, "%s: Unknown parameter '%s'",
-		      fc->fs_type->name, param->key);
+	/* We got an invalid parameter, but sloppy may have been specified
+	 * later on param string.
+	 * Let's wait to process whole params to return EINVAL.
+	 */
+
+	fc->param_inval = true;
+	errorf(fc, "%s: Unknown parameter '%s'", fc->fs_type->name, param->key);
+
+	return 0;
 }
 EXPORT_SYMBOL(vfs_parse_fs_param);
 
@@ -234,6 +241,9 @@ int generic_parse_monolithic(struct fs_context *fc, void *data)
 		}
 	}
 
+	if (!fc->sloppy && fc->param_inval)
+		ret = -EINVAL;
+
 	return ret;
 }
 EXPORT_SYMBOL(generic_parse_monolithic);
diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 4da701fd1424..09da63cc84f7 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -485,7 +485,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 
 	opt = fs_parse(fc, nfs_fs_parameters, param, &result);
 	if (opt < 0)
-		return (opt == -ENOPARAM && ctx->sloppy) ? 1 : opt;
+		return (opt == -ENOPARAM && fc->sloppy) ? 1 : opt;
 
 	if (fc->security)
 		ctx->has_sec_mnt_opts = 1;
@@ -853,7 +853,8 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 		 * Special options
 		 */
 	case Opt_sloppy:
-		ctx->sloppy = true;
+		fc->sloppy = true;
+		dfprintk(MOUNT, "NFS:   relaxing parsing rules\n");
 		break;
 	}
 
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 898dd95bc7a7..83552def96f1 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -90,7 +90,6 @@ struct nfs_fs_context {
 	bool			internal;
 	bool			skip_reconfig_option_check;
 	bool			need_mount;
-	bool			sloppy;
 	unsigned int		flags;		/* NFS{,4}_MOUNT_* flags */
 	unsigned int		rsize, wsize;
 	unsigned int		timeo, retrans;
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index ff1375a16c8c..d91d42bc06ce 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -111,6 +111,8 @@ struct fs_context {
 	bool			need_free:1;	/* Need to call ops->free() */
 	bool			global:1;	/* Goes into &init_user_ns */
 	bool			oldapi:1;	/* Coming from mount(2) */
+	bool                    sloppy:1;       /* If fs support it and was specified */
+	bool                    param_inval:1;  /* If set, check sloppy value */
 };
 
 struct fs_context_operations {

Re: [PATCH] vfs: parse sloppy mount option in correct order

[Jeff Layton]

On Wed, 2022-09-28 at 09:09 +0800, Ian Kent wrote:
> From: Roberto Bergantinos Corpas <rbergant@redhat.com>
> 
> With addition of fs_context support, options string is parsed
> sequentially, if 'sloppy' option is not leftmost one, we may
> return ENOPARAM to userland if a non-valid option preceeds sloopy
> and mount will fail :
> 
> host# mount -o quota,sloppy 172.23.1.225:/share /mnt
> mount.nfs: an incorrect mount option was specified
> host# mount -o sloppy,quota 172.23.1.225:/share /mnt
> host#
> 
> This patch correct that behaviour so that sloppy takes precedence
> if specified anywhere on the string
> 
> changes since v1:
> - add a boolean to fs context and postpone error reporting until
>   parsing is done.
> 
> Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
> ---
>  fs/cifs/fs_context.c       |    4 ++--
>  fs/cifs/fs_context.h       |    1 -
>  fs/fs_context.c            |   14 ++++++++++++--
>  fs/nfs/fs_context.c        |    5 +++--
>  fs/nfs/internal.h          |    1 -
>  include/linux/fs_context.h |    2 ++
>  6 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> index 0e13dec86b25..32c3fdd7d27a 100644
> --- a/fs/cifs/fs_context.c
> +++ b/fs/cifs/fs_context.c
> @@ -864,7 +864,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
>  	if (!skip_parsing) {
>  		opt = fs_parse(fc, smb3_fs_parameters, param, &result);
>  		if (opt < 0)
> -			return ctx->sloppy ? 1 : opt;
> +			return fc->sloppy ? 1 : opt;
>  	}
>  
>  	switch (opt) {
> @@ -1420,7 +1420,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
>  		ctx->multiuser = true;
>  		break;
>  	case Opt_sloppy:
> -		ctx->sloppy = true;
> +		fc->sloppy = true;
>  		break;
>  	case Opt_nosharesock:
>  		ctx->nosharesock = true;
> diff --git a/fs/cifs/fs_context.h b/fs/cifs/fs_context.h
> index bbaee4c2281f..75e4c41466fa 100644
> --- a/fs/cifs/fs_context.h
> +++ b/fs/cifs/fs_context.h
> @@ -157,7 +157,6 @@ struct smb3_fs_context {
>  	bool uid_specified;
>  	bool cruid_specified;
>  	bool gid_specified;
> -	bool sloppy;
>  	bool got_ip;
>  	bool got_version;
>  	bool got_rsize;
> diff --git a/fs/fs_context.c b/fs/fs_context.c
> index df04e5fc6d66..911a36bf2226 100644
> --- a/fs/fs_context.c
> +++ b/fs/fs_context.c
> @@ -157,8 +157,15 @@ int vfs_parse_fs_param(struct fs_context *fc, struct fs_parameter *param)
>  	if (ret != -ENOPARAM)
>  		return ret;
>  
> -	return invalf(fc, "%s: Unknown parameter '%s'",
> -		      fc->fs_type->name, param->key);
> +	/* We got an invalid parameter, but sloppy may have been specified
> +	 * later on param string.
> +	 * Let's wait to process whole params to return EINVAL.
> +	 */
> +
> +	fc->param_inval = true;
> +	errorf(fc, "%s: Unknown parameter '%s'", fc->fs_type->name, param->key);

Is it correct to store an error message when we don't know whether
"sloppy" has been specified yet?

> +
> +	return 0;
>  }
>  EXPORT_SYMBOL(vfs_parse_fs_param);
>  
> @@ -234,6 +241,9 @@ int generic_parse_monolithic(struct fs_context *fc, void *data)
>  		}
>  	}
>  
> +	if (!fc->sloppy && fc->param_inval)
> +		ret = -EINVAL;
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL(generic_parse_monolithic);
> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> index 4da701fd1424..09da63cc84f7 100644
> --- a/fs/nfs/fs_context.c
> +++ b/fs/nfs/fs_context.c
> @@ -485,7 +485,7 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
>  
>  	opt = fs_parse(fc, nfs_fs_parameters, param, &result);
>  	if (opt < 0)
> -		return (opt == -ENOPARAM && ctx->sloppy) ? 1 : opt;
> +		return (opt == -ENOPARAM && fc->sloppy) ? 1 : opt;
>  
>  	if (fc->security)
>  		ctx->has_sec_mnt_opts = 1;
> @@ -853,7 +853,8 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
>  		 * Special options
>  		 */
>  	case Opt_sloppy:
> -		ctx->sloppy = true;
> +		fc->sloppy = true;
> +		dfprintk(MOUNT, "NFS:   relaxing parsing rules\n");
>  		break;
>  	}
>  
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 898dd95bc7a7..83552def96f1 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -90,7 +90,6 @@ struct nfs_fs_context {
>  	bool			internal;
>  	bool			skip_reconfig_option_check;
>  	bool			need_mount;
> -	bool			sloppy;
>  	unsigned int		flags;		/* NFS{,4}_MOUNT_* flags */
>  	unsigned int		rsize, wsize;
>  	unsigned int		timeo, retrans;
> diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
> index ff1375a16c8c..d91d42bc06ce 100644
> --- a/include/linux/fs_context.h
> +++ b/include/linux/fs_context.h
> @@ -111,6 +111,8 @@ struct fs_context {
>  	bool			need_free:1;	/* Need to call ops->free() */
>  	bool			global:1;	/* Goes into &init_user_ns */
>  	bool			oldapi:1;	/* Coming from mount(2) */
> +	bool                    sloppy:1;       /* If fs support it and was specified */
> +	bool                    param_inval:1;  /* If set, check sloppy value */
>  };
>  
>  struct fs_context_operations {
> 
> 

Overall, the patch looks reasonable though.
-- 
Jeff Layton <jlayton@redhat.com>