From 3335de91437bc983c95cfab86489ceb3a0b0a6aa Mon Sep 17 00:00:00 2001

From: Chris Down <chris@chrisdown.name>

Date: Tue, 25 Aug 2020 21:59:11 +0100

Subject: [PATCH 1/2] path: Skip directories when finalising $PATH search

Imagine $PATH /a:/b. There is an echo command at /b/echo. Under this

configuration, this works fine:

    % systemd-run --user --scope echo .

    Running scope as unit: run-rfe98e0574b424d63a641644af511ff30.scope

    .

However, if I do `mkdir /a/echo`, this happens:

    % systemd-run --user --scope echo .

    Running scope as unit: run-rcbe9369537ed47f282ee12ce9f692046.scope

    Failed to execute: Permission denied

We check whether the resulting file is executable for the performing

user, but of course, most directories are anyway, since that's needed to

list within it. As such, another is_dir() check is needed prior to

considering the search result final.

Another approach might be to check S_ISREG, but there may be more gnarly

edge cases there than just eliminating this obviously pathological

example, so let's just do this for now.

---

 src/basic/path-util.c | 3 +++

 1 file changed, 3 insertions(+)

diff --git a/src/basic/path-util.c b/src/basic/path-util.c

index c4e022b3a1..d3b4978239 100644

--- a/src/basic/path-util.c

+++ b/src/basic/path-util.c

@@ -637,6 +637,9 @@ int find_binary(const char *name, char **ret) {

                 if (!j)

                         return -ENOMEM;

+                if (is_dir(j, true))

+                        continue;

+

                 if (access(j, X_OK) >= 0) {

                         /* Found it! */

-- 

2.26.2

From 2f94890f37c13dcd680a63876ed6d34f8e66d0a3 Mon Sep 17 00:00:00 2001

From: Chris Down <chris@chrisdown.name>

Date: Wed, 26 Aug 2020 18:49:27 +0100

Subject: [PATCH 2/2] path: Improve $PATH search directory case

Previously:

1. last_error wouldn't be updated with errors from is_dir;

2. We'd always issue a stat(), even for binaries without execute;

3. We used stat() instead of access(), which is cheaper.

This change avoids all of those, by only checking inside X_OK-positive

case whether access() works on the path with an extra slash appended.

Thanks to Lennart for the suggestion.

---

 src/basic/path-util.c | 25 ++++++++++++++++++-------

 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/basic/path-util.c b/src/basic/path-util.c

index d3b4978239..7b0863f749 100644

--- a/src/basic/path-util.c

+++ b/src/basic/path-util.c

@@ -637,16 +637,27 @@ int find_binary(const char *name, char **ret) {

                 if (!j)

                         return -ENOMEM;

-                if (is_dir(j, true))

-                        continue;

-

                 if (access(j, X_OK) >= 0) {

-                        /* Found it! */

+                        _cleanup_free_ char *with_dash;

-                        if (ret)

-                                *ret = path_simplify(TAKE_PTR(j), false);

+                        with_dash = strjoin(j, "/");

+                        if (!with_dash)

+                                return -ENOMEM;

-                        return 0;

+                        /* If this passes, it must be a directory, and so should be skipped. */

+                        if (access(with_dash, X_OK) >= 0)

+                                continue;

+

+                        /**

+                         * We can't just `continue` inverting this case, since we need to update last_error.

+                         */

+                        if (errno == ENOTDIR) {

+                                /* Found it! */

+                                if (ret)

+                                        *ret = path_simplify(TAKE_PTR(j), false);

+

+                                return 0;

+                        }

                 }

                 /* PATH entries which we don't have access to are ignored, as per tradition. */

-- 

2.26.2