Skip to content

Commit dd22d4b

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. Signed-off-by: zyfjeff <zyfjeff@linux.alibaba.com>
1 parent 68df7ba commit dd22d4b

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)