diff --git a/CLAUDE.md b/CLAUDE.md index 293bf6e..1a9dbe0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -422,9 +422,21 @@ Source layout (all under `src/`): `/proc//ns/{pid,uts,ipc,cgroup}` against this process's own showed them identical, while only `mnt`/`user` differed. `resolve_namespace_pid()` reads `/proc//task//children` (the direct-children list `procfs` - exposes) to find that real inner process and joins *its* namespaces instead — - falls back to `pid` itself (best-effort, not fatal) if that file can't be - read. For each of `{mnt→--mount, uts→--uts, ipc→--ipc, pid→--pid, + exposes) to find that real inner process and joins *its* namespaces instead. + **Real bug found via testing on a real target device, not assumed**: that + file requires `CONFIG_CHECKPOINT_RESTORE`, which not every kernel enables — + confirmed absent (not just unreadable — the file doesn't exist at all) on a + real Android device, where `-e/--exec` then fell back to the outer `bwrap` + pid itself and failed outright (`nsenter: no namespace specified`, since + every namespace type either matched the outer process's own or couldn't be + read at all). Fixed by adding `find_child_by_scanning_proc()`, a portable + fallback used only when the children file is missing/empty: scans + `/proc//stat` for any process whose ppid field equals `pid` — the same + information `pstree` itself reads to build its tree, which is how the actual + sandboxed child was located and confirmed correct on the same device via a + manual `nsenter -t -a -- /bin/sh` before the fix was written. + Picks the lowest matching pid if more than one child exists, for a + deterministic result. For each of `{mnt→--mount, uts→--uts, ipc→--ipc, pid→--pid, cgroup→--cgroup, user→--user}` (`net` deliberately excluded — this project never isolates networking, see `bwrap.cpp` below), `readlink()`s both `/proc//ns/` and `/proc/self/ns/` and only passes diff --git a/src/exec_session.cpp b/src/exec_session.cpp index a415704..ed99c38 100644 --- a/src/exec_session.cpp +++ b/src/exec_session.cpp @@ -18,9 +18,11 @@ #include #include +#include #include #include #include +#include #include #include @@ -31,6 +33,64 @@ namespace { +// Fast path: /proc//task//children lists direct children with no +// scanning needed. Requires CONFIG_CHECKPOINT_RESTORE, which not every kernel +// enables -- absent (not just unreadable), confirmed by direct testing on a +// real Android target, where the file simply doesn't exist. +std::optional find_child_via_children_file(pid_t pid) { + std::ifstream children(fmt::format("/proc/{}/task/{}/children", pid, pid)); + pid_t child = 0; + if (children >> child && child > 0) { + return child; + } + return std::nullopt; +} + +// Portable fallback for kernels without the children file: scans /proc//stat +// for any process whose ppid field (the first whitespace-separated field after +// comm's closing ')' -- comm itself is parenthesized and may contain spaces or +// parens, so it can't just be split on whitespace) equals `pid`. This is the +// same information `pstree` itself reads to build its tree -- confirmed by +// direct testing: `pstree -p ` found the real sandboxed child on a +// device where the children file was missing. Picks the lowest matching pid if +// more than one child exists, for a deterministic result. +std::optional find_child_by_scanning_proc(pid_t pid) { + std::error_code ec; + std::optional found; + for (const auto& entry : std::filesystem::directory_iterator("/proc", ec)) { + if (ec) { + break; + } + const std::string name = entry.path().filename().string(); + if (name.empty() || !std::all_of(name.begin(), name.end(), + [](unsigned char c) { return std::isdigit(c) != 0; })) { + continue; + } + + std::ifstream stat_file(entry.path() / "stat"); + std::string line; + if (!std::getline(stat_file, line)) { + continue; + } + auto close_paren = line.rfind(')'); + if (close_paren == std::string::npos) { + continue; + } + std::istringstream rest(line.substr(close_paren + 1)); + std::string state; + pid_t ppid = 0; + if (!(rest >> state >> ppid) || ppid != pid) { + continue; + } + + pid_t candidate = std::stoi(name); + if (!found || candidate < *found) { + found = candidate; + } + } + return found; +} + // bwrap's own outer process (the one tracked in the session pid file) sets up the // mount/user namespaces itself, then clone()s the actual sandboxed command into // fresh pid/uts/ipc/cgroup/net namespaces -- clone()'s namespace-creation flags @@ -43,10 +103,13 @@ namespace { // child can't be determined -- callers still get a mount/user-namespace join out // of that, just not the rest. pid_t resolve_namespace_pid(pid_t pid) { - std::ifstream children(fmt::format("/proc/{}/task/{}/children", pid, pid)); - pid_t child = 0; - if (children >> child && child > 0) { - return child; + if (auto child = find_child_via_children_file(pid)) { + return *child; + } + if (auto child = find_child_by_scanning_proc(pid)) { + spdlog::debug("pid {}: found sandboxed child {} by scanning /proc (no .../task/.../children file)", + pid, *child); + return *child; } spdlog::debug("could not determine pid {}'s sandboxed child process; joining its own namespaces only", pid);