[Unionfs] File modification problems with unionfs and NFS

[Erez Zadok]

In message <000301ca1021$df96d610$9ec48230$@de>, =?iso-8859-1?Q?Thomas_Schau=DF?= writes:
> Hello Erez,
> 
> Thank you very much for looking into this issue so quickly.
> 
> Applying the patch does solve the error I posted.
> 
> However, the patch also results in a privilege escalation. Any user can
> modify any file for which he has read-permissions (only once, after the
> copy-up was performed the permissions then again work as expected). So I
> guess an additional check on the permissions of the lower branch inode are
> necessary?
> 
> Best Regards,
> Thomas

Here's an improved patch which appears to solve the privilege escalation
problem.  I've tested it on a number of files with various permissions and
uid/gid combinations, as both user root and a non-uid0 user.  Basically, in
the case where the lower branch is nfs2/3, and the branch is marked
readonly, and we suspect that NFS returned a bogus "EACCES" error, then we
we have to ignore nfs's own ->permission method and rely on
generic_permission().

Let me know how this works for you.

Thanks,
Erez.

##############################################################################


Unionfs: fix readonly nfs2/3 permission handling

In unionfs_permission: NFSv2/3 return EACCES on readonly-exported, locally
readonly-mounted file systems, instead of EROFS like other file systems do.
So we have no choice here but to intercept this and ignore it for NFS
branches marked readonly.  Specifically, we avoid using NFS's own "broken"
->permission method, and rely on generic_permission() to do basic checking
for us.

Signed-off-by: Erez Zadok <ezk at cs.sunysb.edu>
diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 7c17093..7e86273 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -843,6 +843,20 @@ static int unionfs_permission(struct inode *inode, int mask)
 		}
 
 		/*
+		 * NFS HACK: NFSv2/3 return EACCES on readonly-exported,
+		 * locally readonly-mounted file systems, instead of EROFS
+		 * like other file systems do.  So we have no choice here
+		 * but to intercept this and ignore it for NFS branches
+		 * marked readonly.  Specifically, we avoid using NFS's own
+		 * "broken" ->permission method, and rely on
+		 * generic_permission() to do basic checking for us.
+		 */
+		if (err && err == -EACCES &&
+		    is_robranch_super(inode->i_sb, bindex) &&
+		    lower_inode->i_sb->s_magic == NFS_SUPER_MAGIC)
+			err = generic_permission(lower_inode, mask, NULL);
+
+		/*
 		 * The permissions are an intersection of the overall directory
 		 * permissions, so we fail if one fails.
 		 */