Bug 131 - [Patch] Incomplete ACL checking in case of atomic OPEN
: [Patch] Incomplete ACL checking in case of atomic OPEN
Status: CLOSED FIXED
Product: kernel
client
: unspecified
: PC Linux
: P3 normal
Assigned To: Trond Myklebust
:
:
:
:
  Show dependency treegraph
 
Reported: 2006-11-17 06:12 UTC by Simon Vallet
Modified: 2014-01-29 15:06 UTC (History)
0 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Vallet 2006-11-17 06:12:23 UTC
When an NFS4 server advertizes atomic OPEN capability, it appears that ACL
execute permissions on a regular file are not checked.

This is due to nfs_permission() returning without issuing an ACCESS call when
the server is marked as being atomic-open capable. While this is no problem for
read and write access, enforcing execute permissions requires an ACCESS call in
this situation.

The minimal patch below tries to fix the problem by forcing an access lookup in
case of both LOOKUP_OPEN and MAY_EXEC. It does break atomicity of OPEN for
executables, but I fail to see other ways to properly check for execute
permissions. This patch applies against the latest CITI kernel, which is
2.6.19-rc3

Signed-off-by: Simon Vallet <svallet@genoscope.cns.fr>

--- fs/nfs/dir.c.orig   2006-11-17 15:01:27.000000000 +0100
+++ fs/nfs/dir.c        2006-11-17 15:08:55.000000000 +0100
@@ -1941,11 +1941,15 @@ int nfs_permission(struct inode *inode, 
                case S_IFLNK:
                        goto out;
                case S_IFREG:
+                       if (nd != NULL && (nd->flags & LOOKUP_OPEN)) {
+                               /* Enforce access call in case we want exec
permissions */
+                               if (mask & MAY_EXEC)
+                                       goto force_lookup;
+
                        /* NFSv4 has atomic_open... */
                        if (nfs_server_capable(inode, NFS_CAP_ATOMIC_OPEN)
-                                       && nd != NULL
-                                       && (nd->flags & LOOKUP_OPEN))
                                goto out;
+                       }
                        break;
                case S_IFDIR:
                        /*
Comment 1 Simon Vallet 2006-11-17 07:10:05 UTC
Hmmm... I think I needed some more coffee. The above patch of course doesn't
compile. The patch below replaces it -- sorry for the noise.

Signed-off-by: Simon Vallet <svallet@genoscope.cns.fr>

--- fs/nfs/dir.c.orig   2006-11-17 15:42:42.000000000 +0100
+++ fs/nfs/dir.c        2006-11-17 15:46:22.000000000 +0100
@@ -1941,11 +1941,16 @@ int nfs_permission(struct inode *inode, 
                case S_IFLNK:
                        goto out;
                case S_IFREG:
-                       /* NFSv4 has atomic_open... */
-                       if (nfs_server_capable(inode, NFS_CAP_ATOMIC_OPEN)
-                                       && nd != NULL
-                                       && (nd->flags & LOOKUP_OPEN))
-                               goto out;
+                       if (nd != NULL && (nd->flags & LOOKUP_OPEN)) {
+
+                               /* Force ACCESS call if we need to check exec
perms */
+                               if ((mask & MAY_EXEC))
+                                       goto force_lookup;
+
+                               /* NFSv4 has atomic_open... */
+                               if (nfs_server_capable(inode,
NFS_CAP_ATOMIC_OPEN))
+                                       goto out;
+                       }
                        break;
                case S_IFDIR:
                        /*
Comment 2 Trond Myklebust 2014-01-29 15:05:46 UTC
Fixed in upstream kernel.