From e0ea4e013a52f326050f25fbdf50ccf267aa58b0 Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Fri, 19 Feb 2021 18:24:05 +0900 Subject: [PATCH] fs: Include Inode in ioctl parameters When the file system implements zero message open support, the file handle is meaningless and it needs to know the inode of the file/directory on which the ioctl was called. BUG=b:180565632 TEST=lsattr, chattr, both work when zero message open is enabled. Android's FileBasedEncryptionPolicyTest[0] gets ENOTTY as an error instead of EBADF [0]: https://android.googlesource.com/platform/cts/+/bfbc00c20d0c3f659c16b8df652155df0e7a229a/tests/tests/security/native/encryption/FileBasedEncryptionPolicyTest.cpp Change-Id: Ic55ee95df928d645874dd8a9c7dc579b708927fa Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2706370 Tested-by: Chirantan Ekbote Tested-by: kokoro Auto-Submit: Chirantan Ekbote Commit-Queue: Stephen Barber Reviewed-by: Stephen Barber Reviewed-by: kokoro --- devices/src/virtio/fs/passthrough.rs | 95 ++++++++++++++-------------- fuse/src/filesystem.rs | 1 + fuse/src/server.rs | 1 + 3 files changed, 48 insertions(+), 49 deletions(-) diff --git a/devices/src/virtio/fs/passthrough.rs b/devices/src/virtio/fs/passthrough.rs index 925bc34b8c..2bf915085b 100644 --- a/devices/src/virtio/fs/passthrough.rs +++ b/devices/src/virtio/fs/passthrough.rs @@ -885,15 +885,15 @@ impl PassthroughFs { fn get_encryption_policy_ex( &self, + inode: Inode, handle: Handle, mut r: R, ) -> io::Result { - let data = self - .handles - .lock() - .get(&handle) - .map(Arc::clone) - .ok_or_else(ebadf)?; + let data: Arc = if self.zero_message_open.load(Ordering::Relaxed) { + self.find_inode(inode)? + } else { + self.find_handle(handle, inode)? + }; // Safe because this only has integer fields. let mut arg = unsafe { MaybeUninit::::zeroed().assume_init() }; @@ -902,10 +902,9 @@ impl PassthroughFs { let policy_size = cmp::min(arg.policy_size, size_of::() as u64); arg.policy_size = policy_size; - let file = data.file.lock(); // Safe because the kernel will only write to `arg` and we check the return value. let res = - unsafe { ioctl_with_mut_ptr(&*file, FS_IOC_GET_ENCRYPTION_POLICY_EX(), &mut arg) }; + unsafe { ioctl_with_mut_ptr(&*data, FS_IOC_GET_ENCRYPTION_POLICY_EX(), &mut arg) }; if res < 0 { Ok(IoctlReply::Done(Err(io::Error::last_os_error()))) } else { @@ -914,19 +913,17 @@ impl PassthroughFs { } } - fn get_fsxattr(&self, handle: Handle) -> io::Result { - let data = self - .handles - .lock() - .get(&handle) - .map(Arc::clone) - .ok_or_else(ebadf)?; + fn get_fsxattr(&self, inode: Inode, handle: Handle) -> io::Result { + let data: Arc = if self.zero_message_open.load(Ordering::Relaxed) { + self.find_inode(inode)? + } else { + self.find_handle(handle, inode)? + }; let mut buf = MaybeUninit::::zeroed(); - let file = data.file.lock(); // Safe because the kernel will only write to `buf` and we check the return value. - let res = unsafe { ioctl_with_mut_ptr(&*file, FS_IOC_FSGETXATTR(), buf.as_mut_ptr()) }; + let res = unsafe { ioctl_with_mut_ptr(&*data, FS_IOC_FSGETXATTR(), buf.as_mut_ptr()) }; if res < 0 { Ok(IoctlReply::Done(Err(io::Error::last_os_error()))) } else { @@ -936,19 +933,22 @@ impl PassthroughFs { } } - fn set_fsxattr(&self, handle: Handle, r: R) -> io::Result { - let data = self - .handles - .lock() - .get(&handle) - .map(Arc::clone) - .ok_or_else(ebadf)?; + fn set_fsxattr( + &self, + inode: Inode, + handle: Handle, + r: R, + ) -> io::Result { + let data: Arc = if self.zero_message_open.load(Ordering::Relaxed) { + self.find_inode(inode)? + } else { + self.find_handle(handle, inode)? + }; let attr = fsxattr::from_reader(r)?; - let file = data.file.lock(); // Safe because this doesn't modify any memory and we check the return value. - let res = unsafe { ioctl_with_ptr(&*file, FS_IOC_FSSETXATTR(), &attr) }; + let res = unsafe { ioctl_with_ptr(&*data, FS_IOC_FSSETXATTR(), &attr) }; if res < 0 { Ok(IoctlReply::Done(Err(io::Error::last_os_error()))) } else { @@ -956,20 +956,18 @@ impl PassthroughFs { } } - fn get_flags(&self, handle: Handle) -> io::Result { - let data = self - .handles - .lock() - .get(&handle) - .map(Arc::clone) - .ok_or_else(ebadf)?; + fn get_flags(&self, inode: Inode, handle: Handle) -> io::Result { + let data: Arc = if self.zero_message_open.load(Ordering::Relaxed) { + self.find_inode(inode)? + } else { + self.find_handle(handle, inode)? + }; // The ioctl encoding is a long but the parameter is actually an int. let mut flags: c_int = 0; - let file = data.file.lock(); // Safe because the kernel will only write to `flags` and we check the return value. - let res = unsafe { ioctl_with_mut_ptr(&*file, FS_IOC_GETFLAGS(), &mut flags) }; + let res = unsafe { ioctl_with_mut_ptr(&*data, FS_IOC_GETFLAGS(), &mut flags) }; if res < 0 { Ok(IoctlReply::Done(Err(io::Error::last_os_error()))) } else { @@ -977,20 +975,18 @@ impl PassthroughFs { } } - fn set_flags(&self, handle: Handle, r: R) -> io::Result { - let data = self - .handles - .lock() - .get(&handle) - .map(Arc::clone) - .ok_or_else(ebadf)?; + fn set_flags(&self, inode: Inode, handle: Handle, r: R) -> io::Result { + let data: Arc = if self.zero_message_open.load(Ordering::Relaxed) { + self.find_inode(inode)? + } else { + self.find_handle(handle, inode)? + }; // The ioctl encoding is a long but the parameter is actually an int. let flags = c_int::from_reader(r)?; - let file = data.file.lock(); // Safe because this doesn't modify any memory and we check the return value. - let res = unsafe { ioctl_with_ptr(&*file, FS_IOC_SETFLAGS(), &flags) }; + let res = unsafe { ioctl_with_ptr(&*data, FS_IOC_SETFLAGS(), &flags) }; if res < 0 { Ok(IoctlReply::Done(Err(io::Error::last_os_error()))) } else { @@ -2203,6 +2199,7 @@ impl FileSystem for PassthroughFs { fn ioctl( &self, _ctx: Context, + inode: Inode, handle: Handle, _flags: IoctlFlags, cmd: u32, @@ -2220,33 +2217,33 @@ impl FileSystem for PassthroughFs { const SET_FLAGS64: u32 = FS_IOC64_SETFLAGS() as u32; match cmd { - GET_ENCRYPTION_POLICY_EX => self.get_encryption_policy_ex(handle, r), + GET_ENCRYPTION_POLICY_EX => self.get_encryption_policy_ex(inode, handle, r), GET_FSXATTR => { if out_size < size_of::() as u32 { Err(io::Error::from_raw_os_error(libc::ENOMEM)) } else { - self.get_fsxattr(handle) + self.get_fsxattr(inode, handle) } } SET_FSXATTR => { if in_size < size_of::() as u32 { Err(io::Error::from_raw_os_error(libc::EINVAL)) } else { - self.set_fsxattr(handle, r) + self.set_fsxattr(inode, handle, r) } } GET_FLAGS32 | GET_FLAGS64 => { if out_size < size_of::() as u32 { Err(io::Error::from_raw_os_error(libc::ENOMEM)) } else { - self.get_flags(handle) + self.get_flags(inode, handle) } } SET_FLAGS32 | SET_FLAGS64 => { if in_size < size_of::() as u32 { Err(io::Error::from_raw_os_error(libc::ENOMEM)) } else { - self.set_flags(handle, r) + self.set_flags(inode, handle, r) } } _ => Err(io::Error::from_raw_os_error(libc::ENOTTY)), diff --git a/fuse/src/filesystem.rs b/fuse/src/filesystem.rs index c465ca6324..8050309ff4 100644 --- a/fuse/src/filesystem.rs +++ b/fuse/src/filesystem.rs @@ -1081,6 +1081,7 @@ pub trait FileSystem { fn ioctl( &self, ctx: Context, + inode: Self::Inode, handle: Self::Handle, flags: IoctlFlags, cmd: u32, diff --git a/fuse/src/server.rs b/fuse/src/server.rs index bfd0c0d97b..0c62896b65 100644 --- a/fuse/src/server.rs +++ b/fuse/src/server.rs @@ -1329,6 +1329,7 @@ impl Server { let res = self.fs.ioctl( in_header.into(), + in_header.nodeid.into(), fh.into(), IoctlFlags::from_bits_truncate(flags), cmd,