diff --git a/src/devices/src/virtio/fs/macos/passthrough.rs b/src/devices/src/virtio/fs/macos/passthrough.rs index aa5c464..ab0fa73 100644 --- a/src/devices/src/virtio/fs/macos/passthrough.rs +++ b/src/devices/src/virtio/fs/macos/passthrough.rs @@ -266,12 +266,13 @@ fn is_valid_owner(owner: Option<(u32, u32)>) -> bool { // We won't need this once expressions like "if let ... &&" are allowed. #[allow(clippy::unnecessary_unwrap)] fn set_xattr_stat( + ctx: &Context, file: &InodeHandle, st: Option, owner: Option<(u32, u32)>, mode: Option, ) -> io::Result<()> { - let st = st.unwrap_or(istat(file, true)?); + let st = st.unwrap_or(istat(ctx, file, true)?); let options = if (st.st_mode & libc::S_IFMT) == libc::S_IFLNK { libc::XATTR_NOFOLLOW } else { @@ -349,32 +350,30 @@ fn set_xattr_stat( } fn stat_common( + _ctx: &Context, mut st: bindings::stat64, uid: Option, gid: Option, mode: Option, - host: bool, ) -> io::Result { - if !host { - if let Some(uid) = uid { - st.st_uid = uid; - } - if let Some(gid) = gid { - st.st_gid = gid; - } - if let Some(mode) = mode { - if mode as u16 & libc::S_IFMT == 0 { - st.st_mode = (st.st_mode & libc::S_IFMT) | mode as u16; - } else { - st.st_mode = mode as u16; - } + if let Some(uid) = uid { + st.st_uid = uid; + } + if let Some(gid) = gid { + st.st_gid = gid; + } + if let Some(mode) = mode { + if mode as u16 & libc::S_IFMT == 0 { + st.st_mode = (st.st_mode & libc::S_IFMT) | mode as u16; + } else { + st.st_mode = mode as u16; } } Ok(st) } -fn fstat(fd: RawFd, host: bool) -> io::Result { +fn fstat(ctx: &Context, fd: RawFd, host: bool) -> io::Result { let mut st = MaybeUninit::::zeroed(); // Safe because the kernel will only write data in `st` and we check the return @@ -385,7 +384,7 @@ fn fstat(fd: RawFd, host: bool) -> io::Result { let st = unsafe { st.assume_init() }; if !host { let (uid, gid, mode) = get_xattr_fstat(fd, st)?; - stat_common(st, uid, gid, mode, host) + stat_common(ctx, st, uid, gid, mode) } else { Ok(st) } @@ -394,7 +393,7 @@ fn fstat(fd: RawFd, host: bool) -> io::Result { } } -fn lstat(c_path: &CString, host: bool) -> io::Result { +fn lstat(ctx: &Context, c_path: &CString, host: bool) -> io::Result { let mut st = MaybeUninit::::zeroed(); // Safe because the kernel will only write data in `st` and we check the return @@ -405,7 +404,7 @@ fn lstat(c_path: &CString, host: bool) -> io::Result { let st = unsafe { st.assume_init() }; if !host { let (uid, gid, mode) = get_xattr_lstat(c_path, st)?; - stat_common(st, uid, gid, mode, host) + stat_common(ctx, st, uid, gid, mode) } else { Ok(st) } @@ -414,10 +413,10 @@ fn lstat(c_path: &CString, host: bool) -> io::Result { } } -fn istat(ihandle: &InodeHandle, host: bool) -> io::Result { +fn istat(ctx: &Context, ihandle: &InodeHandle, host: bool) -> io::Result { match ihandle { - InodeHandle::Fd(fd) => fstat(*fd, host), - InodeHandle::Path(c_path) => lstat(c_path, host), + InodeHandle::Fd(fd) => fstat(ctx, *fd, host), + InodeHandle::Path(c_path) => lstat(ctx, c_path, host), } } @@ -751,6 +750,7 @@ impl PassthroughFs { fn do_open( &self, + ctx: &Context, inode: Inode, kill_priv: bool, flags: u32, @@ -766,10 +766,10 @@ impl PassthroughFs { remove_security_capability(&ihandle); - if let Ok(st) = fstat(fd, false) { + if let Ok(st) = fstat(ctx, fd, false) { let new_mode = clear_suid_sgid(st.st_mode as u32); if new_mode != st.st_mode as u32 - && let Err(err) = set_xattr_stat(&ihandle, Some(st), None, Some(new_mode)) + && let Err(err) = set_xattr_stat(ctx, &ihandle, Some(st), None, Some(new_mode)) { error!("Couldn't clear suid/sgid for inode {inode}: {err}"); } @@ -817,11 +817,11 @@ impl PassthroughFs { Err(ebadf()) } - fn do_getattr(&self, inode: Inode) -> io::Result<(bindings::stat64, Duration)> { + fn do_getattr(&self, ctx: &Context, inode: Inode) -> io::Result<(bindings::stat64, Duration)> { let ihandle = self.inode_to_handle(inode, true)?; let st = match ihandle { - InodeHandle::Path(c_path) => lstat(&c_path, false)?, - InodeHandle::Fd(fd) => fstat(fd, false)?, + InodeHandle::Path(c_path) => lstat(ctx, &c_path, false)?, + InodeHandle::Fd(fd) => fstat(ctx, fd, false)?, }; Ok((st, self.cfg.attr_timeout)) @@ -836,8 +836,8 @@ impl PassthroughFs { Ok(fd) } - fn store_unlinked_fd(&self, unlinked_fd: RawFd) -> io::Result { - let st = fstat(unlinked_fd, true)?; + fn store_unlinked_fd(&self, ctx: &Context, unlinked_fd: RawFd) -> io::Result { + let st = fstat(ctx, unlinked_fd, true)?; let altkey = InodeAltKey { ino: st.st_ino, dev: st.st_dev, @@ -865,7 +865,7 @@ impl PassthroughFs { fn do_unlink( &self, - _ctx: Context, + ctx: Context, parent: Inode, name: &CStr, flags: libc::c_int, @@ -909,7 +909,7 @@ impl PassthroughFs { if res == 0 { if let Some(unlinked_fd) = unlinked_fd { - match self.store_unlinked_fd(unlinked_fd) { + match self.store_unlinked_fd(&ctx, unlinked_fd) { // The tracked inode took ownership of the fd. Ok(true) => {} // No tracked inode: we still own the fd and must close it. @@ -1093,7 +1093,14 @@ impl FileSystem for PassthroughFs { // Safe because we just opened this fd above. let f = unsafe { File::from_raw_fd(fd) }; - let st = fstat(f.as_raw_fd(), true)?; + // Build a fake Context for fstat, it won't be using it anyways + // as it'll be only looking at the host's bits. + let ctx = Context { + uid: 0, + gid: 0, + pid: 0, + }; + let st = fstat(&ctx, f.as_raw_fd(), true)?; // Safe because this doesn't modify any memory and there is no need to check the return // value because this system call always succeeds. We need to clear the umask here because @@ -1154,7 +1161,7 @@ impl FileSystem for PassthroughFs { } } - fn lookup(&self, _ctx: Context, parent: Inode, name: &CStr) -> io::Result { + fn lookup(&self, ctx: Context, parent: Inode, name: &CStr) -> io::Result { let parent_data = self .inodes .read() @@ -1164,7 +1171,7 @@ impl FileSystem for PassthroughFs { .ok_or_else(ebadf)?; let c_path = self.name_to_path(parent, name)?; - let st = lstat(&c_path, false)?; + let st = lstat(&ctx, &c_path, false)?; debug!( "lookup: inode={} path={}", @@ -1240,11 +1247,11 @@ impl FileSystem for PassthroughFs { fn opendir( &self, - _ctx: Context, + ctx: Context, inode: Inode, flags: u32, ) -> io::Result<(Option, OpenOptions)> { - self.do_open(inode, false, flags | libc::O_DIRECTORY as u32) + self.do_open(&ctx, inode, false, flags | libc::O_DIRECTORY as u32) } fn releasedir( @@ -1278,6 +1285,7 @@ impl FileSystem for PassthroughFs { }; set_xattr_stat( + &ctx, &ihandle, None, Some((ctx.uid, ctx.gid)), @@ -1336,12 +1344,12 @@ impl FileSystem for PassthroughFs { fn open( &self, - _ctx: Context, + ctx: Context, inode: Inode, kill_priv: bool, flags: u32, ) -> io::Result<(Option, OpenOptions)> { - self.do_open(inode, kill_priv, flags) + self.do_open(&ctx, inode, kill_priv, flags) } fn release( @@ -1393,6 +1401,7 @@ impl FileSystem for PassthroughFs { let ihandle = InodeHandle::Fd(fd); if let Err(e) = set_xattr_stat( + &ctx, &ihandle, None, Some((ctx.uid, ctx.gid)), @@ -1471,7 +1480,7 @@ impl FileSystem for PassthroughFs { fn write( &self, - _ctx: Context, + ctx: Context, inode: Inode, handle: Handle, mut r: R, @@ -1503,11 +1512,12 @@ impl FileSystem for PassthroughFs { remove_security_capability(&ihandle); - if let Ok(st) = fstat(fd, false) { + if let Ok(st) = fstat(&ctx, fd, false) { let new_mode = clear_suid_sgid(st.st_mode as u32); if new_mode != st.st_mode as u32 { // Update mode in xattr - if let Err(err) = set_xattr_stat(&ihandle, Some(st), None, Some(new_mode)) { + if let Err(err) = set_xattr_stat(&ctx, &ihandle, Some(st), None, Some(new_mode)) + { error!("Couldn't clear suid/sgid for inode {inode}: {err}"); } } @@ -1519,16 +1529,16 @@ impl FileSystem for PassthroughFs { fn getattr( &self, - _ctx: Context, + ctx: Context, inode: Inode, _handle: Option, ) -> io::Result<(bindings::stat64, Duration)> { - self.do_getattr(inode) + self.do_getattr(&ctx, inode) } fn setattr( &self, - _ctx: Context, + ctx: Context, inode: Inode, attr: bindings::stat64, handle: Option, @@ -1552,7 +1562,7 @@ impl FileSystem for PassthroughFs { }; if valid.contains(SetattrValid::MODE) { - set_xattr_stat(&ihandle, None, None, Some(attr.st_mode as u32))? + set_xattr_stat(&ctx, &ihandle, None, None, Some(attr.st_mode as u32))? } if valid.intersects(SetattrValid::UID | SetattrValid::GID) { @@ -1570,7 +1580,7 @@ impl FileSystem for PassthroughFs { }; remove_security_capability(&ihandle); - let st = istat(&ihandle, false)?; + let st = istat(&ctx, &ihandle, false)?; // Clear suid/sgid if UID or GID is being changed let new_mode = clear_suid_sgid(st.st_mode as u32); @@ -1579,7 +1589,7 @@ impl FileSystem for PassthroughFs { } else { None }; - set_xattr_stat(&ihandle, Some(st), Some((uid, gid)), new_mode)?; + set_xattr_stat(&ctx, &ihandle, Some(st), Some((uid, gid)), new_mode)?; } if valid.contains(SetattrValid::SIZE) { @@ -1593,10 +1603,10 @@ impl FileSystem for PassthroughFs { // Clear security.capability on truncate unconditionally remove_security_capability(&ihandle); - let st = fstat(fd, false)?; + let st = fstat(&ctx, fd, false)?; let new_mode = clear_suid_sgid(st.st_mode as u32); if new_mode != st.st_mode as u32 { - set_xattr_stat(&ihandle, Some(st), None, Some(new_mode))?; + set_xattr_stat(&ctx, &ihandle, Some(st), None, Some(new_mode))?; } } InodeHandle::Path(_) => { @@ -1613,10 +1623,10 @@ impl FileSystem for PassthroughFs { // reuse the FD we just opened, thus reducing the number of syscalls. let ihandle = InodeHandle::Fd(f.as_raw_fd()); remove_security_capability(&ihandle); - let st = istat(&ihandle, false)?; + let st = istat(&ctx, &ihandle, false)?; let new_mode = clear_suid_sgid(st.st_mode as u32); if new_mode != st.st_mode as u32 { - set_xattr_stat(&ihandle, Some(st), None, Some(new_mode))?; + set_xattr_stat(&ctx, &ihandle, Some(st), None, Some(new_mode))?; } } }; @@ -1663,7 +1673,7 @@ impl FileSystem for PassthroughFs { } } - self.do_getattr(inode) + self.do_getattr(&ctx, inode) } fn rename( @@ -1736,7 +1746,7 @@ impl FileSystem for PassthroughFs { // `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) { + match self.store_unlinked_fd(&ctx, fd) { Ok(true) => {} Ok(false) | Err(_) => unsafe { libc::close(fd); @@ -1754,6 +1764,7 @@ impl FileSystem for PassthroughFs { }; if fd > 0 { if let Err(e) = set_xattr_stat( + &ctx, &InodeHandle::Fd(fd), None, None, @@ -1809,6 +1820,7 @@ impl FileSystem for PassthroughFs { }; if let Err(e) = set_xattr_stat( + &ctx, &ihandle, None, Some((ctx.uid, ctx.gid)), @@ -1867,7 +1879,13 @@ impl FileSystem for PassthroughFs { let mut entry = self.lookup(ctx, parent, name)?; let mode = libc::S_IFLNK | 0o777; - set_xattr_stat(&ihandle, None, Some((ctx.uid, ctx.gid)), Some(mode as u32))?; + set_xattr_stat( + &ctx, + &ihandle, + None, + Some((ctx.uid, ctx.gid)), + Some(mode as u32), + )?; entry.attr.st_uid = ctx.uid; entry.attr.st_gid = ctx.gid; entry.attr.st_mode = mode; @@ -1973,8 +1991,8 @@ impl FileSystem for PassthroughFs { fn access(&self, ctx: Context, inode: Inode, mask: u32) -> io::Result<()> { let st = match self.inode_to_handle(inode, true)? { - InodeHandle::Path(c_path) => lstat(&c_path, false)?, - InodeHandle::Fd(fd) => fstat(fd, false)?, + InodeHandle::Path(c_path) => lstat(&ctx, &c_path, false)?, + InodeHandle::Fd(fd) => fstat(&ctx, fd, false)?, }; let mode = mask as i32 & (libc::R_OK | libc::W_OK | libc::X_OK); @@ -2242,7 +2260,7 @@ impl FileSystem for PassthroughFs { fn fallocate( &self, - _ctx: Context, + ctx: Context, inode: Inode, handle: Handle, mode: u32, @@ -2280,7 +2298,7 @@ impl FileSystem for PassthroughFs { // The best thing we can do here is extend the file to (offset + length). // This doesn't adhere to the same semantics, but should work fine (albeit // less performant) for most guest applications. - let st = fstat(fd, true)?; + let st = fstat(&ctx, fd, true)?; let new_length = (offset + length) as i64; if keep_size {