Skip to content

Commit 817d6df

Browse files
committed
passthroughfs: avoid extra lookup after readdirplus
The Sys_getdents64 in kernel will pad the name with '\0' bytes up to 8-byte alignment, so @name of LinuxDirent64 may contain a few null terminators. This causes an extra lookup from fuse kernel 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 so that kernel has to issue another lookup to get a valid dentry. Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
1 parent bb93d85 commit 817d6df

File tree

2 files changed

+25
-8
lines changed

2 files changed

+25
-8
lines changed

src/api/server.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1394,7 +1394,8 @@ fn reply_error(err: io::Error, unique: u64, mut w: Writer) -> Result<usize> {
13941394
.map_err(Error::EncodeMessage)
13951395
}
13961396

1397-
fn bytes_to_cstr(buf: &[u8]) -> Result<&CStr> {
1397+
/// trim all trailing nul terminators.
1398+
pub fn bytes_to_cstr(buf: &[u8]) -> Result<&CStr> {
13981399
// There might be multiple 0s at the end of buf, find & use the first one and trim other zeros.
13991400
let len = buf.len();
14001401
let mut n: usize = 0;

src/passthrough/fs.rs

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use crate::api::filesystem::{
2828
Context, DirEntry, Entry, FileSystem, FsOptions, GetxattrReply, ListxattrReply, OpenOptions,
2929
SetattrValid, ZeroCopyReader, ZeroCopyWriter,
3030
};
31+
use crate::api::server;
3132
use crate::api::{BackendFileSystem, VFS_MAX_INO};
3233
#[cfg(feature = "vhost-user-fs")]
3334
use crate::transport::FsCacheReqHandler;
@@ -584,6 +585,20 @@ impl PassthroughFs {
584585
// break the loop so return `Ok` with a non-zero value instead.
585586
Ok(1)
586587
} else {
588+
// The Sys_getdents64 in kernel will pad the name with '\0'
589+
// bytes up to 8-byte alignment, so @name may contain a few null
590+
// terminators. This causes an extra lookup from fuse when
591+
// called by readdirplus, because kernel path walking only takes
592+
// name without null terminators, the dentry with more than 1
593+
// null terminators added by readdirplus doesn't satisfy the
594+
// path walking.
595+
let name = server::bytes_to_cstr(name)
596+
.map_err(|e| {
597+
error!("do_readdir: {:?}", e);
598+
io::Error::from_raw_os_error(libc::EINVAL)
599+
})?
600+
.to_bytes();
601+
587602
add_entry(DirEntry {
588603
ino: dirent64.d_ino,
589604
offset: dirent64.d_off as u64,
@@ -908,13 +923,14 @@ impl FileSystem for PassthroughFs {
908923
add_entry: &mut dyn FnMut(DirEntry, Entry) -> io::Result<usize>,
909924
) -> io::Result<()> {
910925
self.do_readdir(inode, handle, size, offset, &mut |dir_entry| {
911-
// Safe because the kernel guarantees that the buffer is nul-terminated. Additionally,
912-
// the kernel will pad the name with '\0' bytes up to 8-byte alignment and there's no
913-
// way for us to know exactly how many padding bytes there are. This would cause
914-
// `CStr::from_bytes_with_nul` to return an error because it would think there are
915-
// interior '\0' bytes. We trust the kernel to provide us with properly formatted data
916-
// so we'll just skip the checks here.
917-
let name = unsafe { CStr::from_bytes_with_nul_unchecked(dir_entry.name) };
926+
// Safe because do_readdir() has ensured dir_entry.name is a
927+
// valid [u8] generated by CStr::to_bytes().
928+
let name = unsafe {
929+
CStr::from_bytes_with_nul_unchecked(std::slice::from_raw_parts(
930+
&dir_entry.name[0],
931+
dir_entry.name.len() + 1,
932+
))
933+
};
918934
let entry = self.do_lookup(inode, name)?;
919935
let ino = entry.inode;
920936

0 commit comments

Comments
 (0)