Re: [PATCH] io_uring: zero remained bytes when reading to fixed kernel buffer

[Ming Lei <ming.lei@redhat.com>]

On Sat, Mar 22, 2025 at 12:02:02PM +0000, Pavel Begunkov wrote:
> On 3/22/25 07:56, Ming Lei wrote:
> > So far fixed kernel buffer is only used for FS read/write, in which
> > the remained bytes need to be zeroed in case of short read, otherwise
> > kernel data may be leaked to userspace.
> 
> Can you remind me, how that can happen? Normally, IIUC, you register
> a request filled with user pages, so no kernel data there. Is it some
> bounce buffers?

For direct io, it is filled with user pages, but it can be buffered IO,
and the page can be mapped to userspace.

> 
> > Add two helpers for fixing this issue, meantime replace one check
> > with io_use_fixed_kbuf().
> > 
> > Cc: Caleb Sander Mateos <csander@purestorage.com>
> > Cc: Keith Busch <kbusch@kernel.org>
> > Fixes: 27cb27b6d5ea ("io_uring: add support for kernel registered bvecs")
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> ...
> > +/* zero remained bytes of kernel buffer for avoiding to leak data */
> > +static inline void io_req_zero_remained(struct io_kiocb *req,
> > +					struct iov_iter *iter)
> > +{
> > +	size_t left = iov_iter_count(iter);
> > +
> > +	if (left > 0 && iov_iter_rw(iter) == READ)
> > +		iov_iter_zero(left, iter);
> > +}
> > +
> >   #endif
> > diff --git a/io_uring/rw.c b/io_uring/rw.c
> > index 039e063f7091..67dc1a6710c9 100644
> > --- a/io_uring/rw.c
> > +++ b/io_uring/rw.c
> > @@ -541,6 +541,12 @@ static void __io_complete_rw_common(struct io_kiocb *req, long res)
> >   	} else {
> >   		req_set_fail(req);
> >   		req->cqe.res = res;
> > +
> > +		if (io_use_fixed_kbuf(req)) {
> > +			struct io_async_rw *io = req->async_data;
> > +
> > +			io_req_zero_remained(req, &io->iter);
> > +		}
> 
> I think it can be exploited. It's called from ->ki_complete, i.e.
> io_complete_rw, so make the request size enough, if you're stuck
> copying in [soft]irq for too long.

Short read seldom happens, so how it can be exploited? And the request size
can't be too big in this(ublk) use case.

Thanks,
Ming