Support AT_EMPTY_PATH in fchownat/fchmodat2#194
Conversation
fd10b66 to
63b7930
Compare
jserv
left a comment
There was a problem hiding this comment.
Append Close #128 at the end of git commit messages.
63b7930 to
ca1fea1
Compare
fchownat() and fchmodat2() only accepted AT_SYMLINK_NOFOLLOW, so *at(fd, "", ..., AT_EMPTY_PATH) returned EINVAL instead of operating on fd itself. That is the only way to chown or chmod an O_PATH descriptor (plain fchown()/fchmod() reject FD_PATH with EBADF, matching Linux), so unlike the plain EBADF guard this flag combination needs real handling, not just acceptance. An empty path with AT_EMPTY_PATH now operates on dirfd directly, or the current directory when dirfd is AT_FDCWD, mirroring the identical AT_EMPTY_PATH branch already in fs-stat.c's stat_at_path. Close sysprog21#128
ca1fea1 to
153a72d
Compare
jserv
left a comment
There was a problem hiding this comment.
Automated review of the AT_EMPTY_PATH handling. The common thread: the new empty-path branch returns before the normal path's reject_unsupported_fuse_path_op() / interception layer, so it treats some fd classes differently from every other *at op. errno preservation, translate_at_flags dropping AT_EMPTY_PATH, the O_PATH handling, sysroot parity, leaks, and the test-matrix count math all check out.
| /* AT_EMPTY_PATH with an empty path chmods dirfd itself; see the identical | ||
| * branch in sys_fchownat above for the O_PATH/AT_FDCWD rationale. | ||
| */ | ||
| if ((flags & LINUX_AT_EMPTY_PATH) && path[0] == '\0') { |
There was a problem hiding this comment.
Empty-path branch bypasses FUSE interception (mirror at :2519 in fchownat). It returns before path_translate_at + reject_unsupported_fuse_path_op, so it diverges from the named path in two ways:
- A
dirfdinside a FUSE mount hashost_fd == -1(fuse.c:1893), sohost_dirfd_ref_openreturns EBADF here, whereas the named path returns ENOSYS. - If cwd is inside a FUSE mount, the AT_FDCWD sub-case calls host
fchmodat(".", ...)directly, mutating the host backing path instead of returning ENOSYS.
Suggested fix: snapshot dirfd first and return -LINUX_ENOSYS for FD_FUSE_* to match reject_unsupported_fuse_path_op.
| } | ||
|
|
||
| host_fd_ref_t ref; | ||
| if (host_dirfd_ref_open(dirfd, &ref) < 0) |
There was a problem hiding this comment.
Proc-intercept divergence from the mirrored stat_at_path (mirror at :2533 in fchownat). stat_at_path checks snap.type == FD_PATH && snap.proc_path[0] and routes to proc_intercept_stat; this branch skips that and fchmod/fchowns the backing host temp fd directly.
A guest that opens a synthesized /proc path O_PATH then calls fchownat(fd, "", ..., AT_EMPTY_PATH) silently mutates elfuse's internal temp file and reports success (Linux would return EPERM). chown_result's EPERM-to-fakeroot-success mapping hides it further.
Suggested fix: snapshot the fd before host_dirfd_ref_open; for FD_PATH with proc_path set, return EPERM instead of touching the backing file.
| if ((flags & LINUX_AT_EMPTY_PATH) && path[0] == '\0') { | ||
| int mac_flags = translate_at_flags(flags); | ||
| if (dirfd == LINUX_AT_FDCWD) { | ||
| int host_rc = fchownat(AT_FDCWD, ".", owner, group, mac_flags); |
There was a problem hiding this comment.
TOCTOU on the AT_FDCWD overlay update. fchownat(AT_FDCWD, ".") here and the follow-up fstatat(AT_FDCWD, ".") at :2526 resolve "." twice. A concurrent chdir() in another guest thread between them records the requested uid/gid overlay against a different directory's dev/ino. The dirfd sub-case is immune because it fchown+fstat the same fd.
Suggested fix: open "." once and fchown(fd)+fstat(fd) on that single descriptor, like the dirfd branch.
|
|
||
| static int fchmodat2(int dirfd, const char *path, mode_t mode, int flags) | ||
| { | ||
| return (int) syscall(SYS_fchmodat2, dirfd, path, (long) mode, (long) flags); |
There was a problem hiding this comment.
No ENOSYS skip guard for fchmodat2. This issues raw fchmodat2(2) (SYS 452, Linux 6.6+) with no fallback. On a qemu fixture running a <6.6 kernel it fails hard rather than skipping. Worth confirming the fixture kernel is >= 6.6 (the count bump to 217 assumes it).
fchownat() and fchmodat2() only accepted AT_SYMLINK_NOFOLLOW, so
*at(fd, "", ..., AT_EMPTY_PATH) returned EINVAL instead of operating on
fd itself. That is the only way to chown or chmod an O_PATH descriptor
(plain fchown()/fchmod() reject FD_PATH with EBADF, matching Linux), so
unlike the plain EBADF guard this flag combination needs real handling,
not just acceptance.
An empty path with AT_EMPTY_PATH now operates on dirfd directly, or the
current directory when dirfd is AT_FDCWD, mirroring the identical
AT_EMPTY_PATH branch already in fs-stat.c's stat_at_path.
Close #128
Summary by cubic
Adds proper AT_EMPTY_PATH handling to fchownat and fchmodat2 so an empty path acts on the dirfd (or the cwd for AT_FDCWD) instead of returning EINVAL. This enables chown/chmod on O_PATH fds and matches Linux and our stat_at_path behavior.
Written for commit ca1fea1. Summary will update on new commits.