Skip to content

Commit 7b636bd

Browse files
committed
fix request size too large when use direct io
The fuse directio call path is as follows fuse_read_fill+0xa8/0xb0 fuse_send_read+0x3f/0xb0 fuse_direct_io+0x34a/0x5f0 __fuse_direct_read+0x4e/0x70 fuse_file_read_iter+0x9e/0x140 new_sync_read+0xde/0x120 __vfs_read+0x27/0x40 vfs_read+0x94/0x190 ksys_read+0x4e/0xd0 under the direct io path, fuse initiates a request whose request size is determined by the combination of the user-supplied buffer size and max_read mount parameters. size_t nmax = write ? fc->max_write : fc->max_read; size_t count = iov_iter_count(iter); size_t nbytes = min(count, nmax); nres = fuse_send_read(req, io, pos, nbytes, owner); so we have a problem with the checking of the request size in the code, we always compare the request size with MAX_BUFFER_SIZE, but in fact the maximum value of the request size depends on max_read, in virtiofs scenario max_read is UINT_MAX by default, and in fuse scenario it is possible to adjust max_read by the mounting parameter. the current implementation of fuse-backend-rs uses a fixed buffer to store the fuse response the default value of this buffer is as follows, but in fact, the kernel in the direct io path, the size of the request may be larger than the length of this buffer, which leads to the buffer is not enough to fill the read content, resulting in read failure. so here we limit the size of max_read to the length of our buffer so that the fuse kernel will not send requests that exceed the length of the buffer. in virtiofs scene max_read can't be adjusted, his default is UINT_MAX, but we don't have to worry about it, because the buffer is allocated by the kernel driver, we just use this buffer to fill the response, so we don't need to do any adjustment. Signed-off-by: zyfjeff <zyfjeff@linux.alibaba.com>
1 parent 68df7ba commit 7b636bd

File tree

3 files changed

+14
-26
lines changed

3 files changed

+14
-26
lines changed

src/api/server/async_io.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -322,12 +322,6 @@ impl<F: AsyncFileSystem + Sync> Server<F> {
322322
..
323323
} = ctx.r.read_obj().map_err(Error::DecodeMessage)?;
324324

325-
if size > MAX_BUFFER_SIZE {
326-
return ctx
327-
.async_reply_error_explicit(io::Error::from_raw_os_error(libc::ENOMEM))
328-
.await;
329-
}
330-
331325
let owner = if read_flags & READ_LOCKOWNER != 0 {
332326
Some(lock_owner)
333327
} else {

src/api/server/sync_io.rs

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -436,10 +436,6 @@ impl<F: FileSystem + Sync> Server<F> {
436436
..
437437
} = ctx.r.read_obj().map_err(Error::DecodeMessage)?;
438438

439-
if size > MAX_BUFFER_SIZE {
440-
return ctx.reply_error_explicit(io::Error::from_raw_os_error(libc::ENOMEM));
441-
}
442-
443439
let owner = if read_flags & READ_LOCKOWNER != 0 {
444440
Some(lock_owner)
445441
} else {
@@ -495,10 +491,6 @@ impl<F: FileSystem + Sync> Server<F> {
495491
..
496492
} = ctx.r.read_obj().map_err(Error::DecodeMessage)?;
497493

498-
if size > MAX_BUFFER_SIZE {
499-
return ctx.reply_error_explicit(io::Error::from_raw_os_error(libc::ENOMEM));
500-
}
501-
502494
let owner = if fuse_flags & WRITE_LOCKOWNER != 0 {
503495
Some(lock_owner)
504496
} else {
@@ -618,9 +610,6 @@ impl<F: FileSystem + Sync> Server<F> {
618610

619611
pub(super) fn getxattr<S: BitmapSlice>(&self, mut ctx: SrvContext<'_, F, S>) -> Result<usize> {
620612
let GetxattrIn { size, .. } = ctx.r.read_obj().map_err(Error::DecodeMessage)?;
621-
if size > MAX_BUFFER_SIZE {
622-
return ctx.reply_error_explicit(io::Error::from_raw_os_error(libc::ENOMEM));
623-
}
624613

625614
let buf =
626615
ServerUtil::get_message_body(&mut ctx.r, &ctx.in_header, size_of::<GetxattrIn>())?;
@@ -647,10 +636,6 @@ impl<F: FileSystem + Sync> Server<F> {
647636
pub(super) fn listxattr<S: BitmapSlice>(&self, mut ctx: SrvContext<'_, F, S>) -> Result<usize> {
648637
let GetxattrIn { size, .. } = ctx.r.read_obj().map_err(Error::DecodeMessage)?;
649638

650-
if size > MAX_BUFFER_SIZE {
651-
return ctx.reply_error_explicit(io::Error::from_raw_os_error(libc::ENOMEM));
652-
}
653-
654639
match self.fs.listxattr(ctx.context(), ctx.nodeid(), size) {
655640
Ok(ListxattrReply::Names(val)) => ctx.reply_ok(None::<u8>, Some(&val)),
656641
Ok(ListxattrReply::Count(count)) => {
@@ -820,10 +805,6 @@ impl<F: FileSystem + Sync> Server<F> {
820805
fh, offset, size, ..
821806
} = ctx.r.read_obj().map_err(Error::DecodeMessage)?;
822807

823-
if size > MAX_BUFFER_SIZE {
824-
return ctx.reply_error_explicit(io::Error::from_raw_os_error(libc::ENOMEM));
825-
}
826-
827808
let available_bytes = ctx.w.available_bytes();
828809
if available_bytes < size as usize {
829810
return ctx.reply_error_explicit(io::Error::from_raw_os_error(libc::ENOMEM));

src/transport/fusedev/linux_session.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,12 +434,25 @@ fn fuse_kern_mount(
434434
let meta = mountpoint
435435
.metadata()
436436
.map_err(|e| SessionFailure(format!("stat {mountpoint:?}: {e}")))?;
437+
// the current implementation of fuse-backend-rs uses a fixed buffer to store the fuse response,
438+
// the default value of this buffer is as follows, but in fact, the kernel in the direct io path,
439+
// the size of the request may be larger than the length of this buffer (this is determined by
440+
// the max_read option to determine the maximum size of kernel requests, the default value is
441+
// a very large number), which leads to the buffer is not enough to fill the read content,
442+
// resulting in read failure. so here we limit the size of max_read to the length of our buffer,
443+
// so that the fuse kernel will not send requests that exceed the length of the buffer.
444+
// in virtiofs scene max_read can't be adjusted, his default is UINT_MAX, but we don't have to
445+
// worry about it, because the buffer is allocated by the kernel driver, we just use this buffer
446+
// to fill the response, so we don't need to do any adjustment.
447+
let max_read = FUSE_KERN_BUF_PAGES * pagesize() + FUSE_HEADER_SIZE;
448+
437449
let mut opts = format!(
438-
"default_permissions,fd={},rootmode={:o},user_id={},group_id={}",
450+
"default_permissions,fd={},rootmode={:o},user_id={},group_id={},max_read={}",
439451
file.as_raw_fd(),
440452
meta.permissions().mode() & libc::S_IFMT,
441453
getuid(),
442454
getgid(),
455+
max_read
443456
);
444457
if allow_other {
445458
opts.push_str(",allow_other");

0 commit comments

Comments
 (0)