Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/bin/sh Do a better job handling EACCES errors from exec() ca...



details:   https://anonhg.NetBSD.org/src/rev/0a1467712cf0
branches:  trunk
changeset: 373918:0a1467712cf0
user:      kre <kre%NetBSD.org@localhost>
date:      Sun Mar 19 17:55:57 2023 +0000

description:
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).

diffstat:

 bin/sh/error.c |   5 ++-
 bin/sh/exec.c  |  66 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 58 insertions(+), 13 deletions(-)

diffs (136 lines):

diff -r c750c7955bb8 -r 0a1467712cf0 bin/sh/error.c
--- a/bin/sh/error.c    Sun Mar 19 17:47:48 2023 +0000
+++ b/bin/sh/error.c    Sun Mar 19 17:55:57 2023 +0000
@@ -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 @@
 
 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" },
diff -r c750c7955bb8 -r 0a1467712cf0 bin/sh/exec.c
--- a/bin/sh/exec.c     Sun Mar 19 17:47:48 2023 +0000
+++ b/bin/sh/exec.c     Sun Mar 19 17:55:57 2023 +0000
@@ -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 @@
 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 @@
        /* 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 @@
        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 */
 }
 



Home | Main Index | Thread Index | Old Index