From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 36274C369D2 for ; Wed, 25 Sep 2024 11:37:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BDB8E6B0096; Wed, 25 Sep 2024 07:37:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B8B2E6B0098; Wed, 25 Sep 2024 07:37:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A53506B0099; Wed, 25 Sep 2024 07:37:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 885AD6B0096 for ; Wed, 25 Sep 2024 07:37:32 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 4CCB9A0236 for ; Wed, 25 Sep 2024 11:37:32 +0000 (UTC) X-FDA: 82603060344.17.5AB0C17 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf30.hostedemail.com (Postfix) with ESMTP id 96D5F80015 for ; Wed, 25 Sep 2024 11:37:29 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=AiXzXxwf; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf30.hostedemail.com: domain of sashal@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=sashal@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727264152; a=rsa-sha256; cv=none; b=4oNt/bo9ZG85QDiPJX08dCnWbUH3E8WhOYYOooNHcCF3mMWoVGh9EhcNnBqNphdvMAVOzK J/v7OYdyJRTQIl+fB/OzIkEWR/01Cf4OLOt3E2ts3hSPpX9rtx94EHLJGFEIBlYuOq1lMj S4RYfn68Gbhikj5Ewhl488jkdPdXSd8= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=AiXzXxwf; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf30.hostedemail.com: domain of sashal@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=sashal@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727264152; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=+xqUkX2Sr3aIAtDMwMM2ClaunG/ZLpvyyspZWWRI5n4=; b=LJQMydC51mxmRbirqAGWCykZOmDXBFNjzDfeXCW7qnSe1XkC2giGINJ+n/Mij1vWFXbj+q auxHdZmA+8k0WQsDjgAIHQFgUzE4zxPorbB+c5lWpyeu7dmHLxb+rFfv9WjSokHbUMKwjK B4Inb61tvi20t6wUl8xzcqm51ujZG2k= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id C36D8A43F25; Wed, 25 Sep 2024 11:37:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 84309C4CEC7; Wed, 25 Sep 2024 11:37:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1727264248; bh=oxGGPOE4uL7X7MkTJ9BgnFV6wDR+RX7IZb/it7homu0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=AiXzXxwfCszQteN0H2JfpBF1HmMWp0eyH70Tp40fzJo7kzIECnaJsmcv577jq/AKs SWhJR/GafkU95byIdjU/iGTngsmxg5YRAxhgafsigwNZXN2UWJhe/uOzP2jqVVedUJ KxvtN8vwSYBWKeEFRsgjqcIps5iX9XKDOfl7ojybZm6GC/weBB0GhGLU4XoOg/tUpu oLo3G2qLnRk907VfJ8vR4m8aLeBwf2jtNZL/PYHr9vXisqS2X7x7R1wBtzT19gFjKl QiXJdLzrhL5QZlwzEUuigczvjMFtEPlRrilxO48/G2Eknpn1w4QVcOO8bjOLFFzH6L wHN7J2l7aHTMA== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Mateusz Guzik , Christian Brauner , Sasha Levin , viro@zeniv.linux.org.uk, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org Subject: [PATCH AUTOSEL 6.11 020/244] exec: don't WARN for racy path_noexec check Date: Wed, 25 Sep 2024 07:24:01 -0400 Message-ID: <20240925113641.1297102-20-sashal@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240925113641.1297102-1-sashal@kernel.org> References: <20240925113641.1297102-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.11 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 96D5F80015 X-Stat-Signature: ox8kt6afa9zeemizt5wuxk6yp7npup6d X-Rspam-User: X-HE-Tag: 1727264249-738927 X-HE-Meta: U2FsdGVkX1+XXeRQpaK4vbR1xNpb7L6+MJxHQSONq1YtCiA0fZfuiX7YTlFXudh796PSqxz7zmi3E0USSfxU3TExYrXTDfw+jKxG95OAZPyheJ0OhMuiBh+SdlLK7XzEJSkfWF07LX+QQd9grXA/EHkvDsB40jH4UwUhl+KR8l8wdB7jwGWa/GSnAdYwcMpK0u4FOlORSR238ADti1sf2QOnBBfU5KL3MLHaRp6vdN9vTnRqgkuTv+0cGeotjEzy31jn3XR0JL8uT0ZBcijgx6JudSN1sTueHZEwb9+QilBHbBLcf9mdhcyMLOIGipb/Huhy7AgLwUF4cy4IlKtO310jveZTXz8JnCppttqYgOV/HAON+U6GE49tsGra/GNwe37KMiEq6VvXK2NBzk2aTzr7lESTkX99A0kyGklk993HdWGrjfnUp/t5hvo9pFO/ZbpeyGwRYEHx/dlYYzHC8ZEkqFLgOOoVgFat7TLXMlbcqQQdgkFfLn9tO7LKr7S1zfr4FBFxqIU4Nc4MXtDS7CQd0GHbVkRr6UFXJyns5xtkorIlObCulzwxZIH9d5DmszUkz81IyuhlHmcV0i9m4WFn6rpEx0yhZyd7+6y0qTrc2ejc4RJdFcrpO0nn1l8HQ8XRe/mzSmFto5CFMiMUvdQD2pNknxHzcs57IYFLfdNvYSxFYscxmXye2ZqDXk+5J0hjD3nv7EEiAmQt3wFgf4++Ks+FvLN1kHLKy86Ah5rXjOcuKqdU6Hb3ztGZNDC/XERCW2zOeqqQOISdFyb/Z+Rb7AcFAKRHFUU9cx3sEUlp+f5GvejjIP7/qpOEcsD6Vlp9+kqLVFxZaXxhlInnOBX1C7wq18xa1yoE1qfOByKCvCzi3WIaciE6p4/m808a3s3dl/PIbFxrYc+ggDF4xrh/EyttJAFkocpC9IDCTgjqEi51J49EHh3zq///CfN3oebTnmPmd53LS89PdZc dqhZJxFt n/wNamSyCpWwFQLm4vH8WVoYNuoC86H4mapwbi4oH5I258IpxQF2IgshXapoF7VdvHchv18Rs9YVlwz5csZEzilSVXiivZPnTRGwY8RyMT4BAUUbwEaj1TmzmsbD/3gNXFauD21t004kFmfb76VmcNIF/ZPFsSDT2quha9/LW7vbR43oYF13EiEQzJv5b5ufUGxUAvejwtxzm/2NT2jBvDhSt1j/s83pIkf2qhFJVEaRObtiJcZpw1Z1do++KFtoiLtomxfydU4GV22T5LK++f3/fqLk4iWClg/1yCZdXPpvYe4I= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: From: Mateusz Guzik [ Upstream commit 0d196e7589cefe207d5d41f37a0a28a1fdeeb7c6 ] Both i_mode and noexec checks wrapped in WARN_ON stem from an artifact of the previous implementation. They used to legitimately check for the condition, but that got moved up in two commits: 633fb6ac3980 ("exec: move S_ISREG() check earlier") 0fd338b2d2cd ("exec: move path_noexec() check earlier") Instead of being removed said checks are WARN_ON'ed instead, which has some debug value. However, the spurious path_noexec check is racy, resulting in unwarranted warnings should someone race with setting the noexec flag. One can note there is more to perm-checking whether execve is allowed and none of the conditions are guaranteed to still hold after they were tested for. Additionally this does not validate whether the code path did any perm checking to begin with -- it will pass if the inode happens to be regular. Keep the redundant path_noexec() check even though it's mindless nonsense checking for guarantee that isn't given so drop the WARN. Reword the commentary and do small tidy ups while here. Signed-off-by: Mateusz Guzik Link: https://lore.kernel.org/r/20240805131721.765484-1-mjguzik@gmail.com [brauner: keep redundant path_noexec() check] Signed-off-by: Christian Brauner Signed-off-by: Sasha Levin --- fs/exec.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 50e76cc633c4b..caae051c5a956 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -145,13 +145,11 @@ SYSCALL_DEFINE1(uselib, const char __user *, library) 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. + * Check do_open_execat() for an explanation. */ error = -EACCES; - if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) || - path_noexec(&file->f_path))) + if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode)) || + path_noexec(&file->f_path)) goto exit; error = -ENOEXEC; @@ -954,7 +952,6 @@ EXPORT_SYMBOL(transfer_args_to_stack); static struct file *do_open_execat(int fd, struct filename *name, int flags) { struct file *file; - int err; struct open_flags open_exec_flags = { .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC, .acc_mode = MAY_EXEC, @@ -971,24 +968,20 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) file = do_filp_open(fd, name, &open_exec_flags); if (IS_ERR(file)) - goto out; + return file; /* - * 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. + * In the past the regular type check was here. It moved to may_open() in + * 633fb6ac3980 ("exec: move S_ISREG() check earlier"). Since then it is + * an invariant that all non-regular files error out before we get here. */ - err = -EACCES; - if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) || - path_noexec(&file->f_path))) - goto exit; + if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode)) || + path_noexec(&file->f_path)) { + fput(file); + return ERR_PTR(-EACCES); + } -out: return file; - -exit: - fput(file); - return ERR_PTR(err); } /** -- 2.43.0