Re: [Linux-privs-discuss] [PATCH] fix permission checks for executables

From: Christoph Hellwig (hch@caldera.de)
Date: Wed Aug 08 2001 - 12:32:28 EST


On Wed, Aug 08, 2001 at 06:22:19PM +0200, Christoph Hellwig wrote:
> Hi Linux,
>
> vfs_permission in the Linux 2.4 series tries to check for
> CAP_DAC_OVERRIDE if the modes didn't match. This means even
> for an file without executable bits set at all, root will
> be reported that it is. I've actually found one apllication
> (scomail under linux-abi) that fails because of this, besides
> not matching my reading of Posix 1003.1e.
>
> Of the operating systems with capabilty-like features at least
> OpenUNIX gets it right, of the others at least OpenServer and
> 4.4BSD, but these semantics seem natural to me anyway..

Andreas Gruenbacher pointed out that it is much leaner to use
MAY_READ¸MAY_EXEC and MAY_WRITE instead of abusing the stat-macros.

Here is a patch that changes the complete function to use these
and also cleans up and clarifies the comments.

        Christoph

--- linux.really_plain/fs/namei.c Wed Aug 8 17:56:58 2001
+++ linux.plain/fs/namei.c Wed Aug 8 19:27:19 2001
@@ -140,7 +140,7 @@
 }
 
 /*
- * permission()
+ * vfs_permission()
  *
  * is used to check for read/write/execute permissions on a file.
  * We use "fsuid" for this, letting us set arbitrary permissions
@@ -151,24 +151,40 @@
 {
         int mode = inode->i_mode;
 
- if ((mask & S_IWOTH) && IS_RDONLY(inode) &&
- (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
- return -EROFS; /* Nobody gets write access to a read-only fs */
-
- if ((mask & S_IWOTH) && IS_IMMUTABLE(inode))
- return -EACCES; /* Nobody gets write access to an immutable file */
+ if (mask & MAY_WRITE) {
+ /*
+ * Nobody gets write access to a read-only fs.
+ */
+ if (IS_RDONLY(inode) &&
+ (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
+ return -EROFS;
+
+ /*
+ * Nobody gets write access to an immutable file.
+ */
+ if (IS_IMMUTABLE(inode))
+ return -EACCES;
+ }
 
         if (current->fsuid == inode->i_uid)
                 mode >>= 6;
         else if (in_group_p(inode->i_gid))
                 mode >>= 3;
 
- if (((mode & mask & S_IRWXO) == mask) || capable(CAP_DAC_OVERRIDE))
+ if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask))
                 return 0;
 
- /* read and search access */
- if ((mask == S_IROTH) ||
- (S_ISDIR(inode->i_mode) && !(mask & ~(S_IROTH | S_IXOTH))))
+ /*
+ * Only read/write DACs are overridable.
+ */
+ if ((mask & (MAY_READ|MAY_WRITE)) || S_ISDIR(inode->i_mode))
+ if (capable(CAP_DAC_OVERRIDE))
+ return 0;
+
+ /*
+ * Searching includes executable on directories, else just read.
+ */
+ if (mask == MAY_READ || (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE)))
                 if (capable(CAP_DAC_READ_SEARCH))
                         return 0;
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Wed Aug 15 2001 - 21:00:18 EST