Re: [PATCH 02/16] io_uring: cqe init hardening

[Pavel Begunkov <asml.silence@gmail.com>]

On 8/19/23 16:03, Jens Axboe wrote:
> On 8/15/23 11:31 AM, Pavel Begunkov wrote:
>> io_kiocb::cqe stores the completion info which we'll memcpy to
>> userspace, and we rely on callbacks and other later steps to populate
>> it with right values. We have never had problems with that, but it would
>> still be safer to zero it on allocation.
>>
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>   io_uring/io_uring.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>> index e189158ebbdd..4d27655be3a6 100644
>> --- a/io_uring/io_uring.c
>> +++ b/io_uring/io_uring.c
>> @@ -1056,7 +1056,7 @@ static void io_preinit_req(struct io_kiocb *req, struct io_ring_ctx *ctx)
>>   	req->link = NULL;
>>   	req->async_data = NULL;
>>   	/* not necessary, but safer to zero */
>> -	req->cqe.res = 0;
>> +	memset(&req->cqe, 0, sizeof(req->cqe));
>>   }
>>   
>>   static void io_flush_cached_locked_reqs(struct io_ring_ctx *ctx,
> 
> I think this is a good idea, but I wonder if we should open-clear it
> instead. I've had cases in the past where that's more efficient than
> calling memset.

I don't think it ever happens for 16 byte memcpy, and in either
case it's a cache refill, quite a slow path. I believe memcpy is
better here.

-- 
Pavel Begunkov