Messages in this thread | | | Date | Sat, 7 May 2022 19:06:30 +0800 | Subject | Re: [PATCH 3/5] io_uring: let fast poll support multishot | From | 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.
| |