A few bugfixes

src/api/server.rs

@@ -571,7 +571,7 @@ impl<F: FileSystem + Sync> Server<F> {
                 };
 
                 w.write_all(out.as_slice()).map_err(Error::EncodeMessage)?;
-                w.commit(&[&data_writer.0]).map_err(Error::IoError)?;
+                w.commit(&[&data_writer.0]).map_err(Error::EncodeMessage)?;
                 Ok(out.len as usize)
             }
             Err(e) => reply_error(e, in_header.unique, w),
@@ -976,7 +976,7 @@ impl<F: FileSystem + Sync> Server<F> {
             };
 
             w.write_all(out.as_slice()).map_err(Error::EncodeMessage)?;
-            w.commit(&[&cursor]).map_err(Error::IoError)?;
+            w.commit(&[&cursor]).map_err(Error::EncodeMessage)?;
             Ok(out.len as usize)
         }
     }
@@ -1394,10 +1394,23 @@ fn reply_error(err: io::Error, unique: u64, mut w: Writer) -> Result<usize> {
         .map_err(Error::EncodeMessage)
 }
 
-fn bytes_to_cstr(buf: &[u8]) -> Result<&CStr> {
-    // Convert to a `CStr` first so that we can drop the '\0' byte at the end
-    // and make sure there are no interior '\0' bytes.
-    CStr::from_bytes_with_nul(buf).map_err(Error::InvalidCString)
+/// trim all trailing nul terminators.
+pub fn bytes_to_cstr(buf: &[u8]) -> Result<&CStr> {
+    // There might be multiple 0s at the end of buf, find & use the first one and trim other zeros.
+    let len = buf.len();
+    let mut n: usize = 0;
+    for c in buf.iter() {
+        if c == &0 {
+            break;
+        }
+        n += 1;
+    }
+    let nul_pos = if n + 1 >= len { len } else { n + 1 };
+    let newbuf = &buf[0..nul_pos];
+
+    // Convert to a `CStr` so that we can drop the '\0' byte at the end and make sure there are no
+    // interior '\0' bytes.
+    CStr::from_bytes_with_nul(newbuf).map_err(Error::InvalidCString)
 }
 
 fn add_dirent(

src/api/vfs.rs

@@ -431,7 +431,7 @@ impl FileSystem for Vfs {
     ) -> Result<(libc::stat64, Duration)> {
         match self.get_real_rootfs(inode)? {
             (Left(fs), idata) => fs.getattr(ctx, idata.ino, handle),
-            (Right(fs), idata) => fs.getattr(ctx, idata.ino, handle.map(|h| h)),
+            (Right(fs), idata) => fs.getattr(ctx, idata.ino, handle),
         }
     }
 
@@ -445,7 +445,7 @@ impl FileSystem for Vfs {
     ) -> Result<(libc::stat64, Duration)> {
         match self.get_real_rootfs(inode)? {
             (Left(fs), idata) => fs.setattr(ctx, idata.ino, attr, handle, valid),
-            (Right(fs), idata) => fs.setattr(ctx, idata.ino, attr, handle.map(|h| h), valid),
+            (Right(fs), idata) => fs.setattr(ctx, idata.ino, attr, handle, valid),
         }
     }
 
@@ -587,7 +587,7 @@ impl FileSystem for Vfs {
                 fs.create(ctx, idata.ino, name, mode, flags, umask)
                     .map(|(mut a, b, c)| {
                         a.inode = self.hash_inode(idata.super_index, a.inode)?;
-                        Ok((a, b.map(|h| h), c))
+                        Ok((a, b, c))
                     })?
             }
         }

src/lib.rs

@@ -75,8 +75,6 @@ pub enum Error {
     /// The `size` field of the `SetxattrIn` message does not match the length
     /// of the decoded value.
     InvalidXattrSize((u32, usize)),
-    /// An IO related error has happened.
-    IoError(io::Error),
 }
 
 impl error::Error for Error {}
@@ -96,7 +94,6 @@ impl fmt::Display for Error {
                  decoded value: size = {}, value.len() = {}",
                 size, len
             ),
-            IoError(err) => write!(f, "fail to handle request: {}", err),
         }
     }
 }

src/passthrough/fs.rs

@@ -28,6 +28,7 @@ use crate::api::filesystem::{
     Context, DirEntry, Entry, FileSystem, FsOptions, GetxattrReply, ListxattrReply, OpenOptions,
     SetattrValid, ZeroCopyReader, ZeroCopyWriter,
 };
+use crate::api::server;
 use crate::api::{BackendFileSystem, VFS_MAX_INO};
 #[cfg(feature = "vhost-user-fs")]
 use crate::transport::FsCacheReqHandler;
@@ -199,7 +200,7 @@ impl FromStr for CachePolicy {
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
-            "never" | "Never" | "NEVER" => Ok(CachePolicy::Never),
+            "never" | "Never" | "NEVER" | "none" | "None" | "NONE" => Ok(CachePolicy::Never),
             "auto" | "Auto" | "AUTO" => Ok(CachePolicy::Auto),
             "always" | "Always" | "ALWAYS" => Ok(CachePolicy::Always),
             _ => Err("invalid cache policy"),
@@ -561,6 +562,7 @@ impl PassthroughFs {
         }
 
         let mut rem = &buf[..];
+        let orig_rem_len = rem.len();
         while !rem.is_empty() {
             // We only use debug asserts here because these values are coming from the kernel and we
             // trust them implicitly.
@@ -583,6 +585,20 @@ impl PassthroughFs {
                 // break the loop so return `Ok` with a non-zero value instead.
                 Ok(1)
             } else {
+                // The Sys_getdents64 in kernel will pad the name with '\0'
+                // bytes up to 8-byte alignment, so @name may contain a few null
+                // terminators.  This causes an extra lookup from fuse when
+                // called by readdirplus, because kernel path walking only takes
+                // name without null terminators, the dentry with more than 1
+                // null terminators added by readdirplus doesn't satisfy the
+                // path walking.
+                let name = server::bytes_to_cstr(name)
+                    .map_err(|e| {
+                        error!("do_readdir: {:?}", e);
+                        io::Error::from_raw_os_error(libc::EINVAL)
+                    })?
+                    .to_bytes();
+
                 add_entry(DirEntry {
                     ino: dirent64.d_ino,
                     offset: dirent64.d_off as u64,
@@ -599,7 +615,12 @@ impl PassthroughFs {
             match res {
                 Ok(0) => break,
                 Ok(_) => rem = &rem[dirent64.d_reclen as usize..],
-                Err(e) => return Err(e),
+                // If there's an error, we can only signal it if we haven't
+                // stored any entries yet - otherwise we'd end up with wrong
+                // lookup counts for the entries that are already in the
+                // buffer. So we return what we've collected until that point.
+                Err(e) if rem.len() == orig_rem_len => return Err(e),
+                Err(_) => return Ok(()),
             }
         }
 
@@ -902,13 +923,14 @@ impl FileSystem for PassthroughFs {
         add_entry: &mut dyn FnMut(DirEntry, Entry) -> io::Result<usize>,
     ) -> io::Result<()> {
         self.do_readdir(inode, handle, size, offset, &mut |dir_entry| {
-            // Safe because the kernel guarantees that the buffer is nul-terminated. Additionally,
-            // the kernel will pad the name with '\0' bytes up to 8-byte alignment and there's no
-            // way for us to know exactly how many padding bytes there are. This would cause
-            // `CStr::from_bytes_with_nul` to return an error because it would think there are
-            // interior '\0' bytes. We trust the kernel to provide us with properly formatted data
-            // so we'll just skip the checks here.
-            let name = unsafe { CStr::from_bytes_with_nul_unchecked(dir_entry.name) };
+            // Safe because do_readdir() has ensured dir_entry.name is a
+            // valid [u8] generated by CStr::to_bytes().
+            let name = unsafe {
+                CStr::from_bytes_with_nul_unchecked(std::slice::from_raw_parts(
+                    &dir_entry.name[0],
+                    dir_entry.name.len() + 1,
+                ))
+            };
             let entry = self.do_lookup(inode, name)?;
             let ino = entry.inode;
 

src/transport/fusedev/mod.rs

@@ -163,7 +163,9 @@ impl<'a> Writer<'a> {
         if !buf.is_empty() {
             return writev(self.fd, buf.as_slice()).map_err(|e| {
                 error! {"fail to write to fuse device on commit: {}", e};
-                io::Error::new(io::ErrorKind::Other, format!("{}", e))
+                e.as_errno()
+                    .map(|r| io::Error::from_raw_os_error(r as i32))
+                    .unwrap_or_else(|| io::Error::new(io::ErrorKind::Other, format!("{}", e)))
             });
         }
         Ok(0)