2007-11-01 05:27:02

by NeilBrown

[permalink] [raw]
Subject: [PATCH nfs-utils] probe_port should not try other versions in a version was explicitly given.

Currently if probe port is asked to probe for a specific version, and
that version is not supported, it will probe other versions too.
This means that if mountd is run with "--no-nfs-version 3",
It will first probe for NFS version 3, which will succeed (assuming the
kernel supported NFSv3), then it will check the matching mountd version (3)
and probe_port on discovering that isn't supported will try other versions,
find "1" is supported will succeed.

This leaves up using mount version 1 for an NFSv3 mount, which doesn't work
and leads to a SIGSEGV

There is no case where trying other versions is needed the request one is
not supported, so simply remove that code.

Signed-off-by: Neil Brown <[email protected]>
---
utils/mount/network.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 63d5f5a..2b09457 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -417,10 +417,6 @@ static int probe_port(clnt_addr_t *server, const unsigned long *versions,
continue;
p_prot = protos;
}
- if (vers == pmap->pm_vers) {
- p_vers = versions;
- vers = 0;
- }
if (vers || !*++p_vers)
break;
}
--
1.5.3.4


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2007-11-05 09:58:45

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH nfs-utils 3/3] Fix bug in auto-exporting subordinate mounts.



Neil Brown wrote:
> When mountd gets a request to export a mountpoint which is not
> explicitly exported, but is below an export point that is flagged as
> "crossmnt", it passes the wrong path name to the kernel for the
> "filehandle -> directory"
> mapping.
> This can badly confuse the NFS client, and is certainly wrong.
>
> So use the correct path names.
>
> Signed-off-by: Neil Brown <[email protected]>
> ---
> utils/mountd/cache.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index ce1a5a9..fd317cd 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -508,7 +508,7 @@ void nfsd_fh(FILE *f)
> */
> qword_printint(f, 0x7fffffff);
> if (found)
> - qword_print(f, found->e_path);
> + qword_print(f, found_path);
> qword_eol(f);
> out:
> free(found_path);
Committed.

steved.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-11-05 09:59:43

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH nfs-utils 1/3] probe_port should not try other versions in a version was explicitly given.



Neil Brown wrote:
> Currently if probe port is asked to probe for a specific version, and
> that version is not supported, it will probe other versions too.
> This means that if mountd is run with "--no-nfs-version 3",
> It will first probe for NFS version 3, which will succeed (assuming the
> kernel supported NFSv3), then it will check the matching mountd version (3)
> and probe_port on discovering that isn't supported will try other versions,
> find "1" is supported will succeed.
>
> This leaves up using mount version 1 for an NFSv3 mount, which doesn't work
> and leads to a SIGSEGV
>
> There is no case where trying other versions is needed the request one is
> not supported, so simply remove that code.
>
> Signed-off-by: Neil Brown <[email protected]>
> ---
> utils/mount/network.c | 4 ----
> 1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index 63d5f5a..2b09457 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -417,10 +417,6 @@ static int probe_port(clnt_addr_t *server, const unsigned long *versions,
> continue;
> p_prot = protos;
> }
> - if (vers == pmap->pm_vers) {
> - p_vers = versions;
> - vers = 0;
> - }
> if (vers || !*++p_vers)
> break;
> }
Committed.

steved.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-11-05 11:02:56

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH nfs-utils 2/3] Correctly probe mount v2



Neil Brown wrote:
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index 2b09457..38d0135 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -494,7 +494,7 @@ int probe_bothports(clnt_addr_t *mnt_server, clnt_addr_t *nfs_server)
> for (; *probe_vers; probe_vers++) {
> nfs_pmap->pm_vers = mntvers_to_nfs(*probe_vers);
> if ((res = probe_nfsport(nfs_server) != 0)) {
> - mnt_pmap->pm_vers = nfsvers_to_mnt(nfs_pmap->pm_vers);
> + mnt_pmap->pm_vers = *probe_vers;
> if ((res = probe_mntport(mnt_server)) != 0)
> return 1;
> memcpy(mnt_pmap, &save_mnt, sizeof(*mnt_pmap));

This patch does indeed fix mounts in the case where servers only support
version 2 of the mount protocol. Meaning "mount server:/export /mnt/v2"
now work when mountd only registers version 2 of the mnt protocol.

But I happen to also notice that when setting the nfsvers
"mount -o vers=2 server:/export /mnt/v2"

the mount fails, because the mount command assumes version 1 (of the mnt
protocol) should be used and never tries any other versions. A similar
assumption is made on the umount side as well.

So I guess the question is, what correlation (if any) is there between
the mount version and nfs version that is used? Do nfsvers=2 mounts
have to use version 1 of the mnt protocol? What is the problem if
v3 mounts use version 2 of the mnt protocol?

My answers would be None, No and None to those questions, but maybe
I'm missing something...

Below is an RFC patch that removes these assumption:

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 38d0135..c929986 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -481,8 +481,12 @@ int probe_bothports(clnt_addr_t *mnt_server,
clnt_addr_t *nfs_server)

if (mnt_pmap->pm_vers && !nfs_pmap->pm_vers)
nfs_pmap->pm_vers = mntvers_to_nfs(mnt_pmap->pm_vers);
- else if (nfs_pmap->pm_vers && !mnt_pmap->pm_vers)
- mnt_pmap->pm_vers = nfsvers_to_mnt(nfs_pmap->pm_vers);
+ else if (nfs_pmap->pm_vers && !mnt_pmap->pm_vers) {
+ /* Does it matter which mount version is used
+ * when the nfs version is set ?
+ */
+ mnt_pmap->pm_vers = 0;
+ }
if (nfs_pmap->pm_vers)
goto version_fixed;

diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
index 285273b..4209514 100644
--- a/utils/mount/nfsumount.c
+++ b/utils/mount/nfsumount.c
@@ -195,12 +195,6 @@ static int do_nfs_umount23(const char *spec, char
*opts)
pmap->pm_prog = atoi(p+10);
if (opts && (p = strstr(opts, "mountport=")) && isdigit(*(p+10)))
pmap->pm_port = atoi(p+10);
- if (opts && hasmntopt(&mnt, "v2"))
- pmap->pm_vers = nfsvers_to_mnt(2);
- if (opts && hasmntopt(&mnt, "v3"))
- pmap->pm_vers = nfsvers_to_mnt(3);
- if (opts && (p = strstr(opts, "vers=")) && isdigit(*(p+5)))
- pmap->pm_vers = nfsvers_to_mnt(atoi(p+5));
if (opts && (p = strstr(opts, "mountvers=")) && isdigit(*(p+10)))
pmap->pm_vers = atoi(p+10);
if (opts && (hasmntopt(&mnt, "udp") || hasmntopt(&mnt, "proto=udp")))


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-11-05 18:02:28

by Peter Staubach

[permalink] [raw]
Subject: Re: [PATCH nfs-utils 2/3] Correctly probe mount v2

Steve Dickson wrote:
> Neil Brown wrote:
>
>> diff --git a/utils/mount/network.c b/utils/mount/network.c
>> index 2b09457..38d0135 100644
>> --- a/utils/mount/network.c
>> +++ b/utils/mount/network.c
>> @@ -494,7 +494,7 @@ int probe_bothports(clnt_addr_t *mnt_server, clnt_addr_t *nfs_server)
>> for (; *probe_vers; probe_vers++) {
>> nfs_pmap->pm_vers = mntvers_to_nfs(*probe_vers);
>> if ((res = probe_nfsport(nfs_server) != 0)) {
>> - mnt_pmap->pm_vers = nfsvers_to_mnt(nfs_pmap->pm_vers);
>> + mnt_pmap->pm_vers = *probe_vers;
>> if ((res = probe_mntport(mnt_server)) != 0)
>> return 1;
>> memcpy(mnt_pmap, &save_mnt, sizeof(*mnt_pmap));
>>
>
> This patch does indeed fix mounts in the case where servers only support
> version 2 of the mount protocol. Meaning "mount server:/export /mnt/v2"
> now work when mountd only registers version 2 of the mnt protocol.
>
> But I happen to also notice that when setting the nfsvers
> "mount -o vers=2 server:/export /mnt/v2"
>
> the mount fails, because the mount command assumes version 1 (of the mnt
> protocol) should be used and never tries any other versions. A similar
> assumption is made on the umount side as well.
>
> So I guess the question is, what correlation (if any) is there between
> the mount version and nfs version that is used? Do nfsvers=2 mounts
> have to use version 1 of the mnt protocol? What is the problem if
> v3 mounts use version 2 of the mnt protocol?
>
>

MOUNT Version 2 is the same as MOUNT Version 1 plus the addition
of the Posix style PATHCONF operation. Thus, NFS Version 2 can
use either MOUNT Version 1 or MOUNT Version 2 to get the original
file handle.

MOUNT Version 3 was designed to be used with NFS Version 3. It
should not be used with earlier versions of the NFS protocols nor
should NFS Version 3 depend upon the file handles returned from
MOUNT Version 1 or MOUNT Version 2.

Most NFS server implementations _happen_ to use the same file
handles for NFS Version 2 and NFS Version 3, for ease of
implementation, but this is not a safe assumption to make on
behalf of a client.

Thanx...

ps

> My answers would be None, No and None to those questions, but maybe
> I'm missing something...
>
> Below is an RFC patch that removes these assumption:
>
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index 38d0135..c929986 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -481,8 +481,12 @@ int probe_bothports(clnt_addr_t *mnt_server,
> clnt_addr_t *nfs_server)
>
> if (mnt_pmap->pm_vers && !nfs_pmap->pm_vers)
> nfs_pmap->pm_vers = mntvers_to_nfs(mnt_pmap->pm_vers);
> - else if (nfs_pmap->pm_vers && !mnt_pmap->pm_vers)
> - mnt_pmap->pm_vers = nfsvers_to_mnt(nfs_pmap->pm_vers);
> + else if (nfs_pmap->pm_vers && !mnt_pmap->pm_vers) {
> + /* Does it matter which mount version is used
> + * when the nfs version is set ?
> + */
> + mnt_pmap->pm_vers = 0;
> + }
> if (nfs_pmap->pm_vers)
> goto version_fixed;
>
> diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
> index 285273b..4209514 100644
> --- a/utils/mount/nfsumount.c
> +++ b/utils/mount/nfsumount.c
> @@ -195,12 +195,6 @@ static int do_nfs_umount23(const char *spec, char
> *opts)
> pmap->pm_prog = atoi(p+10);
> if (opts && (p = strstr(opts, "mountport=")) && isdigit(*(p+10)))
> pmap->pm_port = atoi(p+10);
> - if (opts && hasmntopt(&mnt, "v2"))
> - pmap->pm_vers = nfsvers_to_mnt(2);
> - if (opts && hasmntopt(&mnt, "v3"))
> - pmap->pm_vers = nfsvers_to_mnt(3);
> - if (opts && (p = strstr(opts, "vers=")) && isdigit(*(p+5)))
> - pmap->pm_vers = nfsvers_to_mnt(atoi(p+5));
> if (opts && (p = strstr(opts, "mountvers=")) && isdigit(*(p+10)))
> pmap->pm_vers = atoi(p+10);
> if (opts && (hasmntopt(&mnt, "udp") || hasmntopt(&mnt, "proto=udp")))
>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems? Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> _______________________________________________
> NFS maillist - [email protected]
> https://lists.sourceforge.net/lists/listinfo/nfs
>


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-11-01 05:27:12

by NeilBrown

[permalink] [raw]
Subject: [PATCH nfs-utils 2/3] Correctly probe mount v2

When following a list of mount versions to probe -
e.g. probe_mnt1_first or probe_mnt3_first - probe_both will first
probe the appropriate NFS version and then, if that succeeds, probe
the actual mount version. However instead of probing the target mount
version, it probes the "most appropriate" mount version for the given NFS version.
This results in it probing:
NFSv2, MOUNTv1
twice rather than
NFSv2, MOUNTv1
NFSv2, MOUNTv2

as would be more correct.

This patch removes the "choose most correct" step and just use the
current mouint version for the probe_vers array.

Signed-off-by: Neil Brown <[email protected]>
---
utils/mount/network.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 2b09457..38d0135 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -494,7 +494,7 @@ int probe_bothports(clnt_addr_t *mnt_server, clnt_addr_t *nfs_server)
for (; *probe_vers; probe_vers++) {
nfs_pmap->pm_vers = mntvers_to_nfs(*probe_vers);
if ((res = probe_nfsport(nfs_server) != 0)) {
- mnt_pmap->pm_vers = nfsvers_to_mnt(nfs_pmap->pm_vers);
+ mnt_pmap->pm_vers = *probe_vers;
if ((res = probe_mntport(mnt_server)) != 0)
return 1;
memcpy(mnt_pmap, &save_mnt, sizeof(*mnt_pmap));
--
1.5.3.4


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-11-01 05:27:07

by NeilBrown

[permalink] [raw]
Subject: [PATCH nfs-utils 1/3] probe_port should not try other versions in a version was explicitly given.

Currently if probe port is asked to probe for a specific version, and
that version is not supported, it will probe other versions too.
This means that if mountd is run with "--no-nfs-version 3",
It will first probe for NFS version 3, which will succeed (assuming the
kernel supported NFSv3), then it will check the matching mountd version (3)
and probe_port on discovering that isn't supported will try other versions,
find "1" is supported will succeed.

This leaves up using mount version 1 for an NFSv3 mount, which doesn't work
and leads to a SIGSEGV

There is no case where trying other versions is needed the request one is
not supported, so simply remove that code.

Signed-off-by: Neil Brown <[email protected]>
---
utils/mount/network.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 63d5f5a..2b09457 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -417,10 +417,6 @@ static int probe_port(clnt_addr_t *server, const unsigned long *versions,
continue;
p_prot = protos;
}
- if (vers == pmap->pm_vers) {
- p_vers = versions;
- vers = 0;
- }
if (vers || !*++p_vers)
break;
}
--
1.5.3.4


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-11-01 05:27:17

by NeilBrown

[permalink] [raw]
Subject: [PATCH nfs-utils 3/3] Fix bug in auto-exporting subordinate mounts.

When mountd gets a request to export a mountpoint which is not
explicitly exported, but is below an export point that is flagged as
"crossmnt", it passes the wrong path name to the kernel for the
"filehandle -> directory"
mapping.
This can badly confuse the NFS client, and is certainly wrong.

So use the correct path names.

Signed-off-by: Neil Brown <[email protected]>
---
utils/mountd/cache.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index ce1a5a9..fd317cd 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -508,7 +508,7 @@ void nfsd_fh(FILE *f)
*/
qword_printint(f, 0x7fffffff);
if (found)
- qword_print(f, found->e_path);
+ qword_print(f, found_path);
qword_eol(f);
out:
free(found_path);
--
1.5.3.4


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs