Skip to content

fix(fs): a chrooted task resolves and reports paths against its own root #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lollipopkit merged 2 commits into from
Aug 21, 2026
Merged

fix(fs): a chrooted task resolves and reports paths against its own root #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e857383
fix(fs): a chrooted task resolves and reports paths against its own root
lollipopkit Aug 21, 2026
ccff02a
fix(fs): the task root is a floor, an identity, and alive while it is…
lollipopkit Aug 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 63 additions & 10 deletions fs/path.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@

struct path_cache_entry {
char input_path[MAX_PATH]; // Original path (with at_path prefix if any)
// The task root the entry was resolved under. Part of the identity because
// an absolute symlink target resolves against it, so the same input can
// normalize two ways — and a thread can chroot itself, so "one cache per
// thread" does not make the root constant for the life of an entry.
char root_path[MAX_PATH];
char normalized[MAX_PATH]; // Normalized result
uint64_t timestamp; // nanosecond timestamp
int flags; // N_SYMLINK_FOLLOW or N_SYMLINK_NOFOLLOW
Expand Down Expand Up @@ -51,7 +56,7 @@ static void path_cache_init(void) {

// Try to get cached normalized path
// Returns 0 on cache hit, -1 on cache miss
static int path_cache_get(const char *full_path, int flags, char *out) {
static int path_cache_get(const char *full_path, const char *root_path, int flags, char *out) {
path_cache_init();

uint32_t hash = path_hash(full_path);
Expand All @@ -62,6 +67,9 @@ static int path_cache_get(const char *full_path, int flags, char *out) {
if (!entry->valid)
return -1;

if (strcmp(entry->root_path, root_path) != 0)
return -1;

// Check path and flags match
if (strcmp(entry->input_path, full_path) != 0)
return -1;
Expand All @@ -82,7 +90,7 @@ static int path_cache_get(const char *full_path, int flags, char *out) {
}

// Store normalized path in cache
static void path_cache_set(const char *full_path, int flags, const char *normalized) {
static void path_cache_set(const char *full_path, const char *root_path, int flags, const char *normalized) {
path_cache_init();

uint32_t hash = path_hash(full_path);
Expand All @@ -93,6 +101,9 @@ static void path_cache_set(const char *full_path, int flags, const char *normali
strncpy(entry->input_path, full_path, MAX_PATH - 1);
entry->input_path[MAX_PATH - 1] = '\0';

strncpy(entry->root_path, root_path, MAX_PATH - 1);
entry->root_path[MAX_PATH - 1] = '\0';

strncpy(entry->normalized, normalized, MAX_PATH - 1);
entry->normalized[MAX_PATH - 1] = '\0';

Expand All @@ -101,7 +112,11 @@ static void path_cache_set(const char *full_path, int flags, const char *normali
entry->valid = true;
}

static int __path_normalize(const char *at_path, const char *path, char *out, int flags, int levels) {
/// [root_path] is the machine-absolute path of the task's own root, empty when
/// that is the machine's own. It exists for one case: a symlink whose target is
/// absolute is absolute *inside the guest*, so it has to be resolved against
/// that root and not against the machine's — see the restart below.
static int __path_normalize(const char *at_path, const char *root_path, const char *path, char *out, int flags, int levels) {
// you must choose one
if (flags & N_SYMLINK_FOLLOW)
assert(!(flags & N_SYMLINK_NOFOLLOW));
Expand All @@ -116,6 +131,16 @@ static int __path_normalize(const char *at_path, const char *path, char *out, in
if (strcmp(path, "") == 0)
return _ENOENT;

// How much of `out` the task's own root occupies. `..` may climb out of a
// working directory but not out of this — otherwise `/../etc` from a task
// rooted at `/jail` normalizes to `/etc`, and the root is not a root.
//
// `root_path` is "/" for a task rooted at the machine's own, which is every
// task until something chroots; the floor is then 0 and nothing changes.
size_t root_floor = 0;
if (root_path != NULL && strcmp(root_path, "/") != 0)
root_floor = strlen(root_path);

if (at_path != NULL && strcmp(at_path, "/") != 0) {
// [T-ish-pathnorm-overflow] The base path must leave room for at least
// "/x" plus the terminator, or the component loop below writes past
Expand Down Expand Up @@ -147,7 +172,7 @@ static int __path_normalize(const char *at_path, const char *path, char *out, in
continue;
} else if (p[1] == '.' && (p[2] == '\0' || p[2] == '/')) {
// double dot path component, delete the last component
if (o != out) {
if (o != out && (size_t)(o - out) > root_floor) {
do {
o--;
n++;
Expand Down Expand Up @@ -201,16 +226,27 @@ static int __path_normalize(const char *at_path, const char *path, char *out, in
return _ELOOP;
// readlink does not null terminate
c[res] = '\0';
// if we should restart from the root, copy down
if (*c == '/')
// An absolute target restarts from a root — and the root it
// means is the *task's*, the way a chrooted process on Linux
// resolves one. Restarting from the machine's walked out of the
// filesystem the task can see: with a task rooted at a subtree,
// Alpine's `/bin/sh -> /bin/busybox` resolved to a `/bin` that
// is not the guest's, and every busybox applet link with it.
//
// Identical when the task's root is the machine's, since
// [root_path] is then empty and prefixing it adds nothing.
const char *restart_at = NULL;
if (*c == '/') {
memmove(out, c, strlen(c) + 1);
restart_at = root_path;
}
char *expanded_path = possible_symlink;
strcpy(expanded_path, out);
if (strcmp(p, "") != 0) {
strcat(expanded_path, "/");
strcat(expanded_path, p);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
return __path_normalize(NULL, expanded_path, out, flags, levels + 1);
return __path_normalize(restart_at, root_path, expanded_path, out, flags, levels + 1);
}

// if there's a slash after this component, ensure that if it
Expand Down Expand Up @@ -259,6 +295,23 @@ int path_normalize(struct fd *at, const char *path, char *out, int flags) {
assert(path_is_normalized(at_path));
}

// Where an absolute symlink target starts from, and the floor `..` cannot
// climb past. "/" for a task rooted at the machine's own, which is every
// task until something chroots.
//
// Read under the lock rather than snapshotting the fd and reading after:
// `sys_chroot` closes the old root, so the pointer can be freed the moment
// the lock is dropped. `sys_getcwd` holds it across the same call.
char root_path[MAX_PATH] = "/";
lock(&current->fs->lock);
int root_err = current->fs->root != NULL
? generic_getpath(current->fs->root, root_path)
: 0;
unlock(&current->fs->lock);
if (root_err < 0)
return root_err;
assert(path_is_normalized(root_path));

// Build full input path for cache lookup
char full_input[MAX_PATH];
if (at != NULL && strcmp(at_path, "/") != 0) {
Expand All @@ -269,17 +322,17 @@ int path_normalize(struct fd *at, const char *path, char *out, int flags) {
}

// Try cache lookup first
if (path_cache_get(full_input, flags, out) == 0) {
if (path_cache_get(full_input, root_path, flags, out) == 0) {
// Cache hit - fast return
return 0;
}

// Cache miss - do full normalization
int result = __path_normalize(at != NULL ? at_path : NULL, path, out, flags, 0);
int result = __path_normalize(at != NULL ? at_path : NULL, root_path, path, out, flags, 0);

// Store result in cache (even on error, we cache the error)
if (result == 0) {
path_cache_set(full_input, flags, out);
path_cache_set(full_input, root_path, flags, out);
}

return result;
Expand Down
26 changes: 26 additions & 0 deletions kernel/fs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,10 +815,36 @@ dword_t sys_getcwd(addr_t buf_addr, dword_t size) {
struct fd *wd = current->fs->pwd;
char pwd[MAX_PATH + 1];
int err = generic_getpath(wd, pwd);
char root[MAX_PATH + 1] = "";
if (err >= 0 && current->fs->root != NULL)
err = generic_getpath(current->fs->root, root);
unlock(&current->fs->lock);
if (err < 0)
return err;

// What a task sees is relative to its own root, the way it is for a
// chrooted process on Linux: `generic_getpath` answers in the machine's
// terms, and a task rooted at a subtree would otherwise be told its working
// directory is a path it cannot name — `/jail` for what is its own `/`.
//
// Two things this must not do. `generic_getpath` answers "/" for a task
// rooted at the machine's own — it repairs an empty result to that — so
// stripping unconditionally would take the leading slash off every path a
// normal task reports and turn `/tmp` into `tmp`. And the prefix has to end
// on a component boundary, or a cwd of `/jail-old` under a root of `/jail`
// would be reported as `-old`.
//
// A cwd outside the root is left as it is: it cannot be spelled from inside,
// but a machine-absolute path is at least true, and there is no reachable
// way to produce one — `fs_chdir` only stores what this task resolved.
size_t root_len = strlen(root);

@winnowl winnowl Bot Aug 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Correctness | 🟠 High

🧩 Analysis
  • Change relation: introduced
  • Confirmation: independently-verified
  • Reachable: ✅
🤖 Prompt for AI agents 
In kernel/fs.c, address this finding:
getcwd() drops the leading slash for every machine-rooted process whose cwd is not `/`. `fs->root` is initialized to a real fd for `/`, so `generic_getpath(root, root)` produces `"/"`, `root_len` is 1, and the unconditional prefix removal turns `/tmp` into `tmp` (only `/` is repaired back to `/`). This violates the absolute-path invariant and breaks callers such as `getcwd` consumers that expect `/tmp`; it is false only if the root fd's getpath implementation returns an empty string for `/`, contrary to `generic_getpath`'s explicit empty-to-`/` repair.

if (root_len > 1 && strncmp(pwd, root, root_len) == 0 &&
(pwd[root_len] == '/' || pwd[root_len] == '\0')) {
memmove(pwd, pwd + root_len, strlen(pwd) - root_len + 1);
if (pwd[0] == '\0')
strcpy(pwd, "/");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

if (strlen(pwd) + 1 > size)
return _ERANGE;
size = strlen(pwd) + 1;
Expand Down
Loading