Re: [PATCH] io_uring: validate user-controlled cq.head in io_cqe_cache_refill()

[Zizhi Wo]

在 2026/5/13 22:20, Jens Axboe 写道:
> On 5/13/26 8:18 AM, Jens Axboe wrote:
> > On 5/13/26 12:32 AM, Zizhi Wo wrote:
> > > diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> > > index 4ed998d60c09..92e255e9e08f 100644
> > > --- a/io_uring/io_uring.c
> > > +++ b/io_uring/io_uring.c
> > > @@ -710,11 +710,13 @@ static bool io_fill_nop_cqe(struct io_ring_ctx *ctx, unsigned int off)
> > >    * fill the cq entry
> > >    */
> > >   bool io_cqe_cache_refill(struct io_ring_ctx *ctx, bool overflow, bool cqe32)
> > >   {
> > >   	struct io_rings *rings = ctx->rings;
> > > -	unsigned int off = ctx->cached_cq_tail & (ctx->cq_entries - 1);
> > > +	unsigned int head = READ_ONCE(ctx->rings->cq.head);
> > > +	unsigned int tail = ctx->cached_cq_tail;
> > > +	unsigned int off = tail & (ctx->cq_entries - 1);
> > >   	unsigned int free, queued, len;
> >
> > This looks wrong, as you're snapshotting 'tail' while it could get
> > modified by if a nop fill before the refill happens. And fwiw, looks
> > like the refill part potentially suffers from the same unsigned issue.

Yes. I wasn't aware of io_fill_nop_cqe(), the fact that
cached_cq_tail can be modified between the snapshot and the refill
was missed. The same oversight also means the unsigned issue in that
function went unnoticed...

> To be clearer, I think you want to add a helper ala:
>
> static unsigned int io_cqring_queued(struct io_ring_ctx *ctx)
> {
> 	struct io_rings *rings = io_get_rings(ctx);
> 	int diff;
>
> 	diff = (int)( ctx->cached_cq_tail - READ_ONCE(rings->cq.head));
> 	if (diff >= 0)
>          	return min((unsigned int) diff, ctx->cq_entries);
> 	return 0;
> }
>
> or something like that, and then use it in both spots. Would make for a
> cleaner fix, too.

Thanks for the suggestion. I'll send a v2 using this helper in both
spots.

Thanks,
Zizhi Wo