• Kees Cook's avatar
    exec: move S_ISREG() check earlier · 633fb6ac
    Kees Cook authored
    The execve(2)/uselib(2) syscalls have always rejected non-regular files.
    Recently, it was noticed that a deadlock was introduced when trying to
    execute pipes, as the S_ISREG() test was happening too late.  This was
    fixed in commit 73601ea5 ("fs/open.c: allow opening only regular files
    during execve()"), but it was added after inode_permission() had already
    run, which meant LSMs could see bogus attempts to execute non-regular
    files.
    
    Move the test into the other inode type checks (which already look for
    other pathological conditions[1]).  Since there is no need to use
    FMODE_EXEC while we still have access to "acc_mode", also switch the test
    to MAY_EXEC.
    
    Also include a comment with the redundant S_ISREG() checks at the end of
    execve(2)/uselib(2) to note that they are present to avoid any mistakes.
    
    My notes on the call path, and related arguments, checks, etc:
    
    do_open_execat()
        struct open_flags open_exec_flags = {
            .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
            .acc_mode = MAY_EXEC,
            ...
        do_filp_open(dfd, filename, open_flags)
            path_openat(nameidata, open_flags, flags)
                file = alloc_empty_file(open_flags, current_cred());
                do_open(nameidata, file, open_flags)
                    may_open(path, acc_mode, open_flag)
    		    /* new location of MAY_EXEC vs S_ISREG() test */
                        inode_permission(inode, MAY_OPEN | acc_mode)
                            security_inode_permission(inode, acc_mode)
                    vfs_open(path, file)
                        do_dentry_open(file, path->dentry->d_inode, open)
                            /* old location of FMODE_EXEC vs S_ISREG() test */
                            security_file_open(f)
                            open()
    
    [1] https://lore.kernel.org/lkml/202006041910.9EF0C602@keescook/
    
    
    
    Signed-off-by: default avatarKees Cook <keescook@chromium.org>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Cc: Aleksa Sarai <cyphar@cyphar.com>
    Cc: Alexander Viro <viro@zeniv.linux.org.uk>
    Cc: Christian Brauner <christian.brauner@ubuntu.com>
    Cc: Dmitry Vyukov <dvyukov@google.com>
    Cc: Eric Biggers <ebiggers3@gmail.com>
    Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Link: http://lkml.kernel.org/r/20200605160013.3954297-3-keescook@chromium.org
    
    
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    633fb6ac