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

[Dylan Yudaken <dyudaken@gmail.com>]

On Mon, Nov 6, 2023 at 2:51 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> 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.

Yes - I think this approach is better, will rebase on these