Re: [PATCH v1] io_uring: fix dangling iovec after provided-buffer bundle grow failure

[Hao-Yu Yang]

Oh, this patch looks more clear. The original iovec is freed only after
 access_ok is verified, and the updated iovec for kmsg is returned to
the upper layer. In the error path, the original iovec object is not
freed, and the upper layer's iovec is not updated along that path.

If there are no other issues, I will send the second version of the
patch when I wake up.

Thanks for your review.

On Mon, Jul 06, 2026 at 10:19:10AM -0600, Jens Axboe wrote:
> On 7/5/26 5:45 PM, Hao-Yu Yang wrote:
> > diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
> > index 3cd29477fff2..4055173e0c48 100644
> > --- a/io_uring/kbuf.c
> > +++ b/io_uring/kbuf.c
> > @@ -256,6 +256,7 @@ static int io_ring_buffers_peek(struct io_kiocb *req, struct buf_sel_arg *arg,
> >  	struct io_uring_buf_ring *br = bl->buf_ring;
> >  	struct iovec *org_iovs = arg->iovs;
> >  	struct iovec *iov = arg->iovs;
> > +	struct iovec *old = NULL;
> >  	int nr_iovs = arg->nr_iovs;
> >  	__u16 nr_avail, tail, head;
> >  	struct io_uring_buf *buf;
> > @@ -288,7 +289,7 @@ static int io_ring_buffers_peek(struct io_kiocb *req, struct buf_sel_arg *arg,
> >  		if (unlikely(!iov))
> >  			return -ENOMEM;
> >  		if (arg->mode & KBUF_MODE_FREE)
> > -			kfree(arg->iovs);
> > +			old = arg->iovs;
> >  		arg->iovs = iov;
> >  		nr_iovs = nr_avail;
> >  	} else if (nr_avail < nr_iovs) {
> > @@ -318,6 +319,8 @@ static int io_ring_buffers_peek(struct io_kiocb *req, struct buf_sel_arg *arg,
> >  		if (unlikely(!access_ok(iov->iov_base, len))) {
> >  			if (arg->iovs != org_iovs)
> >  				kfree(arg->iovs);
> > +			/* hand the still-live cached vec back to the owner */
> > +			arg->iovs = org_iovs;
> >  			return -EFAULT;
> >  		}
> >  		iov++;
> > @@ -330,6 +333,8 @@ static int io_ring_buffers_peek(struct io_kiocb *req, struct buf_sel_arg *arg,
> >  		buf = io_ring_head_to_buf(br, ++head, bl->mask);
> >  	} while (--nr_iovs);
> >  
> > +	kfree(old);
> > +
> >  	if (head == tail)
> >  		req->flags |= REQ_F_BL_EMPTY;
> 
> Can't we just do the below, that seems a lot simpler?
> 
> diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
> index 3cd29477fff2..3bb24d20c890 100644
> --- a/io_uring/kbuf.c
> +++ b/io_uring/kbuf.c
> @@ -287,8 +287,6 @@ static int io_ring_buffers_peek(struct io_kiocb *req, struct buf_sel_arg *arg,
>  		iov = kmalloc_objs(struct iovec, nr_avail);
>  		if (unlikely(!iov))
>  			return -ENOMEM;
> -		if (arg->mode & KBUF_MODE_FREE)
> -			kfree(arg->iovs);
>  		arg->iovs = iov;
>  		nr_iovs = nr_avail;
>  	} else if (nr_avail < nr_iovs) {
> @@ -330,6 +328,9 @@ static int io_ring_buffers_peek(struct io_kiocb *req, struct buf_sel_arg *arg,
>  		buf = io_ring_head_to_buf(br, ++head, bl->mask);
>  	} while (--nr_iovs);
>  
> +	if (arg->mode & KBUF_MODE_FREE)
> +		kfree(org_iovs);
> +
>  	if (head == tail)
>  		req->flags |= REQ_F_BL_EMPTY;
>  
> 
> 
> -- 
> Jens Axboe