Automounter maps can contain mount options valid for other NFS
implementations but not for Linux. The Linux automounter uses the
mount command's "-s" command line option ("s" for "sloppy") so that
mount requests containing such options are not rejected.
Commit f45663ce5fb30f76a3414ab3ac69f4dd320e760a attempted to address a
known regression with text-based NFS mount option parsing. Unrecognized
mount options would cause mount requests to fail, even if the "-s"
option was used on the mount command line.
Unfortunately, this commit was not complete as submitted. It adds a
new mount option, "sloppy". But it is missing a hunk, so it now allows
NFS mounts with unrecognized mount options, even if the "sloppy" option
is not present. This could be a problem if a required critical mount
option such as "sync" is misspelled, for example, and is considered a
regression from 2.6.26.
This patch restores the missing hunk. Now, the default behavior of
text-based NFS mount options is as before: any unrecognized mount option
will cause the mount to fail.
Please include this in 2.6.27-rc.
Thanks to Neil Brown for reporting this.
Signed-off-by: Chuck Lever <[email protected]>
Acked-by: J. Bruce Fields <[email protected]>
---
fs/nfs/super.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 5b2aa04..e3ac650 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1279,6 +1279,12 @@ static int nfs_parse_mount_options(char *raw,
}
}
+ if (errors > 0) {
+ dfprintk(MOUNT, "NFS: parsing encountered %d error%s\n",
+ errors, (errors == 1 ? "" : "s"));
+ if (!sloppy)
+ return 0;
+ }
return 1;
out_nomem:
On Fri, Sep 05, 2008 at 02:16:07PM -0400, Chuck Lever wrote:
> Commit f45663ce5fb30f76a3414ab3ac69f4dd320e760a was missing a hunk that
> prevented the new "sloppy" mount option from having any effect.
I don't think that on its own would justify sending it in for 2.6.27.[1]
>
> Tested against 2.6.27-rc. 2.6.26 is not affected.
But if I understand right, the effect of leaving out this chunk was to
make the *default* behavior "sloppy"? Which seems a drastic change from
the previous behavior. And it's a simple enough patch.
So I'd be inclined to agree and send it in for 2.6.27.... If Trond
doesn't poke his head up by tomorrow, let's go ahead--feel free to send
it to Linus with my
Acked-by: J. Bruce Fields <[email protected]>
if I fall off the face of the earth tomorrow.
--b.
[1] Linus has been pretty hard on -rc patches lately:
http://marc.info/?l=linux-netdev&m=122048427801318&w=2
"Here's a simple rule of thumb:
- if it's not on the regression list
- if it's not a reported security hole
- if it's not on the reported oopses list
then why are people sending it to me?"
>
> Thanks to Neil Brown for reporting this.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>
> fs/nfs/super.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 5b2aa04..e3ac650 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1279,6 +1279,12 @@ static int nfs_parse_mount_options(char *raw,
> }
> }
>
> + if (errors > 0) {
> + dfprintk(MOUNT, "NFS: parsing encountered %d error%s\n",
> + errors, (errors == 1 ? "" : "s"));
> + if (!sloppy)
> + return 0;
> + }
> return 1;
>
> out_nomem:
>
On Fri, Sep 5, 2008 at 5:11 PM, J. Bruce Fields <[email protected]> wrote:
> On Fri, Sep 05, 2008 at 02:16:07PM -0400, Chuck Lever wrote:
>> Commit f45663ce5fb30f76a3414ab3ac69f4dd320e760a was missing a hunk that
>> prevented the new "sloppy" mount option from having any effect.
>
> I don't think that on its own would justify sending it in for 2.6.27.[1]
The original patch for 27 was supposed to fix a regression (ie
automounter stopped working in heterogenous environments). This patch
does fix the full regression.
There is already logic in nfs-utils-1.1.3 that maps the "-s" option
(which has been around for EVAR) to "-o sloppy". This logic is
enabled for 2.6.27 kernels and later. So this does need to go in 27.
Would it help if I rewrote the description?
>> Tested against 2.6.27-rc. 2.6.26 is not affected.
>
> But if I understand right, the effect of leaving out this chunk was to
> make the *default* behavior "sloppy"? Which seems a drastic change from
> the previous behavior. And it's a simple enough patch.
No, the default behavior is as before. The behavior without this
patch is that the kernel recognizes "sloppy" but it doesn't do
anything about it.
> So I'd be inclined to agree and send it in for 2.6.27.... If Trond
> doesn't poke his head up by tomorrow, let's go ahead--feel free to send
> it to Linus with my
>
> Acked-by: J. Bruce Fields <[email protected]>
>
> if I fall off the face of the earth tomorrow.
>
> --b.
>
> [1] Linus has been pretty hard on -rc patches lately:
>
> http://marc.info/?l=linux-netdev&m=122048427801318&w=2
>
> "Here's a simple rule of thumb:
> - if it's not on the regression list
> - if it's not a reported security hole
> - if it's not on the reported oopses list
> then why are people sending it to me?"
>>
>> Thanks to Neil Brown for reporting this.
>>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>>
>> fs/nfs/super.c | 6 ++++++
>> 1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index 5b2aa04..e3ac650 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -1279,6 +1279,12 @@ static int nfs_parse_mount_options(char *raw,
>> }
>> }
>>
>> + if (errors > 0) {
>> + dfprintk(MOUNT, "NFS: parsing encountered %d error%s\n",
>> + errors, (errors == 1 ? "" : "s"));
>> + if (!sloppy)
>> + return 0;
>> + }
>> return 1;
>>
>> out_nomem:
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
"If you simplify your English, you are freed from the worst follies of
orthodoxy."
-- George Orwell
On Fri, Sep 05, 2008 at 05:25:31PM -0400, Chuck Lever wrote:
> On Fri, Sep 5, 2008 at 5:11 PM, J. Bruce Fields <[email protected]> wrote:
> > On Fri, Sep 05, 2008 at 02:16:07PM -0400, Chuck Lever wrote:
> >> Commit f45663ce5fb30f76a3414ab3ac69f4dd320e760a was missing a hunk that
> >> prevented the new "sloppy" mount option from having any effect.
> >
> > I don't think that on its own would justify sending it in for 2.6.27.[1]
>
> The original patch for 27 was supposed to fix a regression (ie
> automounter stopped working in heterogenous environments). This patch
> does fix the full regression.
>
> There is already logic in nfs-utils-1.1.3 that maps the "-s" option
> (which has been around for EVAR) to "-o sloppy". This logic is
> enabled for 2.6.27 kernels and later. So this does need to go in 27.
>
> Would it help if I rewrote the description?
Could be. Assume I'm tired and stupid....
> >> Tested against 2.6.27-rc. 2.6.26 is not affected.
> >
> > But if I understand right, the effect of leaving out this chunk was to
> > make the *default* behavior "sloppy"? Which seems a drastic change from
> > the previous behavior. And it's a simple enough patch.
>
> No, the default behavior is as before. The behavior without this
> patch is that the kernel recognizes "sloppy" but it doesn't do
> anything about it.
That can't be right:
> >> + if (errors > 0) {
> >> + dfprintk(MOUNT, "NFS: parsing encountered %d error%s\n",
> >> + errors, (errors == 1 ? "" : "s"));
> >> + if (!sloppy)
> >> + return 0;
> >> + }
> >> return 1;
Ignoring the printk, the *only* change in behavior here happens when
sloppy is *not* set. Right?
--b. (not a big fan of not's)
On Sep 5, 2008, at Sep 5, 2008, 5:34 PM, J. Bruce Fields wrote:
> On Fri, Sep 05, 2008 at 05:25:31PM -0400, Chuck Lever wrote:
>> On Fri, Sep 5, 2008 at 5:11 PM, J. Bruce Fields
>> <[email protected]> wrote:
>>> On Fri, Sep 05, 2008 at 02:16:07PM -0400, Chuck Lever wrote:
>>>> Commit f45663ce5fb30f76a3414ab3ac69f4dd320e760a was missing a
>>>> hunk that
>>>> prevented the new "sloppy" mount option from having any effect.
>>>
>>> I don't think that on its own would justify sending it in for
>>> 2.6.27.[1]
>>
>> The original patch for 27 was supposed to fix a regression (ie
>> automounter stopped working in heterogenous environments). This
>> patch
>> does fix the full regression.
>>
>> There is already logic in nfs-utils-1.1.3 that maps the "-s" option
>> (which has been around for EVAR) to "-o sloppy". This logic is
>> enabled for 2.6.27 kernels and later. So this does need to go in 27.
>>
>> Would it help if I rewrote the description?
>
> Could be. Assume I'm tired and stupid....
>
>>>> Tested against 2.6.27-rc. 2.6.26 is not affected.
>>>
>>> But if I understand right, the effect of leaving out this chunk
>>> was to
>>> make the *default* behavior "sloppy"? Which seems a drastic
>>> change from
>>> the previous behavior. And it's a simple enough patch.
>>
>> No, the default behavior is as before. The behavior without this
>> patch is that the kernel recognizes "sloppy" but it doesn't do
>> anything about it.
>
> That can't be right:
>
>>>> + if (errors > 0) {
>>>> + dfprintk(MOUNT, "NFS: parsing encountered %d error%s
>>>> \n",
>>>> + errors, (errors == 1 ? "" : "s"));
>>>> + if (!sloppy)
>>>> + return 0;
>>>> + }
>>>> return 1;
>
> Ignoring the printk, the *only* change in behavior here happens when
> sloppy is *not* set. Right?
Hrm. Without today's patch, the behavior is always sloppy, and
"sloppy" is recognized as a valid option (not that it matters). With
the patch, the default behavior is as before, and the sloppy option is
recognized.
So yes, this is a regression in both senses.
I'll rewrite the description and repost.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com