Skip to contentSkip to conversation

Fix NFS IRIX compatibility braindamage

PATCH Dave Jones 3 participants 3 messages Jan 6, 2003 – Jan 8, 2003 1 message not in the archive NFS server

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Dave Jones

author
#1 lore raw

I'm going through the old 2.4 changelogs looking for bits that have been missed out, the little one liners have been going direct to Linus/maintainer, but here's the first one I'm unsure of..

Any reason this is missing in 2.5 ?

Dave

Quoted 86 lines · Linux Kernel

ChangeSet 1.771, 2002/10/29 09:09:05-02:00, [email protected]

[PATCH] Fix NFS IRIX compatibility braindamage

# This patch includes the following deltas:

#	           ChangeSet	1.770   -> 1.771  
#	   fs/nfsd/nfsproc.c	1.9     -> 1.10   
#	       fs/nfsd/vfs.c	1.13    -> 1.14   

# include/linux/nfsd/nfsd.h 1.4 -> 1.5
#

 fs/nfsd/nfsproc.c         |    4 ++--
 fs/nfsd/vfs.c             |   26 +++++++++++++++-----------
 include/linux/nfsd/nfsd.h |    3 ++-
 3 files changed, 19 insertions(+), 14 deletions(-)

diff -Nru a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c

fs/nfsd/nfsproc.c +2 -2
@@ -264,11 +264,11 @@
 					/* this is probably a permission check..
 					 * at least IRIX implements perm checking on
 					 *   echo thing > device-special-file-or-pipe
-					 * by does a CREATE with type==0
+					 * by doing a CREATE with type==0
 					 */
 					nfserr = nfsd_permission(newfhp->fh_export,
 								 newfhp->fh_dentry,
-								 MAY_WRITE);
+								 MAY_WRITE|_NFSD_IRIX_BOGOSITY);
 					if (nfserr && nfserr != nfserr_rofs)
 						goto out_unlock;
 				}

diff -Nru a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c

fs/nfsd/vfs.c +15 -11
@@ -1493,17 +1493,21 @@
 		inode->i_uid, inode->i_gid, current->fsuid, current->fsgid);
 #endif
 
-	/* only care about readonly exports for files and
-	 * directories. links don't have meaningful write access,
-	 * and all else is local to the client
-	 */
-	if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) 
-		if (acc & (MAY_WRITE | MAY_SATTR | MAY_TRUNC)) {
-			if (EX_RDONLY(exp) || IS_RDONLY(inode))
-				return nfserr_rofs;
-			if (/* (acc & MAY_WRITE) && */ IS_IMMUTABLE(inode))
-				return nfserr_perm;
-		}
+	/* The following code is here to make IRIX happy, which
+	 * does a permission check every time a user does
+	 *	echo yaddayadda > special-file
+	 * by sending a CREATE request.
+	 * The original code would check read-only export status
+	 * only for regular files and directories, allowing
+	 * clients to chown/chmod device files and fifos even
+	 * on volumes exported read-only. */
+	if (!(acc & _NFSD_IRIX_BOGOSITY)
+	 && (acc & (MAY_WRITE | MAY_SATTR | MAY_TRUNC))) {
+		if (EX_RDONLY(exp) || IS_RDONLY(inode))
+			return nfserr_rofs;
+		if (/* (acc & MAY_WRITE) && */ IS_IMMUTABLE(inode))
+			return nfserr_perm;
+	}
 	if ((acc & MAY_TRUNC) && IS_APPEND(inode))
 		return nfserr_perm;
 

diff -Nru a/include/linux/nfsd/nfsd.h b/include/linux/nfsd/nfsd.h

include/linux/nfsd/nfsd.h +2 -2
@@ -37,7 +37,8 @@
 #define MAY_TRUNC		16
 #define MAY_LOCK		32
 #define MAY_OWNER_OVERRIDE	64
-#if (MAY_SATTR | MAY_TRUNC | MAY_LOCK | MAX_OWNER_OVERRIDE) & (MAY_READ | MAY_WRITE | MAY_EXEC | MAY_OWNER_OVERRIDE)
+#define _NFSD_IRIX_BOGOSITY	128
+#if (MAY_SATTR | MAY_TRUNC | MAY_LOCK | MAY_OWNER_OVERRIDE | _NFSD_IRIX_BOGOSITY) & (MAY_READ | MAY_WRITE | MAY_EXEC)
 # error "please use a different value for MAY_SATTR or MAY_TRUNC or MAY_LOCK or MAY_OWNER_OVERRIDE."
 #endif
 #define MAY_CREATE		(MAY_EXEC|MAY_WRITE)
-
Quoted 3 lines

To unsubscribe from this list: send the line "unsubscribe bk-commits-24" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

---end quoted text---

Signature · 2 lines
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs
#2 lore raw
Quoted 6 lines · Dave Jones

I'm going through the old 2.4 changelogs looking for bits that have been missed out, the little one liners have been going direct to Linus/maintainer, but here's the first one I'm unsure of..

Any reason this is missing in 2.5 ?

I think I sent it to the NFS mailing list and forgot about it afterwards.

The problem this patch tries to address is that the current code allows diskless clients to chmod device files, even when the directory is exported read-only.

Olaf

Signature · 3 lines
Olaf Kirch     |  Anyone who has had to work with X.509 has probably
[email protected]   |  experienced what can best be described as
---------------+  ISO water torture. -- Peter Gutmann
#3 lore raw
Quoted 6 lines · On Monday January 6, [email protected] wrote:
Quoted 5 lines · Dave Jones

I'm going through the old 2.4 changelogs looking for bits that have been missed out, the little one liners have been going direct to Linus/maintainer, but here's the first one I'm unsure of..

Quoted 8 lines
Quoted 1 line

Any reason this is missing in 2.5 ?

I think I sent it to the NFS mailing list and forgot about it afterwards.

The problem this patch tries to address is that the current code allows diskless clients to chmod device files, even when the directory is exported read-only.

This has been on my list of things to follow up in 2.5 .... for quite a while. Thanks for the prompting.

I'll commit to getting an appropriate patch to Linus, probably something like the following.

Thanks,
NeilBrown

----------- Diffstat output ------------

 ./fs/nfsd/nfsproc.c         |    4 ++--
 ./fs/nfsd/vfs.c             |    8 ++++----
 ./include/linux/nfsd/nfsd.h |    3 ++-
 3 files changed, 8 insertions(+), 7 deletions(-)
fs/nfsd/nfsproc.c renamed from fs/nfsd/nfsproc.c~current~ +2 -2
diff ./fs/nfsd/nfsproc.c~current~ ./fs/nfsd/nfsproc.c
@@ -250,11 +250,11 @@ nfsd_proc_create(struct svc_rqst *rqstp,
 					/* this is probably a permission check..
 					 * at least IRIX implements perm checking on
 					 *   echo thing > device-special-file-or-pipe
-					 * by does a CREATE with type==0
+					 * by doing a CREATE with type==0
 					 */
 					nfserr = nfsd_permission(newfhp->fh_export,
 								 newfhp->fh_dentry,
-								 MAY_WRITE);
+								 MAY_WRITE|MAY_LOCAL_ACCESS);
 					if (nfserr && nfserr != nfserr_rofs)
 						goto out_unlock;
 				}
fs/nfsd/vfs.c renamed from fs/nfsd/vfs.c~current~ +4 -4
diff ./fs/nfsd/vfs.c~current~ ./fs/nfsd/vfs.c
@@ -1515,11 +1515,11 @@ nfsd_permission(struct svc_export *exp, 
 		inode->i_uid, inode->i_gid, current->fsuid, current->fsgid);
 #endif
 
-	/* only care about readonly exports for files and
-	 * directories. links don't have meaningful write access,
-	 * and all else is local to the client
+	/* Normally we reject any write/sattr etc access on a read-only file
+	 * system.  But if it is IRIX doing check on write-access for a 
+	 * device special file, we ignore rofs.
 	 */
-	if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) 
+	if (!(acc & MAY_LOCAL_ACCESS))
 		if (acc & (MAY_WRITE | MAY_SATTR | MAY_TRUNC)) {
 			if (EX_RDONLY(exp) || IS_RDONLY(inode))
 				return nfserr_rofs;
include/linux/nfsd/nfsd.h renamed from include/linux/nfsd/nfsd.h~current~ +2 -1
diff ./include/linux/nfsd/nfsd.h~current~ ./include/linux/nfsd/nfsd.h
@@ -38,7 +38,8 @@
 #define MAY_TRUNC		16
 #define MAY_LOCK		32
 #define MAY_OWNER_OVERRIDE	64
-#if (MAY_SATTR | MAY_TRUNC | MAY_LOCK | MAX_OWNER_OVERRIDE) & (MAY_READ | MAY_WRITE | MAY_EXEC | MAY_OWNER_OVERRIDE)
+#define	MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
+#if (MAY_SATTR | MAY_TRUNC | MAY_LOCK | MAX_OWNER_OVERRIDE | MAY_LOCAL_ACCESS) & (MAY_READ | MAY_WRITE | MAY_EXEC | MAY_OWNER_OVERRIDE)
 # error "please use a different value for MAY_SATTR or MAY_TRUNC or MAY_LOCK or MAY_OWNER_OVERRIDE."
 #endif
 #define MAY_CREATE		(MAY_EXEC|MAY_WRITE)