Re: [PATCH v6 1/2] fuse: bound io-uring payload copies to the registered buffer size

[Xiang Mei]

On Wed, Sep 9, 2026 at 12:20 PM Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
>
> On Sat, Sep 5, 2026 at 6:29 PM Xiang Mei <xmei5@xxxxxxx> wrote:
> >
> > The fuse-io-uring copy paths do not check that the data they copy fits
> > in the ring entry's payload buffer.  fuse_uring_copy_from_ring() passes
> > the server-supplied payload_sz straight to fuse_copy_out_args(), and
> > fuse_uring_args_to_ring() copies the request in-args without comparing
> > them against the buffer.  Either can exhaust the imported iterator and
> > hit fuse_copy_fill()'s BUG_ON(!err).
> >
> > An unprivileged user can trigger the request side by mounting a FUSE
> > filesystem that registers small payload buffers and then issuing a
> > setxattr() whose value exceeds them:
> >
> >   kernel BUG at fs/fuse/dev.c:1064!
> >   Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
> >   CPU: 1 UID: 0 PID: 148 Comm: exploit Not tainted 7.3.0-rc1 #2
> >   RIP: 0010:fuse_copy_fill (fs/fuse/dev.c:1033)
> >   Call Trace:
> >    <TASK>
> >    fuse_copy_args (fs/fuse/dev.c:1354 fs/fuse/dev.c:1380)
> >    fuse_uring_args_to_ring (fs/fuse/dev_uring.c:891)
> >    fuse_uring_prepare_send (fs/fuse/dev_uring.c:940 fs/fuse/dev_uring.c:1057)
> >    fuse_uring_send_in_task (fs/fuse/dev_uring.c:1745)
> >    tctx_task_work_run (io_uring/tw.c:96)
> >    tctx_task_work (io_uring/tw.c:133)
> >    task_work_run (kernel/task_work.c:233)
> >    io_run_task_work (io_uring/tw.h:84)
> >    io_cqring_wait (io_uring/wait.c:278)
> >    __do_sys_io_uring_enter (io_uring/io_uring.c:2676)
> >    do_syscall_64 (arch/x86/entry/syscall_64.c:61)
> >    entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
> >    </TASK>
> >
> > Bound both directions against ent->payload.iov_len, the length the
> > server registered, so an oversized reply is rejected with -EINVAL and an
> > oversized request with -E2BIG/-EIO, matching fuse_dev_do_read().
> >
> > On a zero-copied request the last page-backed argument is handed to the
> > server as registered pages rather than copied into the payload buffer,
> > so exclude it from the request-side sum to avoid rejecting valid
> > zero-copy reads and writes.
> >
> > Fixes: c090c8abae4b ("fuse: Add io-uring sqe commit and fetch support")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Reported-by: Weiming Shi <bestswngs@xxxxxxxxx>
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Xiang Mei <xmei5@xxxxxxx>
> > ---
> > v2: add: Cc stable and Reviewed-by tags
> > v3: propose the patch fixing another issue found by Bernd by Joanne suggested way
> > v4: no context change as v3; add Reviewed-by: Joanne Koong ...
> > v5: no change
> > v6: rebase on fuse.git#for-next.  The ring entry already keeps the payload
> >     iovec and imports it at iov_len, so only the two bounds checks are left.
> >     Skip the page-backed arg on a zero-copied request, it does not consume
> >     the payload buffer.
> >
> >  fs/fuse/dev_uring.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> > index c6dd420c4034..981b7c295e5e 100644
> > --- a/fs/fuse/dev_uring.c
> > +++ b/fs/fuse/dev_uring.c
> > @@ -775,6 +775,9 @@ static int fuse_uring_copy_from_ring(struct fuse_req *req,
> >         if (err)
> >                 return err;
> >
> > +       if (ring_in_out.payload_sz > ent->payload.iov_len)
> > +               return -EINVAL;
> > +
>
> This logic will break fuse zero-copy reads (ent->payload.iov_len is zero).
>

You're right. This check wrongly rejects every zero-copy read reply.

> imo I think the cleanest fix for this bug in general would be to just
> remove the BUG_ON in fuse_copy_fill() and return back the error, eg
>
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ static int fuse_copy_fill(struct fuse_copy_state *cs)
>               err = iov_iter_get_pages2(cs->iter, &page, PAGE_SIZE, 1, &off);
>               if (err < 0)
>                       return err;
> -             BUG_ON(!err);
> +             if (!err)
> +                     return -EIO;
>               cs->len = err;
>

Agreed, and it's a much better fix than bounding the copies.

> This would cover both the request and reply side and get rid of the
> special logic here and below. It'd also make backporting to stable a
> lot easier. I think it would still be worth having the FUSE_SETXATTR
> errno fix (eg returning -E2BIG instead of -EIO)
>
>     if (fuse_len_args(num_args, (struct fuse_arg *)in_args) >
>           ent->payload.iov_len)
>               return args->opcode == FUSE_SETXATTR ? -E2BIG : -EIO;
>
> as its own patch, where that check is only about picking the right
> errno, not about preventing the BUG.
>
> WDYT?
>
Thanks for your suggestions; this patch is better, and I have sent and
tested v7:

https://lore.kernel.org/fuse-devel/20260910022442.3594649-1-xmei5@xxxxxxx/T/#t

Xiang

> Thanks,
> Joanne
>
> >         err = setup_fuse_copy_state(&cs, req, ent, ITER_SOURCE, &iter,
> >                                     issue_flags);
> >         if (err)
> > @@ -854,6 +857,7 @@ static int fuse_uring_args_to_ring(struct fuse_req *req,
> >         int num_args = args->in_numargs;
> >         int err;
> >         struct iov_iter iter;
> > +       size_t copy_size;
> >         struct fuse_uring_ent_in_out ent_in_out = {
> >                 .flags = 0,
> >                 .commit_id = req->in.h.unique,
> > @@ -887,6 +891,16 @@ static int fuse_uring_args_to_ring(struct fuse_req *req,
> >                 num_args--;
> >         }
> >
> > +       copy_size = fuse_len_args(num_args, (struct fuse_arg *)in_args);
> > +       /* a zero-copied page arg does not consume the payload buffer */
> > +       if (cs.skip_folio_copy && args->in_pages && num_args)
> > +               copy_size -= in_args[num_args - 1].size;
> > +
> > +       if (copy_size > ent->payload.iov_len) {
> > +               fuse_copy_finish(&cs);
> > +               return args->opcode == FUSE_SETXATTR ? -E2BIG : -EIO;
> > +       }
> > +
> >         /* copy the payload */
> >         err = fuse_copy_args(&cs, num_args, args->in_pages,
> >                              (struct fuse_arg *)in_args, 0);
> > --
> > 2.43.0
> >