Re: [PATCH 0/8] blk-mq: fix request UAF related with iterating over tagset requests

Jens Axboe
2021-04-25T16:17:06Z

On 4/25/21 2:57 AM, Ming Lei wrote:
> Hi Guys,
>
> Revert 4 patches from Bart which try to fix request UAF issue related
> with iterating over tagset wide requests, because:
>
> 1) request UAF caused by normal completion vs. async completion during
> iterating can't be covered[1]
>
> 2) clearing ->rqs[] is added in fast path, which causes performance loss
> by 1% according to Bart's test
>
> 3) Bart's approach is too complicated, and some changes aren't needed,
> such as adding two versions of tagset iteration
>
> This patchset fixes the request UAF issue by one simpler approach,
> without any change in fast path.
>
> 1) always complete request synchronously when the completing is run
> via blk_mq_tagset_busy_iter(), done in 1st two patches
>
> 2) grab request's ref before calling ->fn in blk_mq_tagset_busy_iter,
> and release it after calling ->fn, so ->fn won't be called for one
> request if its queue is frozen, done in 3rd patch
>
> 3) clearing any stale request referred in ->rqs[] before freeing the
> request pool, one per-tags spinlock is added for protecting
> grabbing request ref vs. clearing ->rqs[tag], so UAF by refcount_inc_not_zero
> in bt_tags_iter() is avoided, done in 4th patch.

I'm going to pull the UAF series for now so we don't need to do a series
of reverts if we deem this a better approach. I'll take a further look
at it tomorrow.

--
Jens Axboe