Module Name:    src
Committed By:   kre
Date:           Sun Mar 19 17:55:57 UTC 2023

Modified Files:
        src/bin/sh: error.c exec.c

Log Message:
Do a better job handling EACCES errors from exec() calls.   If the
EACCES is from the namei(), treat it just like ENOENT or ENOTDIR
(and if that is the final error, the exit status from a failed exec
will be 127).   If the EACCES is from the exec() itself, that indicates
the file to be run exists, but has no 'x' permission.   That's a
meaningful error (as distinct from just "yet another PATH element
search failure").

While here, return the first meaingful error we encountered while
searching PATH, rather than the last (and ENOENT if there are none
of those).

This change results in some failed command executions returning status
127 now, where they returned 126 before - which better reflects the
intent of those values (127 is simply "not found" whereas 126 is "found
but couldn't be executed").

We still do nothing to distinguish errors encountered looking up the
command name give, with errors encountered (by the kernel) attempting to
run an interpreter needed for the exec to succeed (#! line path, or
/libexec/ld.elf_so and similar - or anything else of a similar nature).


To generate a diff of this commit:
cvs rdiff -u -r1.44 -r1.45 src/bin/sh/error.c
cvs rdiff -u -r1.57 -r1.58 src/bin/sh/exec.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/bin/sh/error.c
diff -u src/bin/sh/error.c:1.44 src/bin/sh/error.c:1.45
--- src/bin/sh/error.c:1.44	Wed Nov 10 15:26:34 2021
+++ src/bin/sh/error.c	Sun Mar 19 17:55:57 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: error.c,v 1.44 2021/11/10 15:26:34 kre Exp $	*/
+/*	$NetBSD: error.c,v 1.45 2023/03/19 17:55:57 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)error.c	8.2 (Berkeley) 5/4/95";
 #else
-__RCSID("$NetBSD: error.c,v 1.44 2021/11/10 15:26:34 kre Exp $");
+__RCSID("$NetBSD: error.c,v 1.45 2023/03/19 17:55:57 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -310,6 +310,7 @@ struct errname {
 
 STATIC const struct errname errormsg[] = {
 	{ EINTR,	ALL,	"interrupted" },
+	{ EACCES,	E_EXEC,	"no execute permission" },
 	{ EACCES,	ALL,	"permission denied" },
 	{ EIO,		ALL,	"I/O error" },
 	{ EEXIST,	ALL,	"file exists" },

Index: src/bin/sh/exec.c
diff -u src/bin/sh/exec.c:1.57 src/bin/sh/exec.c:1.58
--- src/bin/sh/exec.c:1.57	Tue Nov 16 11:28:29 2021
+++ src/bin/sh/exec.c	Sun Mar 19 17:55:57 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: exec.c,v 1.57 2021/11/16 11:28:29 kre Exp $	*/
+/*	$NetBSD: exec.c,v 1.58 2023/03/19 17:55:57 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)exec.c	8.4 (Berkeley) 6/8/95";
 #else
-__RCSID("$NetBSD: exec.c,v 1.57 2021/11/16 11:28:29 kre Exp $");
+__RCSID("$NetBSD: exec.c,v 1.58 2023/03/19 17:55:57 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -125,18 +125,56 @@ void
 shellexec(char **argv, char **envp, const char *path, int idx, int vforked)
 {
 	char *cmdname;
-	int e;
+	int e, action;
+	struct stat statb;
+
+	action = E_EXEC;
 
 	if (strchr(argv[0], '/') != NULL) {
 		tryexec(argv[0], argv, envp, vforked);
 		e = errno;
+		if (e == EACCES && stat(argv[0], &statb) == -1)
+			action = E_OPEN;
 	} else {
 		e = ENOENT;
 		while ((cmdname = padvance(&path, argv[0], 1)) != NULL) {
 			if (--idx < 0 && pathopt == NULL) {
+				/*
+				 * tryexec() does not return if it works.
+				 */
 				tryexec(cmdname, argv, envp, vforked);
-				if (errno != ENOENT && errno != ENOTDIR)
-					e = errno;
+				/*
+				 * If do not already have a meaningful error
+				 * from earlier in the PATH, examine this one
+				 * if it is a simple "not found", just keep
+				 * searching.
+				 */
+				if (e == ENOENT &&
+				    errno != ENOENT && errno != ENOTDIR) {
+					/*
+					 * If the error is from permission
+					 * denied on the path search (a call
+					 * to stat() also fails) ignore it
+					 * (just continue with the search)
+					 * If it is EACCESS and the file exists
+					 * (the stat succeeds) that means no
+					 * 'x' perm on the file itself, which
+					 * is a meaningful error, this will be
+					 * the one reported if no later PATH
+					 * element actually succeeds.
+					 */
+					if (errno == EACCES) {
+						if (stat(cmdname, &statb) != -1)
+							e = EACCES;
+					} else {
+						/*
+						 * any other error we will
+						 * remember as the significant
+						 * error
+						 */
+						e = errno;
+					}
+				}
 			}
 			stunalloc(cmdname);
 		}
@@ -145,11 +183,17 @@ shellexec(char **argv, char **envp, cons
 	/* Map to POSIX errors */
 	switch (e) {
 	case EACCES:	/* particularly this (unless no search perm) */
-		/*
-		 * should perhaps check if this EACCES is an exec()
-		 * EACESS or a namei() EACESS - the latter should be 127
-		 * - but not today
-		 */
+		if (action == E_OPEN) {
+			/*
+			 * this is an EACCES from namei
+			 * ie: no permission to search the path given
+			 * rather than an EACCESS from exec
+			 * ie: no 'x' bit on the file to be executed
+			 */
+			exerrno = 127;
+			break;
+		}
+		/* FALLTHROUGH */
 	case EINVAL:	/* also explicitly these */
 	case ENOEXEC:
 	default:	/* and anything else */
@@ -166,7 +210,7 @@ shellexec(char **argv, char **envp, cons
 	CTRACE(DBG_ERRS|DBG_CMDS|DBG_EVAL,
 	    ("shellexec failed for %s, errno %d, vforked %d, suppressint %d\n",
 		argv[0], e, vforked, suppressint));
-	exerror(EXEXEC, "%s: %s", argv[0], errmsg(e, E_EXEC));
+	exerror(EXEXEC, "%s: %s", argv[0], errmsg(e, action));
 	/* NOTREACHED */
 }
 

Reply via email to