From d74bb77a3eb17acc8878f2459ba70d9cf58a731b Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Thu, 26 Mar 2020 16:20:40 +0900 Subject: [PATCH] devices: fs: Use l{get,set,list,remove}xattr Using the `open_inode` method on an fd for a symlink results in the kernel returning -ELOOP. Since there are no `*at` methods for extended attributes, manually read the path for the file and then use the l{get,set,list,remove}xattr method on the returned path. BUG=b:136128512 TEST=boot arcvm with virtio-fs and selinux enabled Change-Id: I2fde57db8a075838a3a877309f6cf89059f19258 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2120763 Auto-Submit: Chirantan Ekbote Tested-by: kokoro Commit-Queue: Stephen Barber Reviewed-by: Stephen Barber --- devices/src/virtio/fs/passthrough.rs | 62 +++++++++++++++++++--------- seccomp/aarch64/fs_device.policy | 6 ++- seccomp/arm/fs_device.policy | 6 ++- seccomp/x86_64/fs_device.policy | 6 ++- 4 files changed, 55 insertions(+), 25 deletions(-) diff --git a/devices/src/virtio/fs/passthrough.rs b/devices/src/virtio/fs/passthrough.rs index 6706272346..4e28e59c04 100644 --- a/devices/src/virtio/fs/passthrough.rs +++ b/devices/src/virtio/fs/passthrough.rs @@ -356,6 +356,38 @@ impl PassthroughFs { vec![self.proc.as_raw_fd()] } + fn get_path(&self, inode: Inode) -> io::Result { + let data = self + .inodes + .read() + .unwrap() + .get(&inode) + .map(Arc::clone) + .ok_or_else(ebadf)?; + + let mut buf = Vec::with_capacity(libc::PATH_MAX as usize); + buf.resize(libc::PATH_MAX as usize, 0); + + let path = CString::new(format!("self/fd/{}", data.file.as_raw_fd())) + .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?; + + // Safe because this will only modify the contents of `buf` and we check the return value. + let res = unsafe { + libc::readlinkat( + self.proc.as_raw_fd(), + path.as_ptr(), + buf.as_mut_ptr() as *mut libc::c_char, + buf.len(), + ) + }; + if res < 0 { + return Err(io::Error::last_os_error()); + } + + buf.resize(res as usize, 0); + CString::new(buf).map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e)) + } + fn open_inode(&self, inode: Inode, mut flags: i32) -> io::Result { let data = self .inodes @@ -1521,14 +1553,12 @@ impl FileSystem for PassthroughFs { value: &[u8], flags: u32, ) -> io::Result<()> { - // The f{set,get,remove,list}xattr functions don't work on an fd opened with `O_PATH` so we - // need to get a new fd. - let file = self.open_inode(inode, libc::O_RDONLY | libc::O_NONBLOCK)?; + let path = self.get_path(inode)?; // Safe because this doesn't modify any memory and we check the return value. let res = unsafe { - libc::fsetxattr( - file.as_raw_fd(), + libc::lsetxattr( + path.as_ptr(), name.as_ptr(), value.as_ptr() as *const libc::c_void, value.len(), @@ -1549,17 +1579,15 @@ impl FileSystem for PassthroughFs { name: &CStr, size: u32, ) -> io::Result { - // The f{set,get,remove,list}xattr functions don't work on an fd opened with `O_PATH` so we - // need to get a new fd. - let file = self.open_inode(inode, libc::O_RDONLY | libc::O_NONBLOCK)?; + let path = self.get_path(inode)?; let mut buf = Vec::with_capacity(size as usize); buf.resize(size as usize, 0); // Safe because this will only modify the contents of `buf`. let res = unsafe { - libc::fgetxattr( - file.as_raw_fd(), + libc::lgetxattr( + path.as_ptr(), name.as_ptr(), buf.as_mut_ptr() as *mut libc::c_void, size as libc::size_t, @@ -1578,17 +1606,15 @@ impl FileSystem for PassthroughFs { } fn listxattr(&self, _ctx: Context, inode: Inode, size: u32) -> io::Result { - // The f{set,get,remove,list}xattr functions don't work on an fd opened with `O_PATH` so we - // need to get a new fd. - let file = self.open_inode(inode, libc::O_RDONLY | libc::O_NONBLOCK)?; + let path = self.get_path(inode)?; let mut buf = Vec::with_capacity(size as usize); buf.resize(size as usize, 0); // Safe because this will only modify the contents of `buf`. let res = unsafe { - libc::flistxattr( - file.as_raw_fd(), + libc::llistxattr( + path.as_ptr(), buf.as_mut_ptr() as *mut libc::c_char, size as libc::size_t, ) @@ -1606,12 +1632,10 @@ impl FileSystem for PassthroughFs { } fn removexattr(&self, _ctx: Context, inode: Inode, name: &CStr) -> io::Result<()> { - // The f{set,get,remove,list}xattr functions don't work on an fd opened with `O_PATH` so we - // need to get a new fd. - let file = self.open_inode(inode, libc::O_RDONLY | libc::O_NONBLOCK)?; + let path = self.get_path(inode)?; // Safe because this doesn't modify any memory and we check the return value. - let res = unsafe { libc::fremovexattr(file.as_raw_fd(), name.as_ptr()) }; + let res = unsafe { libc::lremovexattr(path.as_ptr(), name.as_ptr()) }; if res == 0 { Ok(()) diff --git a/seccomp/aarch64/fs_device.policy b/seccomp/aarch64/fs_device.policy index ec9d1556e4..7bf794a8fc 100644 --- a/seccomp/aarch64/fs_device.policy +++ b/seccomp/aarch64/fs_device.policy @@ -9,8 +9,10 @@ fallocate: 1 fchmodat: 1 fchownat: 1 fdatasync: 1 -fgetxattr: 1 -fsetxattr: 1 +lgetxattr: 1 +lsetxattr: 1 +llistxattr: 1 +lremovexattr: 1 fsync: 1 newfstatat: 1 fstatfs: 1 diff --git a/seccomp/arm/fs_device.policy b/seccomp/arm/fs_device.policy index 4078f418a9..661883a805 100644 --- a/seccomp/arm/fs_device.policy +++ b/seccomp/arm/fs_device.policy @@ -9,8 +9,10 @@ fallocate: 1 fchmodat: 1 fchownat: 1 fdatasync: 1 -fgetxattr: 1 -fsetxattr: 1 +lgetxattr: 1 +lsetxattr: 1 +llistxattr: 1 +lremovexattr: 1 fstatat64: 1 fstatfs64: 1 fsync: 1 diff --git a/seccomp/x86_64/fs_device.policy b/seccomp/x86_64/fs_device.policy index eb5a1c4912..1c10601571 100644 --- a/seccomp/x86_64/fs_device.policy +++ b/seccomp/x86_64/fs_device.policy @@ -9,8 +9,10 @@ fallocate: 1 fchmodat: 1 fchownat: 1 fdatasync: 1 -fgetxattr: 1 -fsetxattr: 1 +lgetxattr: 1 +lsetxattr: 1 +llistxattr: 1 +lremovexattr: 1 fstatfs: 1 fsync: 1 ftruncate: 1