From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6A6DC18D629; Mon, 28 Oct 2024 06:32:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730097126; cv=none; b=PpquSdxignNwIY1bNIErCYdOk/qnn3s/53SbJEptHDzK1JPFI9yGV58XWvqD7hzA01tRx8080VaWpFDeRymxFLTMoP4Ls2BoCvk/QljxzXTjWJLQuRMmwzGvVzZo3nYwd3KcxAq3t2egHtRob3TlRq8hiC319yNmUJuK0WMeojM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730097126; c=relaxed/simple; bh=PEnxsS8GPdv5zsRDzw4RI5mz1Qn0fiWaOpCOd6Hh74E=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=FYksI9zNpmykXZLInW3VGC0oSsucxCvMyFt+8OZ0T5qReJZP6okwsnFB6cvS4wNOuLcmTTQboFZI0wxBIKUpniviv80kEK0JZEdmFoC6GtcLDJyOO6mx3KqGuE20ahVz8xkyFtkhoKWRRSSc1jdfUh/NXHBuqwekWJsiPh4TpDU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=1r2+ZJ2f; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="1r2+ZJ2f" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 051E8C4CEC7; Mon, 28 Oct 2024 06:32:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1730097126; bh=PEnxsS8GPdv5zsRDzw4RI5mz1Qn0fiWaOpCOd6Hh74E=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=1r2+ZJ2f6kMr6eRkBGp9MLABR8iKWQr7arhvJ6rVeDxXaS7Vi+uQup6qvCSJ0T8SM P8/nnriAriYGCK7kg1FTYUw5PhyeQcIBB3fEWJKO3nvYT3lOauT0BCqGDPbz5t84eX DddIBWjPECGkiIYUPKlSZiyW7OvmQjhNVpd+wIdg= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Mateusz Guzik , Christian Brauner , Thadeu Lima de Souza Cascardo , Sasha Levin Subject: [PATCH 5.15 40/80] exec: dont WARN for racy path_noexec check Date: Mon, 28 Oct 2024 07:25:20 +0100 Message-ID: <20241028062253.735097443@linuxfoundation.org> X-Mailer: git-send-email 2.47.0 In-Reply-To: <20241028062252.611837461@linuxfoundation.org> References: <20241028062252.611837461@linuxfoundation.org> User-Agent: quilt/0.67 X-stable: review X-Patchwork-Hint: ignore Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 5.15-stable review patch. If anyone has any objections, please let me know. ------------------ 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 [cascardo: keep exit label and use it] Signed-off-by: Thadeu Lima de Souza Cascardo Signed-off-by: Sasha Levin --- fs/exec.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 26f0b79cb4f94..8395e7ff7b940 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -142,13 +142,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; fsnotify_open(file); @@ -919,16 +917,16 @@ 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))) + if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode)) || + path_noexec(&file->f_path)) goto exit; err = deny_write_access(file); @@ -938,7 +936,6 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) if (name->name[0] != '\0') fsnotify_open(file); -out: return file; exit: -- 2.43.0