[PATCH] move executable checking into ->permission()
authorMiklos Szeredi <miklos@szeredi.hu>
Thu, 31 Jul 2008 11:41:58 +0000 (13:41 +0200)
committerAl Viro <viro@zeniv.linux.org.uk>
Thu, 23 Oct 2008 09:13:25 +0000 (05:13 -0400)
For execute permission on a regular files we need to check if file has
any execute bits at all, regardless of capabilites.

This check is normally performed by generic_permission() but was also
added to the case when the filesystem defines its own ->permission()
method.  In the latter case the filesystem should be responsible for
performing this check.

Move the check from inode_permission() inside filesystems which are
not calling generic_permission().

Create a helper function execute_ok() that returns true if the inode
is a directory or if any execute bits are present in i_mode.

Also fix up the following code:

 - coda control file is never executable
 - sysctl files are never executable
 - hfs_permission seems broken on MAY_EXEC, remove
 - hfsplus_permission is eqivalent to generic_permission(), remove

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
fs/cifs/cifsfs.c
fs/coda/dir.c
fs/coda/pioctl.c
fs/hfs/inode.c
fs/hfsplus/inode.c
fs/namei.c
fs/nfs/dir.c
fs/proc/proc_sysctl.c
include/linux/fs.h

index 89c64a8dcb99a3568a58f9b6d2ed8266a0f8dec4..84cc011a16e4d1afc90fe29d88ba2d1350902944 100644 (file)
@@ -275,9 +275,12 @@ static int cifs_permission(struct inode *inode, int mask)
 
        cifs_sb = CIFS_SB(inode->i_sb);
 
-       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_PERM)
-               return 0;
-       else /* file mode might have been restricted at mount time
+       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_PERM) {
+               if ((mask & MAY_EXEC) && !execute_ok(inode))
+                       return -EACCES;
+               else
+                       return 0;
+       } else /* file mode might have been restricted at mount time
                on the client (above and beyond ACL on servers) for
                servers which do not support setting and viewing mode bits,
                so allowing client to check permissions is useful */
index c5916228243c1516b2d62e6b06cea208523a09ac..75b1fa90b2cb8ee205efc96bbbb8a7c71fc9a1e1 100644 (file)
@@ -146,6 +146,9 @@ int coda_permission(struct inode *inode, int mask)
        if (!mask)
                return 0; 
 
+       if ((mask & MAY_EXEC) && !execute_ok(inode))
+               return -EACCES;
+
        lock_kernel();
 
        if (coda_cache_check(inode, mask))
index c51365422aa8da4311133c3b0ff773421efbc723..773f2ce9aa068e48a865c677cf07c717d408777b 100644 (file)
@@ -43,7 +43,7 @@ const struct file_operations coda_ioctl_operations = {
 /* the coda pioctl inode ops */
 static int coda_ioctl_permission(struct inode *inode, int mask)
 {
-        return 0;
+       return (mask & MAY_EXEC) ? -EACCES : 0;
 }
 
 static int coda_pioctl(struct inode * inode, struct file * filp, 
index 7e19835efa2ea91675980a18e963281a02d46bb8..c69b7ac75bf7d16bb53f6f1603e9a47019a2071b 100644 (file)
@@ -511,13 +511,6 @@ void hfs_clear_inode(struct inode *inode)
        }
 }
 
-static int hfs_permission(struct inode *inode, int mask)
-{
-       if (S_ISREG(inode->i_mode) && mask & MAY_EXEC)
-               return 0;
-       return generic_permission(inode, mask, NULL);
-}
-
 static int hfs_file_open(struct inode *inode, struct file *file)
 {
        if (HFS_IS_RSRC(inode))
@@ -616,7 +609,6 @@ static const struct inode_operations hfs_file_inode_operations = {
        .lookup         = hfs_file_lookup,
        .truncate       = hfs_file_truncate,
        .setattr        = hfs_inode_setattr,
-       .permission     = hfs_permission,
        .setxattr       = hfs_setxattr,
        .getxattr       = hfs_getxattr,
        .listxattr      = hfs_listxattr,
index 963be644297aeb45c2d376721c5cc70ee8ce28f5..b207f0e6fc22cec1feb87b127016f622b169656d 100644 (file)
@@ -238,18 +238,6 @@ static void hfsplus_set_perms(struct inode *inode, struct hfsplus_perm *perms)
        perms->dev = cpu_to_be32(HFSPLUS_I(inode).dev);
 }
 
-static int hfsplus_permission(struct inode *inode, int mask)
-{
-       /* MAY_EXEC is also used for lookup, if no x bit is set allow lookup,
-        * open_exec has the same test, so it's still not executable, if a x bit
-        * is set fall back to standard permission check.
-        */
-       if (S_ISREG(inode->i_mode) && mask & MAY_EXEC && !(inode->i_mode & 0111))
-               return 0;
-       return generic_permission(inode, mask, NULL);
-}
-
-
 static int hfsplus_file_open(struct inode *inode, struct file *file)
 {
        if (HFSPLUS_IS_RSRC(inode))
@@ -281,7 +269,6 @@ static int hfsplus_file_release(struct inode *inode, struct file *file)
 static const struct inode_operations hfsplus_file_inode_operations = {
        .lookup         = hfsplus_file_lookup,
        .truncate       = hfsplus_file_truncate,
-       .permission     = hfsplus_permission,
        .setxattr       = hfsplus_setxattr,
        .getxattr       = hfsplus_getxattr,
        .listxattr      = hfsplus_listxattr,
index 9e2a534383d9f3c26780e4ae937d3b52df16cfc9..09ce58e49e72bb000004851a1ea23e958b0b917d 100644 (file)
@@ -212,8 +212,7 @@ int generic_permission(struct inode *inode, int mask,
         * Read/write DACs are always overridable.
         * Executable DACs are overridable if at least one exec bit is set.
         */
-       if (!(mask & MAY_EXEC) ||
-           (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode))
+       if (!(mask & MAY_EXEC) || execute_ok(inode))
                if (capable(CAP_DAC_OVERRIDE))
                        return 0;
 
@@ -249,23 +248,11 @@ int inode_permission(struct inode *inode, int mask)
        }
 
        /* Ordinary permission routines do not understand MAY_APPEND. */
-       if (inode->i_op && inode->i_op->permission) {
+       if (inode->i_op && inode->i_op->permission)
                retval = inode->i_op->permission(inode, mask);
-               if (!retval) {
-                       /*
-                        * Exec permission on a regular file is denied if none
-                        * of the execute bits are set.
-                        *
-                        * This check should be done by the ->permission()
-                        * method.
-                        */
-                       if ((mask & MAY_EXEC) && S_ISREG(inode->i_mode) &&
-                           !(inode->i_mode & S_IXUGO))
-                               return -EACCES;
-               }
-       } else {
+       else
                retval = generic_permission(inode, mask, NULL);
-       }
+
        if (retval)
                return retval;
 
index c216c8786c511049f45738fdf5c3191118a26a8b..3e64b98f3a9337ab241624fc5a544425e7f05e45 100644 (file)
@@ -1957,6 +1957,9 @@ force_lookup:
        } else
                res = PTR_ERR(cred);
 out:
+       if (!res && (mask & MAY_EXEC) && !execute_ok(inode))
+               res = -EACCES;
+
        dfprintk(VFS, "NFS: permission(%s/%ld), mask=0x%x, res=%d\n",
                inode->i_sb->s_id, inode->i_ino, mask, res);
        return res;
index 5fe210c091719d7d1cda43f8eac01617dd4aee8e..7b997754a25e705ddb41009b0741ddad3c3799a2 100644 (file)
@@ -298,13 +298,19 @@ static int proc_sys_permission(struct inode *inode, int mask)
         * sysctl entries that are not writeable,
         * are _NOT_ writeable, capabilities or not.
         */
-       struct ctl_table_header *head = grab_header(inode);
-       struct ctl_table *table = PROC_I(inode)->sysctl_entry;
+       struct ctl_table_header *head;
+       struct ctl_table *table;
        int error;
 
+       /* Executable files are not allowed under /proc/sys/ */
+       if ((mask & MAY_EXEC) && S_ISREG(inode->i_mode))
+               return -EACCES;
+
+       head = grab_header(inode);
        if (IS_ERR(head))
                return PTR_ERR(head);
 
+       table = PROC_I(inode)->sysctl_entry;
        if (!table) /* global root - r-xr-xr-x */
                error = mask & MAY_WRITE ? -EACCES : 0;
        else /* Use the permissions on the sysctl table entry */
index 5f70aa62cf0fda420db26919164cc67210740aa1..025a4a251b644a2e6d5ab51de382aed15953f787 100644 (file)
@@ -1851,6 +1851,11 @@ extern int inode_permission(struct inode *, int);
 extern int generic_permission(struct inode *, int,
                int (*check_acl)(struct inode *, int));
 
+static inline bool execute_ok(struct inode *inode)
+{
+       return (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode);
+}
+
 extern int get_write_access(struct inode *);
 extern int deny_write_access(struct file *);
 static inline void put_write_access(struct inode * inode)