fix(daemon): SIGABRT on shutdown, SIGCHLD=SIG_IGN breaking every exec, in-process runCrate on HTTP workers, fd CLOEXEC, IPv6-route gateway, stack DNS never stopped, physmem 500 (1.1.26)

CHANGELOG.md

@@ -6,6 +6,86 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ---
 
+## [1.1.26] — 2026-09-14
+
+**Correctness: seven daemon/runtime fixes from a correctness-lens audit
+(exception safety, process/fd hygiene, threading) — "the daemon works".**
+
+- **`crated` shut down with SIGABRT on every clean stop — `daemon/server.cpp`
+  (HIGH).** The Unix-socket `httplib::Server` lived only inside its
+  thread's lambda, so `Server::stop()` could never stop it, `listen()`
+  never returned, and `~Impl` destroyed a still-joinable `std::thread` →
+  `std::terminate`. With the default `unixSocket` configured this hit
+  every `service crated stop`. The UDS server is now an `Impl` member;
+  `stop()` stops it and joins its thread.
+
+- **Enabling `console.port` broke every exec in the daemon —
+  `daemon/ws_console.cpp` (HIGH).** `WsConsole::start` set
+  `signal(SIGCHLD, SIG_IGN)` process-wide. On FreeBSD that flags
+  `PS_NOCLDWAIT`: children are auto-reaped and `waitpid()` returns
+  `ECHILD` for **all** of crated's children, so every
+  `Util::execCommand*`/`execPipeline*` (privops verbs, stats, export/
+  import, control-socket ops) threw "waitpid failed" even on success, and
+  `JailExec` decoded an uninitialized status. The SIG_IGN is gone;
+  session shells are reaped explicitly (SIGTERM, ~2s grace, then SIGKILL
+  + blocking wait).
+
+- **`POST …/start` and `…/restart` ran the container in-process —
+  `daemon/routes.cpp` (HIGH).** Both called `runCrate()` on the httplib
+  worker thread: (a) the worker blocked for the jail's lifetime — a
+  service-only container never returned, and after ~8 starts the whole
+  HTTP API incl. `/healthz` stopped responding; (b) `runCrate` installed
+  SIGINT/SIGTERM handlers process-wide, so `service crated stop` hung;
+  (c) `jailXname`/`FwSlots`/`FwUsers` are keyed on `getpid()`, so every
+  daemon-started jail shared one key — a later start overwrote the first
+  jail's firewall slot and the first teardown removed the shared NAT rule
+  while other jails still ran. Both routes now fork+setsid+exec
+  `crate run -f <file>` as its own process (the control-socket start
+  path already did this). **API note:** the response is now async —
+  `{"started":true,"async":true,"pid":N,…}` is returned as soon as the
+  child is spawned; poll `GET /api/v1/containers/:name` for state.
+
+- **fds leaked into long-lived children — `lib/util.cpp`,
+  `lib/jail_query.cpp`, `daemon/{control_socket,privops_listener,ws_console}.cpp`
+  (HIGH).** Capture pipes were created without `O_CLOEXEC`, so a pipe's
+  write end leaked into any child forked concurrently on another daemon
+  thread (the control-socket `crate run` supervisor, ws-console shells —
+  both long-lived), and the reading request thread waited for an EOF that
+  only came when *that* child exited. Listening sockets leaked the same
+  way (→ `EADDRINUSE` on `service crated restart`; a jailed shell holding
+  the host daemon's listener). Pipes now use `pipe2(O_CLOEXEC)` (dup2
+  clears it on the intended child's stdio) and the three hand-rolled
+  listeners use `SOCK_CLOEXEC`. Not yet covered: httplib's own TCP
+  listener and `accept()`ed connection fds — recorded in `TODO`.
+
+- **NAT-mode `crate run` failed on any host with an IPv6 default route —
+  `lib/run_net.cpp` (MED).** `detectGateway` ran `netstat -rn` without
+  `-f inet`, so the inet6 table's `default` line doubled the token count
+  and the strict `!= 4` check threw "Unable to determine host's gateway".
+  Now `-f inet` and `>= 4`, mirroring the IPv6 query.
+
+- **Stack DNS was never stopped — `lib/stack.cpp` (MED).** unbound ran
+  with `do-daemonize: yes`, so the pid `startStackDns` returned was the
+  short-lived launcher (its 200ms liveness check then reported "exited
+  immediately" on *success*), and `stack down` passed `-1` because it is
+  a different process anyway → an orphan unbound stayed bound to
+  `<gateway>:53` and the next `up` failed to bind. unbound now runs in the
+  foreground under `setsid`, and `stopStackDns` signals the pid from the
+  per-network `unbound.pid` file.
+
+- **`GET /api/v1/host` was a permanent 500 on 64-bit — `daemon/routes.cpp`,
+  `lib/util.cpp` (MED).** `hw.physmem` is `CTLTYPE_ULONG` (8 bytes) but
+  was read through the 4-byte `getSysctlInt` → `ENOMEM`. New
+  `Util::getSysctlUInt64`.
+
+All seven are runtime-only (daemon / FreeBSD runtime; no pure unit
+surface) and are compile-gated by the FreeBSD lite build. Exercising
+them on a live host is recommended, especially the async start/restart
+semantics. The remaining correctness-audit items (fd hygiene for
+httplib/accept fds, teardown-after-throw leaks in `run.cpp`, `FwUsers`
+dead-pid GC, listener read timeouts, and a LOW batch) are listed in
+`TODO`.
+
 ## [1.1.25] — 2026-07-11
 
 **Security & robustness: five fixes from a third-pass audit of the

TODO

@@ -100,6 +100,69 @@ harden BEFORE wiring this code up:
   for vmName/vol.tag before any of this gets a caller. memoryToKiB
   (vm_spec.cpp:84) also has an unchecked `val*1024*1024` overflow.
 
+Remaining from the 2026-09 correctness-lens audit (its seven HIGH/MED
+daemon items shipped as 1.1.26; these are what is left, all runtime-only
+and best validated on a live FreeBSD host):
+
+* (correctness, MED) lib/stack.cpp `stack up` still calls runCrate()
+  IN-PROCESS and sequentially per container (the same defect 1.1.26
+  fixed for the daemon's HTTP start/restart): a service-only first
+  container never returns, so the second is never started. Spawn each
+  `crate run` as its own process like daemon/routes.cpp spawnCrateRun.
+
+* (correctness, MED) fd hygiene, part 2: 1.1.26 put O_CLOEXEC on the
+  capture pipes and SOCK_CLOEXEC on the three hand-rolled listeners.
+  Still inherited by forked children: httplib's own TCP listener
+  (server.cpp — needs httplib::Server::set_socket_options or an
+  fcntl(FD_CLOEXEC) after bind) and every accept()ed connection fd in
+  control_socket.cpp / privops_listener.cpp / ws_console.cpp (use
+  accept4(SOCK_CLOEXEC)). Consequence today: a long-lived child can
+  pin another client's connection open.
+
+* (correctness, MED) lib/run.cpp registers several teardown handlers
+  AFTER the operations that can throw, so the error path leaks host
+  resources (distinct from the documented teardown-ORDER item above):
+  NetworkLease::allocateFor succeeds (~:1153) but configureStaticIp/
+  copyFile throws before releaseLeaseAtEnd is set → the lease row for
+  the unique jail-<name>-<hex> stays in network-leases.txt forever
+  (nothing reclaims it; clean.cpp does not touch leases); same for
+  IPv6 (~:1184). Bridge auto-created (~:1090) then setUp/
+  createBridgeEpair throws before destroyBridgeEpairAtEnd.reset
+  (~:1115) → bridge + epair leak; run_net.cpp createBridgeEpair /
+  the extras loop (~:1664) leak an epairN pair per failed attempt.
+  Fix shape: register the RunAtEnd immediately after each successful
+  acquisition, before the next fallible step.
+
+* (correctness, MED) lib/ctx.cpp FwUsers has no dead-pid garbage
+  collection (FwSlots does, ctx.cpp:~200) and lib/clean.cpp:~120
+  claims to GC it but only locks/unlocks. A `crate run` killed without
+  teardown (SIGKILL/OOM/panic) leaves its pid in ctx-fw-users, so
+  FwUsers::isEmpty() is never true again and the shared NAT rule 50000
+  + net.inet.ip.forwarding are never restored (run_net.cpp:~488). Add
+  kill(pid,0)-based GC mirroring FwSlots and make clean.cpp call it.
+
+* (robustness, MED/LOW) daemon/privops_listener.cpp nvlist_recv and
+  daemon/control_socket.cpp read() block forever with no timeout, and
+  both accept loops spawn one detached thread per connection with no
+  cap — an idle client pins a thread + fd; N of them exhaust the
+  daemon. Add a recv timeout (SO_RCVTIMEO) and a concurrency cap.
+
+* (LOW batch) daemon/routes.cpp import tmp file is <name>.crate.tmp.<pid>
+  — constant across threads, so two concurrent imports of one name
+  interleave writes; no fsync before rename. lib/util.cpp writeFile(data,
+  fd) closes the CALLER's fd on error (ctx.cpp then double-closes).
+  execPipelineImpl: fork() failure for child i>0 throws without reaping
+  children 0..i-1 (zombies). Capture loops treat read()==-1 as EOF
+  (EINTR) and jail_query.cpp ignores waitpid failure / decodes an
+  uninitialized status. lib/util.cpp ~:631/662 test `st_mode & S_IFREG/
+  S_IFDIR` instead of S_ISREG/S_ISDIR (a socket matches S_IFDIR →
+  directory_iterator throws out of findElfFiles). daemon/routes.cpp
+  gmtime() (static buffer) in worker threads. ws_console.cpp select()
+  with fds >= FD_SETSIZE. run.cpp hcThread joinable while execInJail
+  can throw on fork EAGAIN → std::terminate skips every RunAtEnd.
+  run_services.cpp ignores fork() failure for socat. stack.cpp
+  inet_pton results unchecked for ip_range.
+
 === High priority — blocking production use ===
 
 (High-priority items above are now complete; the next batch is

cli/args.cpp

@@ -753,7 +753,7 @@ Args parseArguments(int argc, char** argv, unsigned &processed) {
         args.noColor = true;
         break;
       } else if (strEq(argv[a], "--version")) {
-        std::cout << "crate 1.1.25" << std::endl;
+        std::cout << "crate 1.1.26" << std::endl;
         exit(0);
       } else if (auto argShort = isShort(argv[a])) {
         switch (argShort) {
@@ -764,7 +764,7 @@ Args parseArguments(int argc, char** argv, unsigned &processed) {
           args.logProgress = true;
           break;
         case 'V':
-          std::cout << "crate 1.1.25" << std::endl;
+          std::cout << "crate 1.1.26" << std::endl;
           exit(0);
         default:
           err("unsupported short option '%s'", argv[a]);

daemon/control_socket.cpp

@@ -559,7 +559,11 @@ struct SocketRuntime {
 // printed but the bind continues.
 int bindSocketOrThrow(const ControlSocketPure::ControlSocketSpec &spec,
                       long expectedGid) {
-  int fd = ::socket(AF_UNIX, SOCK_STREAM, 0);
+  // 1.1.26: SOCK_CLOEXEC — this listener must not be inherited by the
+  // long-lived `crate run` children handleStart forks (they held the
+  // LISTEN fd → EADDRINUSE on `service crated restart`, and the jail
+  // supervisor carried the host daemon's control socket).
+  int fd = ::socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
   if (fd < 0)
     throw std::runtime_error(std::string("socket(AF_UNIX): ") + std::strerror(errno));
 

daemon/privops_listener.cpp

@@ -178,7 +178,9 @@ void acceptLoop(Runtime *rt) {
 #ifdef __FreeBSD__
 int openListener(const std::string &path, const std::string &group,
                  unsigned mode) {
-  int fd = ::socket(AF_UNIX, SOCK_STREAM, 0);
+  // 1.1.26: SOCK_CLOEXEC so forked children never inherit the privops
+  // listener (see control_socket.cpp bindSocketOrThrow).
+  int fd = ::socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
   if (fd < 0) {
     throw std::runtime_error(std::string("socket: ") + std::strerror(errno));
   }

daemon/routes.cpp

@@ -83,6 +83,36 @@ static std::string getClientId(const httplib::Request &req) {
   return req.remote_addr.empty() ? "tcp" : req.remote_addr;
 }
 
+// 1.1.26: start a container by spawning `crate run` as its OWN process
+// (fork + setsid + exec) — exactly what the control-socket start path
+// already does. The previous code called runCrate() IN-PROCESS on the
+// httplib worker thread, which:
+//   (a) blocked that worker for the jail's whole lifetime — a
+//       service-only container never returns, and after ~8 starts the
+//       entire HTTP API, /healthz included, stopped responding;
+//   (b) installed runCrate's SIGINT/SIGTERM handlers process-wide, so
+//       `service crated stop` hung until SIGKILL;
+//   (c) keyed jailXname / FwSlots / FwUsers on getpid(), so every
+//       daemon-started jail shared ONE key — a later start overwrote the
+//       first jail's firewall slot and the first teardown removed the
+//       shared NAT rule while other jails still ran.
+// A child process has its own pid, signal handlers and lifetime.
+// Returns the child pid, or -1 on fork failure.
+static pid_t spawnCrateRun(const std::string &crateFile) {
+  pid_t pid = ::fork();
+  if (pid < 0) return -1;
+  if (pid == 0) {
+    ::setsid();                       // no controlling tty; survives crated teardown
+    ::close(0); ::close(1); ::close(2);
+    ::open("/dev/null", O_RDONLY);
+    ::open("/dev/null", O_WRONLY);
+    ::open("/dev/null", O_WRONLY);
+    ::execl(CRATE_PATH_CRATE, "crate", "run", "-f", crateFile.c_str(), nullptr);
+    ::_exit(127);
+  }
+  return pid;
+}
+
 // Cap constants live in daemon/rate_limit.h since 0.7.15.
 // (Pre-0.8.8 this file had local constexpr aliases; removed in
 // the routes.cpp rate-limit refactor.)
@@ -149,7 +179,10 @@ static void handleHostInfo(const httplib::Response &, httplib::Response &res) {
     auto hostname = Util::getSysctlString("kern.hostname");
     auto machine = Util::getSysctlString("hw.machine");
     auto ncpu = Util::getSysctlInt("hw.ncpu");
-    auto physmem = Util::getSysctlInt("hw.physmem");
+    // 1.1.26: hw.physmem is CTLTYPE_ULONG (8 bytes on amd64/arm64);
+    // reading it through the 4-byte getSysctlInt made sysctlbyname
+    // fail with ENOMEM, so this endpoint was a permanent 500 on 64-bit.
+    auto physmem = Util::getSysctlUInt64("hw.physmem");
 
     std::ostringstream ss;
     ss << "{\"hostname\":\"" << hostname << "\""
@@ -298,19 +331,16 @@ static void handleContainerStart(const httplib::Request &req, httplib::Response
     return;
   }
 
-  try {
-    Args runArgs;
-    runArgs.cmd = CmdRun;
-    runArgs.runCrateFile = crateFile;
-    int returnCode = 0;
-    if (runCrate(runArgs, 0, nullptr, returnCode)) {
-      jsonOk(res, "{\"started\":true,\"name\":\"" + name + "\"}");
-    } else {
-      jsonError(res, 500, "failed to start container");
-    }
-  } catch (const std::exception &e) {
-    jsonError(res, 500, e.what());
+  pid_t pid = spawnCrateRun(crateFile);
+  if (pid < 0) {
+    jsonError(res, 500, "failed to start container: fork failed");
+    return;
   }
+  // The child is the jail's foreground supervisor and outlives this
+  // request: "started" now means "spawned". Poll
+  // GET /api/v1/containers/:name for the running state.
+  jsonOk(res, "{\"started\":true,\"async\":true,\"pid\":" + std::to_string(pid)
+              + ",\"name\":\"" + name + "\"}");
 }
 
 // --- F2: DELETE /api/v1/containers/:name ---
@@ -608,19 +638,13 @@ static void handleContainerRestart(const httplib::Request &req, httplib::Respons
     jsonError(res, 404, "no saved .crate file for '" + name + "'; container stopped but cannot restart");
     return;
   }
-  try {
-    Args runArgs;
-    runArgs.cmd = CmdRun;
-    runArgs.runCrateFile = crateFile;
-    int returnCode = 0;
-    if (runCrate(runArgs, 0, nullptr, returnCode)) {
-      jsonOk(res, "{\"restarted\":true,\"name\":\"" + name + "\"}");
-    } else {
-      jsonError(res, 500, "failed to start container after stop");
-    }
-  } catch (const std::exception &e) {
-    jsonError(res, 500, e.what());
+  pid_t pid = spawnCrateRun(crateFile);
+  if (pid < 0) {
+    jsonError(res, 500, "failed to start container after stop: fork failed");
+    return;
   }
+  jsonOk(res, "{\"restarted\":true,\"async\":true,\"pid\":" + std::to_string(pid)
+              + ",\"name\":\"" + name + "\"}");
 }
 
 // --- Snapshot helpers ---

daemon/server.cpp

@@ -24,7 +24,13 @@
 namespace Crated {
 
 struct Server::Impl {
-  std::unique_ptr<httplib::Server> httpSrv;
+  std::unique_ptr<httplib::Server> httpSrv;   // TCP (optionally TLS) listener
+  // 1.1.26: the Unix-socket server is a member, not a lambda-local.
+  // It used to live only inside unixThread's closure, so stop() could
+  // never call its stop(), listen() never returned, and ~Impl destroyed
+  // a still-joinable std::thread → std::terminate (SIGABRT) on every
+  // clean shutdown with the default unixSocket configured.
+  std::unique_ptr<httplib::Server> udsSrv;
   std::thread tcpThread;
   std::thread unixThread;
 };
@@ -65,13 +71,14 @@ void Server::start() {
     // Remove stale socket
     ::unlink(config_.unixSocket.c_str());
 
+    // This is the Unix-socket listener (local, root-owned socket) —
+    // isUnixListener = true, so its peers are treated as trusted.
+    // Built here (not inside the thread) so stop() can reach it.
+    impl_->udsSrv = std::make_unique<httplib::Server>();
+    registerRoutes(*impl_->udsSrv, config_, /*isUnixListener=*/true);
+    impl_->udsSrv->set_address_family(AF_UNIX);
     impl_->unixThread = std::thread([this]() {
-      httplib::Server udsSrv;
-      // This is the Unix-socket listener (local, root-owned socket) —
-      // isUnixListener = true, so its peers are treated as trusted.
-      registerRoutes(udsSrv, config_, /*isUnixListener=*/true);
-      udsSrv.set_address_family(AF_UNIX);
-      udsSrv.listen(config_.unixSocket, 0);
+      impl_->udsSrv->listen(config_.unixSocket, 0);
     });
 
     // 0.8.19: enforce post-bind filesystem perms.
@@ -157,7 +164,14 @@ void Server::stop() {
     impl_->httpSrv->stop();
   if (impl_->tcpThread.joinable())
     impl_->tcpThread.join();
-  // Unix socket thread will exit when server stops
+  // 1.1.26: stop the Unix-socket server too and JOIN its thread. The
+  // old comment "Unix socket thread will exit when server stops" was
+  // false — nothing ever stopped that server, and destroying the
+  // joinable thread aborted the process.
+  if (impl_->udsSrv)
+    impl_->udsSrv->stop();
+  if (impl_->unixThread.joinable())
+    impl_->unixThread.join();
   if (!config_.unixSocket.empty())
     ::unlink(config_.unixSocket.c_str());
 }