linux-kernel - [PATCH -next] exec: Remove redundant check in do_open

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 18 May 2022 16:12:27 +0800
From:   Zhihao Cheng <chengzhihao1@...wei.com>
To:     <keescook@...omium.org>, <viro@...iv.linux.org.uk>,
        <akpm@...ux-foundation.org>
CC:     <linux-mm@...ck.org>, <linux-fsdevel@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <chengzhihao1@...wei.com>,
        <yukuai3@...wei.com>
Subject: [PATCH -next] exec: Remove redundant check in do_open_execat/uselib

There is a false positive WARNON happening in execve(2)/uselib(2)
syscalls with concurrent noexec-remount.

       execveat                           remount
do_open_execat(path/bin)
  do_filp_open
    path_openat
      do_open
        may_open
          path_noexec() // PASS
	                            remount(path->mnt, MS_NOEXEC)
WARNON(path_noexec(&file->f_path)) // path_noexec() checks fail

Since may_open() has already checked the same conditions, fix it by
removing 'S_ISREG' and 'path_noexec' check in do_open_execat()/uselib(2).

Fixes: 0fd338b2d2cdf8 ("exec: move path_noexec() check earlier")
Signed-off-by: Zhihao Cheng <chengzhihao1@...wei.com>
---
 fs/exec.c | 22 +---------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index e3e55d5e0be1..0f8ea7e9e03c 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -141,16 +141,6 @@ 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)))
-		goto exit;
-
 	fsnotify_open(file);
 
 	error = -ENOEXEC;
@@ -169,7 +159,7 @@ SYSCALL_DEFINE1(uselib, const char __user *, library)
 			break;
 	}
 	read_unlock(&binfmt_lock);
-exit:
+
 	fput(file);
 out:
   	return error;
@@ -919,16 +909,6 @@ 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)))
-		goto exit;
-
 	err = deny_write_access(file);
 	if (err)
 		goto exit;
-- 
2.31.1