Re: [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot

[Gabriel Krisman Bertazi <krisman@suse.de>]

Jann Horn <jannh@google.com> writes:

>
> diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c
> index 3ae4804765b9..0b656a6c8428 100644
> --- a/io_uring/fdinfo.c
> +++ b/io_uring/fdinfo.c
> @@ -155,9 +155,11 @@ static void __io_uring_show_fdinfo(struct io_ring_ctx *ctx, struct seq_file *m)
>  	for (i = 0; i < cq_entries; i++) {
>  		struct io_uring_cqe *cqe;
>  		bool cqe32 = false;
> +		bool is_last_cqarray_slot = (cq_head == cq_mask);
>  
>  		cqe = &r->cqes[(cq_head & cq_mask)];
> -		if (cqe->flags & IORING_CQE_F_32 || ctx->flags & IORING_SETUP_CQE32)
> +		if ((cqe->flags & IORING_CQE_F_32 || ctx->flags & IORING_SETUP_CQE32) &&
> +		    !is_last_cqarray_slot)

Could we have a small comment here explaining we can't just trust
IORING_CQE_F_32 for the last slot?  it is very non-obvious to me from
the code. Something as simple as (if I got it right):

/* userspace can manipulate IORING_CQE_F_32 which is usually harmless,
 * except in the last CQ slot.  Ignore the flag only for that slot to avoid
 * reading memory out-of-bounds.
 */

Either way,

Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de>

Thanks,

>  			cqe32 = true;
>  		seq_printf(m, "%5u: user_data:%llu, res:%d, flags:%x",
>  			   cq_head & cq_mask, cqe->user_data, cqe->res,
>
> ---
> base-commit: 50d05c7c76c96b90462f24debacca971d2e86713
> change-id: 20260910-uring-fdinfo-tighten-1b1c18945305
>
> Best regards,
> --  
> Jann Horn <jannh@google.com>
>

-- 
Gabriel Krisman Bertazi