summary refs log tree commit diff
diff options
context:
space:
mode:
authorChirantan Ekbote <chirantan@chromium.org>2020-03-26 16:20:40 +0900
committerCommit Bot <commit-bot@chromium.org>2020-03-27 00:44:06 +0000
commitd74bb77a3eb17acc8878f2459ba70d9cf58a731b (patch)
treefc9e9f1f3ac49faeaaeb2dee16023c26af17b923
parent9e8aa131d3e9884ff1244dc300aeb40879c7ceb5 (diff)
downloadcrosvm-d74bb77a3eb17acc8878f2459ba70d9cf58a731b.tar
crosvm-d74bb77a3eb17acc8878f2459ba70d9cf58a731b.tar.gz
crosvm-d74bb77a3eb17acc8878f2459ba70d9cf58a731b.tar.bz2
crosvm-d74bb77a3eb17acc8878f2459ba70d9cf58a731b.tar.lz
crosvm-d74bb77a3eb17acc8878f2459ba70d9cf58a731b.tar.xz
crosvm-d74bb77a3eb17acc8878f2459ba70d9cf58a731b.tar.zst
crosvm-d74bb77a3eb17acc8878f2459ba70d9cf58a731b.zip
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 <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
-rw-r--r--devices/src/virtio/fs/passthrough.rs62
-rw-r--r--seccomp/aarch64/fs_device.policy6
-rw-r--r--seccomp/arm/fs_device.policy6
-rw-r--r--seccomp/x86_64/fs_device.policy6
4 files changed, 55 insertions, 25 deletions
diff --git a/devices/src/virtio/fs/passthrough.rs b/devices/src/virtio/fs/passthrough.rs
index 6706272..4e28e59 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<CString> {
+        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<File> {
         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<GetxattrReply> {
-        // 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<ListxattrReply> {
-        // 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 ec9d155..7bf794a 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 4078f41..661883a 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 eb5a1c4..1c10601 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