p9: use *at() functions for set_attr

Use fchmodat(), fchownat(), and utimensat() to implement the SET_ATTR
request rather than using the non-'at' variants of these functions.
These can operate on a file descriptor path using the /proc file handle
and "self/fd/N" filename to modify the attributes of a file without
actually opening it, which means we can avoid problems like not being
able to open a read-only file with O_RDWR, which happened previously
with chmod requests.

This means we don't need to open the file at all, except in the case of
a request that needs to set the size, since there is no equivalent
truncateat() function.

BUG=chromium:1369647
TEST=touch /mnt/chromeos/MyFiles/Downloads/hello.txt
TEST=chmod -w /mnt/chromeos/MyFiles/Downloads/hello.txt
TEST=chmod +w /mnt/chromeos/MyFiles/Downloads/hello.txt
TEST=chmod a-r /mnt/chromeos/MyFiles/Downloads/hello.txt
TEST=chmod a+r /mnt/chromeos/MyFiles/Downloads/hello.txt
TEST=chown $USER /mnt/chromeos/MyFiles/Downloads/hello.txt
TEST=truncate -s1 /mnt/chromeos/MyFiles/Downloads/hello.txt

Change-Id: I0461ed231cc78b26bcc37ede1a364af984c87f8b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3935537
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>

common/p9/src/server/mod.rs

@@ -775,21 +775,13 @@ impl Server {
 
     fn set_attr(&mut self, set_attr: &Tsetattr) -> io::Result<()> {
         let fid = self.fids.get(&set_attr.fid).ok_or_else(ebadf)?;
-
-        let file = if let Some(ref file) = fid.file {
-            MaybeOwned::Borrowed(file)
-        } else {
-            let flags = match fid.filetype {
-                FileType::Regular => P9_RDWR,
-                FileType::Directory => P9_RDONLY | P9_DIRECTORY,
-                FileType::Other => P9_RDWR,
-            };
-            MaybeOwned::Owned(open_fid(&self.proc, &fid.path, P9_NONBLOCK | flags)?)
-        };
+        let path = string_to_cstring(format!("self/fd/{}", fid.path.as_raw_fd()))?;
 
         if set_attr.valid & P9_SETATTR_MODE != 0 {
             // Safe because this doesn't modify any memory and we check the return value.
-            syscall!(unsafe { libc::fchmod(file.as_raw_fd(), set_attr.mode) })?;
+            syscall!(unsafe {
+                libc::fchmodat(self.proc.as_raw_fd(), path.as_ptr(), set_attr.mode, 0)
+            })?;
         }
 
         if set_attr.valid & (P9_SETATTR_UID | P9_SETATTR_GID) != 0 {
@@ -805,10 +797,18 @@ impl Server {
             };
 
             // Safe because this doesn't modify any memory and we check the return value.
-            syscall!(unsafe { libc::fchown(file.as_raw_fd(), uid, gid) })?;
+            syscall!(unsafe { libc::fchownat(self.proc.as_raw_fd(), path.as_ptr(), uid, gid, 0) })?;
         }
 
         if set_attr.valid & P9_SETATTR_SIZE != 0 {
+            let file = if fid.filetype == FileType::Directory {
+                return Err(io::Error::from_raw_os_error(libc::EISDIR));
+            } else if let Some(ref file) = fid.file {
+                MaybeOwned::Borrowed(file)
+            } else {
+                MaybeOwned::Owned(open_fid(&self.proc, &fid.path, P9_NONBLOCK | P9_RDWR)?)
+            };
+
             file.set_len(set_attr.size)?;
         }
 
@@ -837,7 +837,14 @@ impl Server {
             ];
 
             // Safe because file is valid and we have initialized times fully.
-            let ret = unsafe { libc::futimens(file.as_raw_fd(), &times as *const libc::timespec) };
+            let ret = unsafe {
+                libc::utimensat(
+                    self.proc.as_raw_fd(),
+                    path.as_ptr(),
+                    &times as *const libc::timespec,
+                    0,
+                )
+            };
             if ret < 0 {
                 return Err(io::Error::last_os_error());
             }
@@ -849,10 +856,12 @@ impl Server {
             // Setting -1 as the uid and gid will not actually change anything but will
             // still update the ctime.
             let ret = unsafe {
-                libc::fchown(
-                    file.as_raw_fd(),
+                libc::fchownat(
+                    self.proc.as_raw_fd(),
+                    path.as_ptr(),
                     libc::uid_t::max_value(),
                     libc::gid_t::max_value(),
+                    0,
                 )
             };
             if ret < 0 {

seccomp/aarch64/9p_device.policy

@@ -23,7 +23,9 @@ socket: arg0 == AF_UNIX
 utimensat: 1
 ftruncate: 1
 fchmod: 1
+fchmodat: 1
 fchown: 1
+fchownat: 1
 fstatfs: 1
 newfstatat: 1
 prctl: arg0 == PR_SET_NAME

seccomp/arm/9p_device.policy

@@ -26,7 +26,9 @@ utimensat: 1
 utimensat_time64: 1
 ftruncate64: 1
 fchmod: 1
+fchmodat: 1
 fchown: 1
+fchownat: 1
 fstatfs: 1
 fstatfs64: 1
 fstatat64: 1

seccomp/x86_64/9p_device.policy

@@ -25,7 +25,9 @@ fdatasync: 1
 utimensat: 1
 ftruncate: 1
 fchmod: 1
+fchmodat: 1
 fchown: 1
+fchownat: 1
 fstatfs: 1
 newfstatat: 1
 prctl: arg0 == PR_SET_NAME