Re: [PATCH 1/2] io_uring: do not allow multishot read to set addr or len

[Jens Axboe <axboe@kernel.dk>]

On 11/6/23 7:32 AM, Jens Axboe wrote:
> On 11/5/23 3:30 PM, Dylan Yudaken wrote:
>> For addr: this field is not used, since buffer select is forced. But by forcing
>> it to be zero it leaves open future uses of the field.
>>
>> len is actually usable, you could imagine that you want to receive
>> multishot up to a certain length.
>> However right now this is not how it is implemented, and it seems
>> safer to force this to be zero.
>>
>> Fixes: fc68fcda0491 ("io_uring/rw: add support for IORING_OP_READ_MULTISHOT")
>> Signed-off-by: Dylan Yudaken <dyudaken@gmail.com>
>> ---
>>  io_uring/rw.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/io_uring/rw.c b/io_uring/rw.c
>> index 1c76de483ef6..ea86498d8769 100644
>> --- a/io_uring/rw.c
>> +++ b/io_uring/rw.c
>> @@ -111,6 +111,13 @@ int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>  	rw->len = READ_ONCE(sqe->len);
>>  	rw->flags = READ_ONCE(sqe->rw_flags);
>>  
>> +	if (req->opcode == IORING_OP_READ_MULTISHOT) {
>> +		if (rw->addr)
>> +			return -EINVAL;
>> +		if (rw->len)
>> +			return -EINVAL;
>> +	}
> 
> Should we just put these in io_read_mshot_prep() instead? Ala the below.
> In general I think it'd be nice to have a core prep_rw, and then each
> variant will have its own prep. Then we can get away from random opcode
> checking in there.
> 
> I do agree with the change in general, just think we can tweak it a bit
> to make it a bit cleaner.

Sent out two cleanups that take it in this direction in general, fwiw.

-- 
Jens Axboe