Re: [PATCH] Fix NFS IRIX compatibility braindamage

[Neil Brown]

On Monday January 6, [e-mail] wrote:
> On Mon, Jan 06, 2003 at 07:33:20PM +0000, Dave Jones wrote:
> > 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.

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(-)

diff ./fs/nfsd/nfsproc.c~current~ ./fs/nfsd/nfsproc.c
--- ./fs/nfsd/nfsproc.c~current~	2003-01-08 11:20:22.000000000 +1100
+++ ./fs/nfsd/nfsproc.c	2003-01-08 11:20:41.000000000 +1100
@@ -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;
 				}

diff ./fs/nfsd/vfs.c~current~ ./fs/nfsd/vfs.c
--- ./fs/nfsd/vfs.c~current~	2003-01-08 11:20:51.000000000 +1100
+++ ./fs/nfsd/vfs.c	2003-01-08 11:23:00.000000000 +1100
@@ -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;

diff ./include/linux/nfsd/nfsd.h~current~ ./include/linux/nfsd/nfsd.h
--- ./include/linux/nfsd/nfsd.h~current~	2003-01-08 11:18:25.000000000 +1100
+++ ./include/linux/nfsd/nfsd.h	2003-01-08 11:19:51.000000000 +1100
@@ -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)