Re: [PATCH 3/5] io_uring: let fast poll support multishot

[Hao Xu]

在 2022/5/7 下午5:47, Pavel Begunkov 写道:
> On 5/7/22 08:08, Hao Xu wrote:
> > 在 2022/5/7 上午1:19, Pavel Begunkov 写道:
> > > On 5/6/22 08:01, Hao Xu wrote:
> [...]
> > > That looks dangerous, io_queue_sqe() usually takes the request ownership
> > > and doesn't expect that someone, i.e. io_poll_check_events(), may 
> > > still be
> > > actively using it.
> > >
> > > E.g. io_accept() fails on fd < 0, return an error,
> > > io_queue_sqe() -> io_queue_async() -> io_req_complete_failed()
> > > kills it. Then io_poll_check_events() and polling in general
> > > carry on using the freed request => UAF. Didn't look at it
> > > too carefully, but there might other similar cases.
> > I checked this when I did the coding, it seems the only case is
> > while (atomic_sub_return(v & IO_POLL_REF_MASK, &req->poll_refs));
> > uses req again after req recycled in io_queue_sqe() path like you
> > pointed out above, but this case should be ok since we haven't
> > reuse the struct req{} at that point.
>
> Replied to another message with an example that I think might
> be broken, please take a look.
I saw it just now, it looks a valid case to me. Thanks.
> The issue is that io_queue_sqe() was always consuming / freeing /
> redirecting / etc. requests, i.e. call it and forget about the req.
> With io_accept now it may or may not free it and not even returning
> any return code about that. This implicit knowledge is quite tricky
> to maintain.
>
> might make more sense to "duplicate" io_queue_sqe()
>
> ret = io_issue_sqe(req, IO_URING_F_NONBLOCK|IO_URING_F_COMPLETE_DEFER);
> // REQ_F_COMPLETE_INLINE should never happen, no check for that
> // don't care about io_arm_ltimeout(), should already be armed
> // ret handling here
This is what I'm doing for v3, indeed make more sense.