Skip to contentSkip to conversation

SGI 900985: fix nohide export option for nfsv3

PATCH In review Greg Banks 2 participants 5 messages Oct 3, 2003 – Oct 16, 2003

Subsystems (3)
Kbuild NFS client NFS server
lore
Share & tools
lore

Greg Banks

author
#0 lore raw

PATCH 1/1 SGI 900985: fix nohide export option for nfsv3

G'day,

SGI bug 900985.

The "nohide" export option fails over NFSv3 because the client gets as a side effect of READDIRPLUS file handles for server-side mountpoints which haven't gone through the nohide logic in LOOKUP and thus refer to the underlying directory instead the covering one.

This patch fixes that by changing the XDR encode for the READDIRPLUS reply to send no file handle if the directory's child is a mountpoint. The protocol says the filehandle is optional in the READDIRPLUS reply. If the client has a client-side mountpoint matching the server-side mountpoint, it will get the file handle of the mountpoint's covering directory by traversing the client-side mountpoint. Otherwise, the client will issue a LOOKUP which will return either the underlying or covering directories as per the normal nohide logic.

This is on-the-wire identical to how IRIX deals with the issue, as can be easily demonstrated by sniffing NFS traffic to an IRIX box with a nohide export.

Tested on Linux 2.4.21 servers with Linux 2.4 and IRIX clients. The patch is against 2.4 but the same bug exists in 2.6 and the same patch should fix it (untested).

--- /usr/tmp/TmpDir.24872-0/linux/linux/fs/nfsd/nfs3xdr.c_1.23 Fri Sep 19 20:12:50
2003
+++ linux/linux/fs/nfsd/nfs3xdr.c Fri Sep 19 20:09:25 2003
@@ -721,6 +721,8 @@
dchild = lookup_one_len(name, dparent,namlen);
if (IS_ERR(dchild))
goto noexec;
+ if (d_mountpoint(dchild))
+ goto noexec;
if (fh_compose(&fh, exp, dchild, cd->dirfh) != 0 || !dchild->d_inode)
goto noexec;
p = encode_post_op_attr(cd->rqstp, p, &fh);

linux/fs/nfsd/vfs.c renamed from usr/tmp/TmpDir.24872-0/linux/linux/fs/nfsd/vfs.c_1.47 +1 -1
@@ -86,7 +86,7 @@
  * accepted as it stands and the mounted directory is
  * returned. Otherwise the covered directory is returned.
  * NOTE: this mountpoint crossing is not supported properly by all
- *   clients and is explicitly disallowed for NFSv3
+ *   clients.
  *      NeilBrown <[email protected]>
  */
 int

Greg.

Signature · 11 lines
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
#1 lore raw
Quoted 8 lines · On Friday October 3, [email protected] wrote:

G'day,

SGI bug 900985.

The "nohide" export option fails over NFSv3 because the client gets as a side effect of READDIRPLUS file handles for server-side mountpoints which haven't gone through the nohide logic in LOOKUP and thus refer to the underlying directory instead the covering one.

...

Quoted 7 lines

--- /usr/tmp/TmpDir.24872-0/linux/linux/fs/nfsd/nfs3xdr.c_1.23 Fri Sep 19 20:12:50
2003
+++ linux/linux/fs/nfsd/nfs3xdr.c Fri Sep 19 20:09:25 2003

Review line 728

Quoted 8 lines
(quoted diff) +2 -0
@@ -721,6 +721,8 @@
 			dchild = lookup_one_len(name, dparent,namlen);
 		if (IS_ERR(dchild))
 			goto noexec;
+		if (d_mountpoint(dchild))
+			goto noexec;
 		if (fh_compose(&fh, exp, dchild, cd->dirfh) != 0 || !dchild->d_inode)
 			goto noexec;
 		p = encode_post_op_attr(cd->rqstp, p, &fh);

This looks OK. Thanks.

linux/fs/nfsd/vfs.c line 90

Quoted 9 lines
linux/fs/nfsd/vfs.c renamed from usr/tmp/TmpDir.24872-0/linux/linux/fs/nfsd/vfs.c_1.47 +1 -1
@@ -86,7 +86,7 @@
  * accepted as it stands and the mounted directory is
  * returned. Otherwise the covered directory is returned.
  * NOTE: this mountpoint crossing is not supported properly by all
- *   clients and is explicitly disallowed for NFSv3
+ *   clients.
  *      NeilBrown <[email protected]>

This isn't quite right. What the comment *Should* say is:
... and RFC1813 explicitly dis-allows it for NFSv3 (See section 3.3.3).

NeilBrown

-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program. SourceForge.net hosts over 70,000 Open Source Projects. See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

author
#2 lore raw
Quoted 4 lines · Neil Brown
Quoted 2 lines · On Friday October 3, [email protected] wrote:

G'day,

Quoted 12 lines

linux/fs/nfsd/vfs.c Fri Sep 19 20:08:03 2003 line 90

Quoted 9 lines
linux/fs/nfsd/vfs.c Fri Sep 19 20:08:03 2003 renamed from usr/tmp/TmpDir.24872-0/linux/linux/fs/nfsd/vfs.c_1.47 Fri Sep 19 20:12:50 2003 +1 -1
@@ -86,7 +86,7 @@
  * accepted as it stands and the mounted directory is
  * returned. Otherwise the covered directory is returned.
  * NOTE: this mountpoint crossing is not supported properly by all
- *   clients and is explicitly disallowed for NFSv3
+ *   clients.
  *      NeilBrown <[email protected]>

This isn't quite right. What the comment *Should* say is:
... and RFC1813 explicitly dis-allows it for NFSv3 (See section 3.3.3).

RFC1813 section 3.3.3 says...

Quoted 7 lines

A server will not allow a LOOKUP operation to cross a mountpoint to the root of a different filesystem, even if the filesystem is exported.
[...]
A given server implementation may refine these rules given capabilities or limitations particular to that implementation. Refer to [X/OpenNFS] for a discussion on exporting file systems.

Reads to me like there's a loophole designed specifically for the nohide option.

The thing is, we have customers moving from Irix systems with an "sgi" logo on the front to Linux systems with an "sgi" logo on the front; they see the "nohide" option like they're used to and (poor naive creatures) expect it to work. Having it work on v2 and silently fail on v3 is really submarvellous.

Greg.

Signature · 12 lines
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.


-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
#3 lore raw
Quoted 28 lines · On Monday October 13, [email protected] wrote:
Quoted 4 lines · Neil Brown
Quoted 2 lines · On Friday October 3, [email protected] wrote:

G'day,

Quoted 12 lines

linux/fs/nfsd/vfs.c Fri Sep 19 20:08:03 2003 line 90

Quoted 9 lines
linux/fs/nfsd/vfs.c Fri Sep 19 20:08:03 2003 renamed from usr/tmp/TmpDir.24872-0/linux/linux/fs/nfsd/vfs.c_1.47 Fri Sep 19 20:12:50 2003 +1 -1
@@ -86,7 +86,7 @@
  * accepted as it stands and the mounted directory is
  * returned. Otherwise the covered directory is returned.
  * NOTE: this mountpoint crossing is not supported properly by all
- *   clients and is explicitly disallowed for NFSv3
+ *   clients.
  *      NeilBrown <[email protected]>

This isn't quite right. What the comment *Should* say is:
... and RFC1813 explicitly dis-allows it for NFSv3 (See section 3.3.3).

RFC1813 section 3.3.3 says...

Quoted 7 lines

A server will not allow a LOOKUP operation to cross a mountpoint to the root of a different filesystem, even if the filesystem is exported.
[...]
A given server implementation may refine these rules given capabilities or limitations particular to that implementation. Refer to [X/OpenNFS] for a discussion on exporting file systems.

Quoted 2 lines

Reads to me like there's a loophole designed specifically for the nohide option.

So it does. I guess I never read that far, and mistakenly read "A server will not allow..." as "A server SHALL not allow..." :-)

Quoted 6 lines

The thing is, we have customers moving from Irix systems with an "sgi" logo on the front to Linux systems with an "sgi" logo on the front; they see the "nohide" option like they're used to and (poor naive creatures) expect it to work. Having it work on v2 and silently fail on v3 is really submarvellous.

Certainly there is no suggestion of not supporting it for V3. The comment was just meant to say that we weren't allowed to support it, but that we didn't.

NeilBrown

-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program. SourceForge.net hosts over 70,000 Open Source Projects. See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

author
#4 lore raw
Quoted 6 lines · Neil Brown
Quoted 2 lines · On Monday October 13, [email protected] wrote:

The thing is, [...] Having it work on v2 and silently fail on v3 is really submarvellous.

Certainly there is no suggestion of not supporting it for V3.

Sorry, I missed your "Ok, thanks" for the first part of the diff.

Greg.

Signature · 12 lines
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.


-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs