Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
62 changes: 33 additions & 29 deletions src/syscall/fs-stat.c
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,20 @@ static bool statfs_path_is_proc(const char *path)
return !strncmp(path, "/proc", 5) && (path[5] == '\0' || path[5] == '/');
}

static void fill_proc_statfs(linux_statfs_t *lin)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The linux_statfs_t layout here is complete and correct (all fields covered by memset(0); f_type/f_bsize/f_namelen/f_frsize consistent with a real procfs). No regression test asserts the magic, though -- the existing tests only compare statfs vs fstatfs, not the value 0x9fa0. Worth adding one assertion for f_type == 0x9fa0 on both statfs("/proc/...") and fstatfs(open("/proc/...")).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fix: Added test assertions verifying that st.f_type == 0x9fa0 (representing PROC_SUPER_MAGIC) for /proc, /proc/ (trailing slash), and /proc/self/cmdline under both statfs and fstatfs.

{
memset(lin, 0, sizeof(*lin));
lin->f_type = 0x9fa0; /* PROC_SUPER_MAGIC */
lin->f_bsize = 4096;
lin->f_blocks = 0;
lin->f_bfree = 0;
lin->f_bavail = 0;
lin->f_files = 0;
lin->f_ffree = 0;
lin->f_namelen = 255;
lin->f_frsize = 4096;
}

int64_t sys_statfs(guest_t *g, uint64_t path_gva, uint64_t buf_gva)
{
char path[LINUX_PATH_MAX];
Expand All @@ -379,38 +393,18 @@ int64_t sys_statfs(guest_t *g, uint64_t path_gva, uint64_t buf_gva)
if (tx.fuse_path)
return -LINUX_ENOSYS;

struct statfs mac_st;

/* /proc has no host-filesystem counterpart under the sysroot: every entry
* is synthesized on open (see proc_intercept_open), so a raw statfs() on
* the guest path always misses with ENOENT even for "/proc" itself. Try
* the lightweight statfs-specific resolver first (it shortcuts nodes whose
* open path would otherwise allocate scratch state statfs never uses),
* then fall back to the same intercept open/fstatfs/close that
* sys_newfstatat uses via proc_intercept_stat.
*/
if (statfs_path_is_proc(tx.intercept_path)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

statfs on a nonexistent /proc path now returns success instead of ENOENT. statfs_path_is_proc() matches any /proc-prefixed path, so this block fills PROC_SUPER_MAGIC and returns 0 without confirming the entry exists. The previous code fell through proc_intercept_statfs -> proc_intercept_open -> statfs(host_path), so a miss propagated ENOENT.

Trigger: statfs("/proc/bogus", &buf) or statfs("/proc/self/not_a_file", &buf) -- real Linux and pre-PR return -ENOENT; this returns 0 with f_type=0x9fa0. Programs that use statfs as an existence/feature probe are misled.

Fix: gate synthesis on a positive existence check (only synthesize when proc_intercept_stat(tx.intercept_path, ...) recognizes the path, otherwise fall through to the host statfs that yields ENOENT). Only f_type needed to change; existence semantics should be preserved.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fix: I modified sys_statfs to execute a positive existence check first:

  1. Check via proc_intercept_stat if it is a synthetic /proc node.
  2. If proc_intercept_stat returns PROC_NOT_INTERCEPTED, check if the path exists on the host's sysroot mapping via stat(tx.host_path).
  3. Only synthesize PROC_SUPER_MAGIC statfs if either check confirms the file/directory exists; otherwise, fall through to let host statfs yield ENOENT.

int intercepted = proc_intercept_statfs(tx.intercept_path, &mac_st);
if (intercepted == PROC_NOT_INTERCEPTED) {
int host_fd = proc_intercept_open(g, tx.intercept_path, 0, 0);
if (host_fd == PROC_NOT_INTERCEPTED) {
if (statfs(tx.host_path, &mac_st) < 0)
return linux_errno();
} else if (host_fd < 0) {
return linux_errno();
} else {
int rc = fstatfs(host_fd, &mac_st);
close_keep_errno(host_fd);
if (rc < 0)
return linux_errno();
}
} else if (intercepted < 0) {
return linux_errno();
}
} else if (statfs(tx.host_path, &mac_st) < 0) {
return linux_errno();
linux_statfs_t lin_st;
fill_proc_statfs(&lin_st);
if (guest_write_small(g, buf_gva, &lin_st, sizeof(lin_st)) < 0)
return -LINUX_EFAULT;
return 0;
}

struct statfs mac_st;
if (statfs(tx.host_path, &mac_st) < 0)
return linux_errno();

linux_statfs_t lin_st;
translate_statfs(&mac_st, &lin_st);
if (guest_write_small(g, buf_gva, &lin_st, sizeof(lin_st)) < 0)
Expand All @@ -421,6 +415,16 @@ int64_t sys_statfs(guest_t *g, uint64_t path_gva, uint64_t buf_gva)

int64_t sys_fstatfs(guest_t *g, int fd, uint64_t buf_gva)
{
fd_entry_t snap;
memset(&snap, 0, sizeof(snap));
if (fd_snapshot(fd, &snap) && statfs_path_is_proc(snap.proc_path)) {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
linux_statfs_t proc_st;
fill_proc_statfs(&proc_st);
if (guest_write_small(g, buf_gva, &proc_st, sizeof(proc_st)) < 0)
return -LINUX_EFAULT;
return 0;
}

host_fd_ref_t host_ref;
if (host_fd_ref_open(fd, &host_ref) < 0)
return -LINUX_EBADF;
Expand Down
66 changes: 62 additions & 4 deletions src/syscall/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,43 @@ static const char *proc_stateful_file_path(const char *path)
return NULL;
}

static bool proc_path_is_symlink(const char *path)
{
if (!path)
return false;

if (!strcmp(path, "/proc/self/exe") || !strcmp(path, "/proc/self/cwd") ||
!strcmp(path, "/proc/self/root")) {
return true;
}

if (!strncmp(path, "/proc/self/fd/", 14)) {
char *endp;
long n = strtol(path + 14, &endp, 10);
if (endp != path + 14 && *endp == '\0' && n >= 0)
return true;
}

if (!strncmp(path, "/proc/self/task/", 16)) {
char *endp;
strtol(path + 16, &endp, 10);
if (endp != path + 16 && *endp == '/') {
const char *sub = endp + 1;
if (!strcmp(sub, "exe") || !strcmp(sub, "cwd") ||
!strcmp(sub, "root")) {
return true;
}
if (!strncmp(sub, "fd/", 3)) {
long n = strtol(sub + 3, &endp, 10);
if (endp != sub + 3 && *endp == '\0' && n >= 0)
return true;
}
}
}

return false;
}

/* Resolve the proc_path the fd table should record for an intercepted path.
* Returns true and fills *out when a mapping exists; false otherwise so the
* caller can skip the install entirely. Pure string work; safe to call before
Expand All @@ -149,11 +186,32 @@ static bool resolve_virtual_path(const char *path, char *out, size_t out_size)
const char *virt = proc_virtual_dir_path(path, virt_buf, sizeof(virt_buf));
if (!virt)
virt = proc_stateful_file_path(path);
if (!virt)
return false;

str_copy_trunc(out, virt, out_size);
return true;
if (virt) {
str_copy_trunc(out, virt, out_size);
return true;
}

/* If it has a valid /proc prefix, normalize it and record it. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Stamping proc_path for every non-symlink /proc/* entry -- including regular files like /proc/cpuinfo and /proc/self/maps -- leaks dirfd semantics into consumers that key off proc_path. dirfd_guest_base_path (path.c:883) accepts any fd with a non-empty proc_path as a resolution base and returns before the snap.type != FD_DIR -> ENOTDIR guard at path.c:909 -- unlike resolve_proc_dirfd_path (path.c:522), which requires FD_DIR.

Trigger: open /proc/cpuinfo as fd 5, then openat(5, "x", ...). Real Linux returns ENOTDIR; this builds /proc/cpuinfo/x and resolves it (wrong errno / wrong target). It also reroutes plain fstat on such fds through proc_intercept_stat.

Fix: only stamp proc_path for /proc entries that are directories (the cases where the fd is a legitimate dirfd), or record a separate proc-mount flag used solely by the statfs path rather than overloading proc_path, which carries dirfd-resolution meaning.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fix: I updated dirfd_guest_base_path to assert that the file descriptor is of type FD_DIR. If a regular emulated file descriptor under /proc (e.g. /proc/cpuinfo) is passed as a dirfd, it now correctly throws ENOTDIR and returns -1 (matching real Linux). This resolves the leaking dirfd semantics while allowing fstat and emulated read/write slow paths to continue referencing proc_path for regular files.

if (path[5] == '\0' || path[5] == '/') {
if (strncmp(path, "/proc/", 6) == 0) {
char *endp;
long pid = strtol(path + 6, &endp, 10);
if (endp != path + 6 && pid == (long) proc_get_pid() &&
(*endp == '\0' || *endp == '/')) {
snprintf(out, out_size, "/proc/self%s", endp);
if (proc_path_is_symlink(out))
return false;
return true;
}
}
if (proc_path_is_symlink(path))
return false;
str_copy_trunc(out, path, out_size);
return true;
}

return false;
}

static const char *proc_virtual_dir_path(const char *path,
Expand Down
Loading