linux-kernel - Re: [PATCH 2/8] blk-mq: change the method of iterating busy tags of a request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bf563d09-c243-5bf5-ab3e-0fa2bd583edd@grimberg.me>
Date:   Wed, 20 Mar 2019 11:52:52 -0700
From:   Sagi Grimberg <sagi@...mberg.me>
To:     Jianchao Wang <jianchao.w.wang@...cle.com>, axboe@...nel.dk
Cc:     hch@....de, jthumshirn@...e.de, hare@...e.de, josef@...icpanda.com,
        bvanassche@....org, keith.busch@...el.com, jsmart2021@...il.com,
        linux-block@...r.kernel.org, linux-nvme@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/8] blk-mq: change the method of iterating busy tags of a
 request_queue



On 3/15/19 1:57 AM, Jianchao Wang wrote:
> tags->rqs[] will not been cleaned when free driver tag and there
> is a window between get driver tag and write tags->rqs[], so we
> may see stale rq in tags->rqs[] which may have been freed, as
> following case,
> blk_mq_get_request         blk_mq_queue_tag_busy_iter
>    -> blk_mq_get_tag
>                               -> bt_for_each
>                                 -> bt_iter
>                                   -> rq = taags->rqs[]
>                                   -> rq->q
>    -> blk_mq_rq_ctx_init
>      -> data->hctx->tags->rqs[rq->tag] = rq;
> 
> To fix this, the blk_mq_queue_tag_busy_iter is changed in this
> patch to use tags->static_rqs[] instead of tags->rqs[]. We have
> to identify whether there is a io scheduler attached to decide
> to use hctx->tags or hctx->sched_tags. And we will try to get a
> non-zero q_usage_counter before that, so it is safe to access
> them. Add 'inflight' parameter to determine to iterate in-flight
> requests or just busy tags. A correction here is that
> part_in_flight should count the busy tags instead of rqs that
> have got driver tags.

IMO, instead of this parameter, add a wrapper like
blk_mq_queue_tag_inflight_iter() or keep the parameter out until
we actually have a user that calls it for busy and not inflight.

Other than that, I think that iterating over static_rqs is a good
solution to the problem described.