linux-kernel - [PATCH] exec: fix search binary handler to have more clear retvals

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>] [day] [month] [year] [list]

Message-ID: <20241116063947.GA216691@lichtman.org>
Date: Sat, 16 Nov 2024 06:39:47 +0000
From: Nir Lichtman <nir@...htman.org>
To: ebiederm@...ssion.com, kees@...nel.org, viro@...iv.linux.org.uk,
	brauner@...nel.org, jack@...e.cz, linux-kernel@...r.kernel.org
Subject: [PATCH] exec: fix search binary handler to have more clear retvals

Preamble: In case no suitable binary format is found,
and the enumeration itself does not return any retval failure,
the flow either goes and attempts to load a kernel module to
add support of the unknown binary format, or just returns.
In both of these cases the current value of retval is returned.
Before the enumeration, retval is initialized to be
"No such file or directory" but can be amended during the loop

Problem: The current situation forwards retval as is in these
cases which is not necessarily suitable for the flow, for example
when no binary formats are configured in the kernel, the enumeration
will not be entered and retval will be returned as -ENOENT which will
lead to a very misleading error of "No such file or directory".

Solution: Refactor to remove the initialization of retval ot -ENOENT
and clearly return suitable -ENOEXEC in those flows.

Signed-off-by: Nir Lichtman <nir@...htman.org>
---

v2: Remove retval init to -ENOENT entirely, and return suitable
error values in the relavent flows instead.

Side-note: This is sort-of a v2 of
"fix no kernel module found error to be more clear"
Even-though I have sent yesterday that it should be disregarded,
after further research, I found that is still relavent as explained
above.

 fs/exec.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 3b4c7548427f..28755088175f 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1741,7 +1741,6 @@ static int search_binary_handler(struct linux_binprm *bprm)
 	if (retval)
 		return retval;

-	retval = -ENOENT;
  retry:
 	read_lock(&binfmt_lock);
 	list_for_each_entry(fmt, &formats, lh) {
@@ -1763,14 +1762,14 @@ static int search_binary_handler(struct linux_binprm *bprm)
 	if (need_retry) {
 		if (printable(bprm->buf[0]) && printable(bprm->buf[1]) &&
 		    printable(bprm->buf[2]) && printable(bprm->buf[3]))
-			return retval;
+			return -ENOEXEC;
 		if (request_module("binfmt-%04x", *(ushort *)(bprm->buf + 2)) < 0)
-			return retval;
+			return -ENOEXEC;
 		need_retry = false;
 		goto retry;
 	}

-	return retval;
+	return -ENOEXEC;
 }

 /* binfmt handlers will call back into begin_new_exec() on success. */
-- 
2.39.2