Re: [PATCH v2 2/2] fuse: reject oversized payload_sz in fuse_uring_copy_from_ring()

[Bernd Schubert]

Hi Xiang,

On 7/7/26 20:44, Xiang Mei wrote:
> fuse_uring_copy_from_ring() imports the payload buffer with length
> ring->max_payload_sz but passes the server-controlled payload_sz to
> fuse_copy_out_args() unchecked.  A larger payload_sz drains the iterator
> to exhaustion and fuse_copy_fill() hits BUG_ON(!err), panicking the
> kernel.  Reject replies whose payload_sz exceeds the imported buffer.
> 
>   kernel BUG at fs/fuse/dev.c:1053!
>   RIP: 0010:fuse_copy_fill+0x6c6/0x7e0
>   Call Trace:
>    fuse_copy_args
>    fuse_uring_copy_from_ring     fs/fuse/dev_uring.c:686
>    fuse_uring_cmd
>    io_uring_cmd
>    __io_issue_sqe
>    io_submit_sqes
>    __do_sys_io_uring_enter
>    entry_SYSCALL_64_after_hwframe
> 
> 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>
> Reviewed-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> ---
> v2: add: Cc stable and Reviewed-by tags
> 
>  fs/fuse/dev_uring.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index 0814681eb04b..f6127c230dd9 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -679,6 +679,9 @@ static int fuse_uring_copy_from_ring(struct fuse_ring *ring,
>  	if (err)
>  		return err;
>  
> +	if (ring_in_out.payload_sz > ring->max_payload_sz)
> +		return -EINVAL;
> +
>  	err = setup_fuse_copy_state(&cs, ring, req, ent, ITER_SOURCE, &iter);
>  	if (err)
>  		return err;

Good catch and sorry for lare review! Hrmm, it just gives me a bit headache,
because idea for max_payload_size in fuse_uring_create() that it prevents 
exactly that.

After tracing through the code, I think we have two cases where max_payload calculation
in fuse_uring_create() is not enough for xattr and ioctl

For xattr we have an additional in addition to the patch above - it sends unchecked
against max_pages and  fuse_dev_do_read() has an additional op code protection
that I had missed

       /* If request is too large, reply with an error and restart the read */                                                                                         
        if (nbytes < reqsize) {                                                                                                                                         
                req->out.h.error = -EIO;                                                                                                                                
                /* SETXATTR is special, since it may contain too large data */                                                                                          
                if (args->opcode == FUSE_SETXATTR)                                                                                                                      
                        req->out.h.error = -E2BIG;                                                                                                                      
                fuse_request_end(req);                                                                                                                                  
                goto restart;                                                                                                                                           
        }                                                                                                                                                               



And I think with the current patch is incomplete and missing something like this

diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index 77c8cec43d9c..449b84ac24e7 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -725,6 +725,14 @@ static int fuse_uring_args_to_ring(struct fuse_ring *ring, struct fuse_req *req,
                num_args--;
        }
 
+       /*
+        * A FUSE_SETXATTR value may exceed the ring buffer; match
+        * fuse_dev_do_read() instead of overrunning the payload iterator.
+        */
+       if (fuse_len_args(num_args, (struct fuse_arg *)in_args) >
+           ring->max_payload_sz)
+               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);



And a generic patch, but that has the potential to break existing userspace is

diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index 449b84ac24e7..d25d7922bbdd 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -251,7 +251,14 @@ static struct fuse_ring *fuse_uring_create(struct fuse_chan *fch)
                goto out_err;
 
        max_payload_size = max(FUSE_MIN_READ_BUFFER, fch->max_write);
-       max_payload_size = max(max_payload_size, fch->max_pages * PAGE_SIZE);
+       /*
+        * A max_pages-sized paged reply may be preceded by a fixed op reply
+        * header (e.g. FUSE_IOCTL); reserve a page of header room generically.
+        */
+       max_payload_size = max(max_payload_size,
+                              fch->max_pages * PAGE_SIZE + PAGE_SIZE);
+       /* getxattr/listxattr values are bounded only by XATTR_SIZE_MAX */
+       max_payload_size = max(max_payload_size, (size_t)XATTR_SIZE_MAX);
 
        spin_lock(&fch->lock);
        if (!fch->connected) {


Question is how we could add it in, maybe with a feature flag?


Thanks,
Bernd