From 5842e6c7223327767a18c5ecd07b4b8533fb58d1 Mon Sep 17 00:00:00 2001 From: Alex NOVAC Date: Fri, 29 May 2026 15:47:22 +0300 Subject: [PATCH] fs/macos: preserve the inode a rename overwrites The macOS virtio-fs passthrough addresses inodes by their volfs path ("/.vol/{dev}/{ino}"), which only resolves while the inode still has a directory entry. A rename that REPLACES an existing target drops that target's last link, but unlike do_unlink -- which stashes an fd to the doomed inode in InodeData.unlinked_fd -- rename never preserved the overwritten inode. Any FUSE op the guest later issues on it then resolves a dangling volfs path and returns ENOENT. This breaks the common atomic write-new-then-rename-over pattern when the guest still holds the old target open: apt/dpkg rewrite /var/lib/dpkg/status this way while apt holds it open, so apt's final close reports "Problem closing the file /var/lib/dpkg/status - close (2: No such file or directory)" even though the install succeeded. It reproduces on any overlayfs upperdir backed by this server. Mirror do_unlink: grab an fd to the target before renamex_np (replace case only -- not RENAME_SWAP/RENAME_EXCL) and hand it to the displaced inode on success. store_unlinked_fd now reports whether a tracked inode took ownership so the caller closes the fd otherwise instead of leaking it. Signed-off-by: Alex NOVAC --- .../src/virtio/fs/macos/passthrough.rs | 90 +++++++++++++++++-- 1 file changed, 81 insertions(+), 9 deletions(-) diff --git a/src/devices/src/virtio/fs/macos/passthrough.rs b/src/devices/src/virtio/fs/macos/passthrough.rs index e95a93c95..27e1a5aa5 100644 --- a/src/devices/src/virtio/fs/macos/passthrough.rs +++ b/src/devices/src/virtio/fs/macos/passthrough.rs @@ -826,17 +826,27 @@ impl PassthroughFs { Ok(fd) } - fn store_unlinked_fd(&self, unlinked_fd: RawFd) -> io::Result<()> { + fn store_unlinked_fd(&self, unlinked_fd: RawFd) -> io::Result { let st = fstat(unlinked_fd, true)?; let altkey = InodeAltKey { ino: st.st_ino, dev: st.st_dev, }; if let Some(data) = self.inodes.read().unwrap().get_alt(&altkey).cloned() { - data.unlinked_fd - .store(unlinked_fd as i64, Ordering::Release); + // Swap rather than store so that if this inode already had a + // preserved fd (e.g. another hard link was unlinked/overwritten + // earlier), we recover and close it instead of leaking it. + let old_fd = data.unlinked_fd.swap(unlinked_fd as i64, Ordering::AcqRel); + if old_fd >= 0 { + unsafe { libc::close(old_fd as RawFd) }; + } + // The tracked inode now owns `unlinked_fd` (closed in `forget_one`). + Ok(true) + } else { + // No tracked inode for this (dev, ino): the caller keeps ownership + // of `unlinked_fd` and must close it to avoid a leak. + Ok(false) } - Ok(()) } fn do_unlink( @@ -884,11 +894,19 @@ impl PassthroughFs { } if res == 0 { - if let Some(unlinked_fd) = unlinked_fd - && let Err(err) = self.store_unlinked_fd(unlinked_fd) - { - unsafe { libc::close(unlinked_fd) }; - warn!("Couldn't store unlinked fd \"{}\": {err}", unlinked_fd); + if let Some(unlinked_fd) = unlinked_fd { + match self.store_unlinked_fd(unlinked_fd) { + // The tracked inode took ownership of the fd. + Ok(true) => {} + // No tracked inode: we still own the fd and must close it. + Ok(false) => unsafe { + libc::close(unlinked_fd); + }, + Err(err) => { + unsafe { libc::close(unlinked_fd) }; + warn!("Couldn't store unlinked fd \"{}\": {err}", unlinked_fd); + } + } } Ok(()) } else { @@ -1660,8 +1678,58 @@ impl FileSystem for PassthroughFs { let old_cpath = self.name_to_path(olddir, oldname)?; let new_cpath = self.name_to_path(newdir, newname)?; + // macOS addresses inodes by their volfs path ("/.vol/{dev}/{ino}"), + // which only resolves while the inode still has a directory entry. A + // rename that REPLACES an existing target drops that target's last + // link, so any inode the guest still holds open there would afterwards + // resolve to a dangling volfs path and fail path-based ops + // (getattr/open/setattr/...) with ENOENT (e.g. apt/dpkg's atomic + // rewrite of /var/lib/dpkg/status, surfaced as + // "close (2: No such file or directory)"). `do_unlink` already guards + // the unlink case by stashing an fd to the doomed inode in + // `InodeData.unlinked_fd`; mirror that for the overwritten target. Grab + // it *before* the rename, while its entry still exists. RENAME_SWAP + // keeps both inodes linked and RENAME_EXCL never overwrites, so skip + // those; best-effort otherwise (a non-overwriting rename finds nothing). + let doomed_fd = if (flags as i32) + & (bindings::LINUX_RENAME_EXCHANGE | bindings::LINUX_RENAME_NOREPLACE) + == 0 + { + match self.inode_to_handle(newdir, true) { + Ok(InodeHandle::Path(parent_cpath)) => { + let parent_fd = unsafe { + libc::open(parent_cpath.as_ptr(), libc::O_NOFOLLOW | libc::O_CLOEXEC) + }; + if parent_fd < 0 { + None + } else { + let grabbed = self.grab_unlinked_fd(parent_fd, newname).ok(); + unsafe { libc::close(parent_fd) }; + grabbed + } + } + Ok(InodeHandle::Fd(parent_fd)) => self.grab_unlinked_fd(parent_fd, newname).ok(), + Err(_) => None, + } + } else { + None + }; + let res = unsafe { libc::renamex_np(old_cpath.as_ptr(), new_cpath.as_ptr(), mflags) }; if res == 0 { + // If the rename overwrote a tracked inode, hand its preserved fd to + // the inode store so later ops resolve by fd, not the vanished path. + // `store_unlinked_fd` takes ownership only when that inode is + // tracked; close the fd ourselves otherwise so it is never leaked. + if let Some(fd) = doomed_fd { + match self.store_unlinked_fd(fd) { + Ok(true) => {} + Ok(false) | Err(_) => unsafe { + libc::close(fd); + }, + } + } + if ((flags as i32) & bindings::LINUX_RENAME_WHITEOUT) != 0 { let fd = unsafe { libc::open( @@ -1689,6 +1757,10 @@ impl FileSystem for PassthroughFs { Ok(()) } else { + if let Some(fd) = doomed_fd { + // The rename failed; nothing was overwritten. Drop the fd. + unsafe { libc::close(fd) }; + } Err(linux_error(io::Error::last_os_error())) } }