From 29298d156e951f03fdbb3f82079a142fb4b6f8d3 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Thu, 13 Aug 2020 16:26:22 +0200 Subject: [PATCH] Revert "exec: move S_ISREG() check earlier" This reverts commit 633fb6ac39801514613fbe050db6abdc3fe744d5 as it breaks execev() semantics. Fixes: 633fb6ac3980 ("exec: move S_ISREG() check earlier") Cc: Kees Cook Reported-by: Marc Zyngier Signed-off-by: Greg Kroah-Hartman Change-Id: I50eb97745b1ee4f8bd5d3f1d1e6c98e30aa7c5ea --- fs/exec.c | 16 ++-------------- fs/namei.c | 6 ++---- fs/open.c | 6 ++++++ 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index a91003e28eaa..33020350595b 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -141,14 +141,8 @@ SYSCALL_DEFINE1(uselib, const char __user *, library) if (IS_ERR(file)) goto out; - /* - * may_open() has already checked for this, so it should be - * impossible to trip now. But we need to be extra cautious - * and check again at the very end too. - */ error = -EACCES; - if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) || - path_noexec(&file->f_path))) + if ((!S_ISREG(file_inode(file)->i_mode) || path_noexec(&file->f_path))) goto exit; fsnotify_open(file); @@ -911,14 +905,8 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) if (IS_ERR(file)) goto out; - /* - * may_open() has already checked for this, so it should be - * impossible to trip now. But we need to be extra cautious - * and check again at the very end too. - */ err = -EACCES; - if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) || - path_noexec(&file->f_path))) + if ((!S_ISREG(file_inode(file)->i_mode) || path_noexec(&file->f_path))) goto exit; err = deny_write_access(file); diff --git a/fs/namei.c b/fs/namei.c index 3afc833f0104..a5c4f5b16a36 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2933,18 +2933,16 @@ static int may_open(const struct path *path, int acc_mode, int flag) case S_IFLNK: return -ELOOP; case S_IFDIR: - if (acc_mode & (MAY_WRITE | MAY_EXEC)) + if (acc_mode & MAY_WRITE) return -EISDIR; break; case S_IFBLK: case S_IFCHR: if (!may_open_dev(path)) return -EACCES; - fallthrough; + /*FALLTHRU*/ case S_IFIFO: case S_IFSOCK: - if (acc_mode & MAY_EXEC) - return -EACCES; flag &= ~O_TRUNC; break; case S_IFREG: diff --git a/fs/open.c b/fs/open.c index 9af548fb841b..c80e9f497e9b 100644 --- a/fs/open.c +++ b/fs/open.c @@ -779,6 +779,12 @@ static int do_dentry_open(struct file *f, return 0; } + /* Any file opened for execve()/uselib() has to be a regular file. */ + if (unlikely(f->f_flags & FMODE_EXEC && !S_ISREG(inode->i_mode))) { + error = -EACCES; + goto cleanup_file; + } + if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) { error = get_write_access(inode); if (unlikely(error))