diff options
Diffstat (limited to 'devices/src/virtio/fs')
| -rw-r--r-- | devices/src/virtio/fs/filesystem.rs | 4 | ||||
| -rw-r--r-- | devices/src/virtio/fs/fuse.rs | 1 | ||||
| -rw-r--r-- | devices/src/virtio/fs/passthrough.rs | 110 | ||||
| -rw-r--r-- | devices/src/virtio/fs/server.rs | 13 |
4 files changed, 91 insertions, 37 deletions
diff --git a/devices/src/virtio/fs/filesystem.rs b/devices/src/virtio/fs/filesystem.rs index eb9726c..474a278 100644 --- a/devices/src/virtio/fs/filesystem.rs +++ b/devices/src/virtio/fs/filesystem.rs @@ -9,8 +9,6 @@ use std::io; use std::mem; use std::time::Duration; -use libc; - use crate::virtio::fs::fuse; pub use fuse::{FsOptions, IoctlFlags, IoctlIovec, OpenOptions, SetattrValid, ROOT_ID}; @@ -77,7 +75,7 @@ pub struct DirEntry<'a> { /// The name of this directory entry. There are no requirements for the contents of this field /// and any sequence of bytes is considered valid. - pub name: &'a [u8], + pub name: &'a CStr, } /// A reply to a `getxattr` method call. diff --git a/devices/src/virtio/fs/fuse.rs b/devices/src/virtio/fs/fuse.rs index 5c531cf..981596b 100644 --- a/devices/src/virtio/fs/fuse.rs +++ b/devices/src/virtio/fs/fuse.rs @@ -7,7 +7,6 @@ use std::mem; use bitflags::bitflags; use data_model::DataInit; use enumn::N; -use libc; /// Version number of this interface. pub const KERNEL_VERSION: u32 = 7; diff --git a/devices/src/virtio/fs/passthrough.rs b/devices/src/virtio/fs/passthrough.rs index d2034ba..bcc7c5d 100644 --- a/devices/src/virtio/fs/passthrough.rs +++ b/devices/src/virtio/fs/passthrough.rs @@ -15,7 +15,6 @@ use std::sync::Arc; use std::time::Duration; use data_model::DataInit; -use libc; use sync::Mutex; use sys_util::{error, ioctl_ior_nr, ioctl_iow_nr, ioctl_with_mut_ptr, ioctl_with_ptr}; @@ -26,8 +25,6 @@ use crate::virtio::fs::filesystem::{ use crate::virtio::fs::fuse; use crate::virtio::fs::multikey::MultikeyBTreeMap; -const CURRENT_DIR_CSTR: &[u8] = b".\0"; -const PARENT_DIR_CSTR: &[u8] = b"..\0"; const EMPTY_CSTR: &[u8] = b"\0"; const ROOT_CSTR: &[u8] = b"/\0"; const PROC_CSTR: &[u8] = b"/proc\0"; @@ -199,6 +196,20 @@ fn stat(f: &File) -> io::Result<libc::stat64> { } } +// Like `CStr::from_bytes_with_nul` but strips any bytes after the first '\0'-byte. Panics if `b` +// doesn't contain any '\0' bytes. +fn strip_padding(b: &[u8]) -> &CStr { + // It would be nice if we could use memchr here but that's locked behind an unstable gate. + let pos = b + .iter() + .position(|&c| c == 0) + .expect("`b` doesn't contain any nul bytes"); + + // Safe because we are creating this string with the first nul-byte we found so we can + // guarantee that it is nul-terminated and doesn't contain any interior nuls. + unsafe { CStr::from_bytes_with_nul_unchecked(&b[..pos + 1]) } +} + /// The caching policy that the file system should report to the FUSE client. By default the FUSE /// protocol uses close-to-open consistency. This means that any cached contents of the file are /// invalidated the next time that file is opened. @@ -576,19 +587,15 @@ impl PassthroughFs { let namelen = dirent64.d_reclen as usize - size_of::<LinuxDirent64>(); debug_assert!(namelen <= back.len(), "back is smaller than `namelen`"); - let name = &back[..namelen]; - let res = if name.starts_with(CURRENT_DIR_CSTR) || name.starts_with(PARENT_DIR_CSTR) { - // We don't want to report the "." and ".." entries. However, returning `Ok(0)` will - // break the loop so return `Ok` with a non-zero value instead. - Ok(1) - } else { - add_entry(DirEntry { - ino: dirent64.d_ino, - offset: dirent64.d_off as u64, - type_: dirent64.d_ty as u32, - name, - }) - }; + // The kernel will pad the name with additional nul bytes until it is 8-byte aligned so + // we need to strip those off here. + let name = strip_padding(&back[..namelen]); + let res = add_entry(DirEntry { + ino: dirent64.d_ino, + offset: dirent64.d_off as u64, + type_: dirent64.d_ty as u32, + name, + }); debug_assert!( rem.len() >= dirent64.d_reclen as usize, @@ -806,7 +813,8 @@ impl FileSystem for PassthroughFs { }), ); - let mut opts = FsOptions::DO_READDIRPLUS | FsOptions::READDIRPLUS_AUTO; + let mut opts = + FsOptions::DO_READDIRPLUS | FsOptions::READDIRPLUS_AUTO | FsOptions::EXPORT_SUPPORT; if self.cfg.writeback && capable.contains(FsOptions::WRITEBACK_CACHE) { opts |= FsOptions::WRITEBACK_CACHE; self.writeback.store(true, Ordering::Relaxed); @@ -933,16 +941,38 @@ impl FileSystem for PassthroughFs { F: FnMut(DirEntry, Entry) -> io::Result<usize>, { self.do_readdir(inode, handle, size, offset, |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) }; - let entry = self.do_lookup(inode, name)?; - - add_entry(dir_entry, entry) + let name = dir_entry.name.to_bytes(); + let entry = if name == b"." || name == b".." { + // Don't do lookups on the current directory or the parent directory. Safe because + // this only contains integer fields and any value is valid. + let mut attr = unsafe { MaybeUninit::<libc::stat64>::zeroed().assume_init() }; + attr.st_ino = dir_entry.ino; + attr.st_mode = dir_entry.type_; + + // We use 0 for the inode value to indicate a negative entry. + Entry { + inode: 0, + generation: 0, + attr, + attr_timeout: Duration::from_secs(0), + entry_timeout: Duration::from_secs(0), + } + } else { + self.do_lookup(inode, dir_entry.name)? + }; + + let entry_inode = entry.inode; + add_entry(dir_entry, entry).map_err(|e| { + if entry_inode != 0 { + // Undo the `do_lookup` for this inode since we aren't going to report it to + // the kernel. If `entry_inode` was 0 then that means this was the "." or + // ".." entry and there wasn't a lookup in the first place. + let mut inodes = self.inodes.lock(); + forget_one(&mut inodes, entry_inode, 1); + } + + e + }) }) } @@ -1747,3 +1777,29 @@ impl FileSystem for PassthroughFs { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn padded_cstrings() { + assert_eq!(strip_padding(b".\0\0\0\0\0\0\0").to_bytes(), b"."); + assert_eq!(strip_padding(b"..\0\0\0\0\0\0").to_bytes(), b".."); + assert_eq!( + strip_padding(b"normal cstring\0").to_bytes(), + b"normal cstring" + ); + assert_eq!(strip_padding(b"\0\0\0\0").to_bytes(), b""); + assert_eq!( + strip_padding(b"interior\0nul bytes\0\0\0").to_bytes(), + b"interior" + ); + } + + #[test] + #[should_panic(expected = "`b` doesn't contain any nul bytes")] + fn no_nul_byte() { + strip_padding(b"no nul bytes in string"); + } +} diff --git a/devices/src/virtio/fs/server.rs b/devices/src/virtio/fs/server.rs index c9025f2..33b7c98 100644 --- a/devices/src/virtio/fs/server.rs +++ b/devices/src/virtio/fs/server.rs @@ -8,7 +8,6 @@ use std::io::{self, Read, Write}; use std::mem::size_of; use data_model::DataInit; -use libc; use sys_util::error; use crate::virtio::fs::filesystem::{ @@ -19,7 +18,7 @@ use crate::virtio::fs::fuse::*; use crate::virtio::fs::{Error, Result}; use crate::virtio::{Reader, Writer}; -const MAX_BUFFER_SIZE: u32 = (1 << 20); +const MAX_BUFFER_SIZE: u32 = 1 << 20; const DIRENT_PADDING: [u8; 8] = [0; 8]; struct ZCReader<'a>(Reader<'a>); @@ -1369,12 +1368,14 @@ fn add_dirent( d: DirEntry, entry: Option<Entry>, ) -> io::Result<usize> { - if d.name.len() > ::std::u32::MAX as usize { + // Strip the trailing '\0'. + let name = d.name.to_bytes(); + if name.len() > ::std::u32::MAX as usize { return Err(io::Error::from_raw_os_error(libc::EOVERFLOW)); } let dirent_len = size_of::<Dirent>() - .checked_add(d.name.len()) + .checked_add(name.len()) .ok_or_else(|| io::Error::from_raw_os_error(libc::EOVERFLOW))?; // Directory entries must be padded to 8-byte alignment. If adding 7 causes @@ -1402,12 +1403,12 @@ fn add_dirent( let dirent = Dirent { ino: d.ino, off: d.offset, - namelen: d.name.len() as u32, + namelen: name.len() as u32, type_: d.type_, }; cursor.write_all(dirent.as_slice())?; - cursor.write_all(d.name)?; + cursor.write_all(name)?; // We know that `dirent_len` <= `padded_dirent_len` due to the check above // so there's no need for checked arithmetic. |