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

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.

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.

-- 
Jens Axboe