Leave a started program a descriptor for itself, not for its base directory

kal_process_spawn duplicated the base directory without close-on-exec and
passed the copy to execveat, so that a `#!` script or a binfmt_misc
interpreter could open /dev/fd/<n>/<name> after the replacement. The copy
stays open in the started program and is passed on to whatever it starts. The
base is often `/`, and a command run under bubblewrap could reach the host
file system through /proc/self/fd/<n>/.

Open the program itself with O_PATH, without O_CLOEXEC, and start it with
execveat(fd, "", ..., AT_EMPTY_PATH). The interpreter is then given
/dev/fd/<n>, which names the file, and nothing can be walked out of a
descriptor for a file. A name that cannot be opened is reported through the
same pipe as one that cannot be started, with the same errno.

A script now sees $0 as /dev/fd/<n> and not as a path under its directory,
as with fexecve. The descriptor can still be reopened through
/proc/self/fd/<n>, so a program that sandboxes what it starts should close
what it inherits.

The test starts a `#!` script and checks that it inherits no descriptor for
the base directory. It fails without the change.

Author: yspbwx2010

src/process.cpp

@@ -246,7 +246,7 @@ int kal_process_spawn(const kal_spawn* how,
 
         // THE DIRECTORY THE PROGRAM RUNS IN, AND THIS LINE IS THE WHOLE OF IT.
         //
-        // `execveat' below takes `b' as a dirfd, but that only RESOLVES the
+        // `openat' below takes `b' as a dirfd, but that only RESOLVES the
         // name --- resolving a name is not entering a directory, which is what
         // the comment here used to get wrong. Until 0.11 there was no second
         // directory to enter, and a started program ran wherever this
@@ -279,8 +279,9 @@ int kal_process_spawn(const kal_spawn* how,
                 okl::sys(okl::nr_exit_group, 127);
         }
 
-        // THE BASE IS DUPLICATED SO THAT IT SURVIVES THE REPLACEMENT, AND
-        // WITHOUT THIS A WHOLE CLASS OF PROGRAMS COULD NOT BE STARTED AT ALL.
+        // WHAT THE STARTED PROGRAM KEEPS IS A DESCRIPTOR FOR THE PROGRAM, NOT
+        // FOR THE DIRECTORY IT WAS FOUND IN, AND WITHOUT A DESCRIPTOR AT ALL A
+        // WHOLE CLASS OF PROGRAMS COULD NOT BE STARTED.
         //
         // `execveat' with a dirfd and a relative name gives the program's name to
         // the kernel as `/dev/fd/<dirfd>/<name>'. That spelling is invisible to a
@@ -289,31 +290,55 @@ int kal_process_spawn(const kal_spawn* how,
         // needs an INTERPRETER: a `#!' script, or a binary of another
         // architecture registered through `binfmt_misc'. The kernel then starts
         // the interpreter and hands it that name to open --- AFTER the
-        // replacement, by which time a close-on-exec dirfd is gone. The
+        // replacement, by which time a close-on-exec descriptor is gone. The
         // interpreter is told the script does not exist.
         //
         // Measured in twenty lines of plain C, with everything else identical:
         //
-        //     dirfd WITH O_CLOEXEC       execveat -> ENOENT
-        //     dirfd WITHOUT O_CLOEXEC    STARTED ok
+        //     descriptor WITH O_CLOEXEC       execveat -> ENOENT
+        //     descriptor WITHOUT O_CLOEXEC    STARTED ok
         //
         // It is not a property of one architecture. It was FOUND on aarch64,
         // where every foreign binary needs the binfmt interpreter and so every
         // start failed --- and it was mistaken there for a limit of the emulator.
         // It reproduces natively on x86_64 with a `#!' script, which is what a
         // consumer meets on any machine.
         //
-        // Duplicated HERE, in the started image, and not where the preopens are
-        // made: the caller's own descriptors stay close-on-exec, which is what
-        // every other operation of this implementation relies upon. `dup' clears
-        // the flag by definition, so the copy is the exec-visible one.
-        const okl_long visible = okl::sys(okl::nr_fcntl, b, okl::f_dupfd, 0);
-        const okl_long base = okl::failed(visible) ? b : visible;
+        // The descriptor that survives must not be the base. A copy of the base
+        // is a handle on a whole directory, often `/', and a program that runs
+        // other programs in a sandbox hands it on to them, where
+        // `/proc/self/fd/<n>/' reaches whatever the sandbox hid. So the program
+        // itself is opened here, with `O_PATH' because it is only to be named
+        // and not read, and without `O_CLOEXEC' for the reason above, and
+        // `execveat' is given that descriptor and an empty name. The
+        // interpreter's `/dev/fd/<n>' then names the file, and nothing can be
+        // walked out of a descriptor for a file.
+        //
+        // What is given up: a script now sees `$0' as `/dev/fd/<n>' and not as
+        // a path under its directory, so one that locates its neighbours with
+        // `dirname "$0"' no longer finds them. That is the price of leaving a
+        // descriptor for the file and not for the directory, the same one
+        // `fexecve' charges. The descriptor still names the program, and a
+        // caller that sandboxes what it starts should close what it inherits.
+        //
+        // Opened HERE, in the started image, and not where the preopens are made:
+        // the caller's own descriptors stay close-on-exec, which is what every
+        // other operation of this implementation relies upon.
+        //
+        // A name that cannot be opened is reported exactly as one that cannot be
+        // started is, through the same pipe.
+        const okl_long exe = okl::sys(okl::nr_openat, b, reinterpret_cast<okl_long>(p.buf),
+                                      okl::o_path, 0);
+        if (okl::failed(exe)) {
+            report.say(exe);
+            okl::sys(okl::nr_exit_group, 127);
+            for (;;) { }
+        }
 
         const okl_long why =
-            okl::sys(okl::nr_execveat, base, reinterpret_cast<okl_long>(p.buf),
+            okl::sys(okl::nr_execveat, exe, reinterpret_cast<okl_long>(""),
                      reinterpret_cast<okl_long>(args.slots),
-                     reinterpret_cast<okl_long>(envs.slots), 0);
+                     reinterpret_cast<okl_long>(envs.slots), okl::at_empty_path);
         // Reached only when the replacement did not happen, because when it does
         // there is nothing here to reach.
         report.say(why);

src/sys.h

@@ -256,20 +256,17 @@ enum : okl_long {
     o_creat = 0100, o_excl = 0200, o_trunc = 01000, o_append = 02000,
     o_cloexec = 02000000,
 
+    // A descriptor that only NAMES a node, and the empty name `execveat' accepts
+    // for it. See the start in `kal_process_spawn'.
+    o_path = 010000000, at_empty_path = 0x1000,
+
     // THE LOWEST FREE DESCRIPTOR AT OR ABOVE A BOUND, which is the one
     // primitive that moves a descriptor out of the way WITHOUT NAMING the
     // number it moves to --- and therefore without closing whatever a caller
     // already had there. `dup3' cannot do this: it is told the number, and it
     // closes what is on it.
     f_dupfd_cloexec = 1030,
 
-    // The same primitive WITHOUT the flag, which is the point of having both.
-    // A descriptor duplicated this way survives a replacement, and starting a
-    // program that needs an interpreter depends on exactly that --- see the
-    // duplication in `kal_process_spawn'. `dup' would do as well and this
-    // architecture pair does not agree on whether it exists.
-    f_dupfd = 0,
-
     // THE OPEN-FILE FORM AND NOT THE PROCESS FORM, WHICH IS THE WHOLE
     // DIFFERENCE.
     //

tests/conformance_additions.cpp

@@ -181,6 +181,37 @@ int main() {
         kal_fs_remove(here(), prog, pn);
     }
 
+    // A started `#!' script is handed `/dev/fd/<n>' by the kernel and opens it
+    // after the replacement, so a descriptor has to be left behind for it. It
+    // must be one for the script and not for the directory the name was found
+    // in: a program that starts other programs under a sandbox passes every
+    // descriptor on, and a directory among them reaches outside it. The base
+    // and the working directory are the same here, so it is `.' that must not
+    // be among them.
+    {
+        const char* prog = "okl-fd-probe.tmp";
+        const kal_uintptr pn = std::strlen(prog);
+        check(put(prog, "#!/bin/sh\nfor f in /proc/self/fd/*; do\n"
+                        "  [ \"$f\" -ef . ] && exit 1\ndone\nexit 0\n"),
+              "a script is written");
+        check(kal_fs_set_executable_at(here(), prog, pn, 1) == kal_ok,
+              "the script is recorded as startable");
+
+        kal_process p{};
+        const char* argv[1] = { prog };
+        const kal_uintptr lens[1] = { pn };
+        const kal_spawn how{ here(), here(), nullptr, nullptr, 0, 0 };
+        check(kal_process_spawn(&how, prog, pn, argv, lens, 1,
+                                nullptr, nullptr, 0, nullptr, &p) == kal_ok,
+              "a `#!' script is started");
+        int status = -1, terminated = -1;
+        kal_process_wait(p, &status, &terminated);
+        check(terminated == 0 && status == 0,
+              "the script ran, and it inherited no descriptor for its base directory");
+        kal_process_close(p);
+        kal_fs_remove(here(), prog, pn);
+    }
+
     std::printf("openkal-linux: the operations version 0.5 added\n");
     return failures == 0 ? 0 : 1;
 }