On Fri, Apr 2, 2021 at 8:26 PM Bart Van Assche <bvanassche@xxxxxxx> wrote:
>
> On 4/2/21 4:59 PM, Khazhy Kumykov wrote:
> > On Sun, Mar 28, 2021 at 7:00 PM Bart Van Assche <bvanassche@xxxxxxx> wrote:
> >> @@ -209,7 +209,12 @@ static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
> >>
> >> if (!reserved)
> >> bitnr += tags->nr_reserved_tags;
> >> - rq = tags->rqs[bitnr];
> >> + /*
> >> + * See also the percpu_ref_tryget() and blk_queue_exit() calls in
> >> + * blk_mq_queue_tag_busy_iter().
> >> + */
> >> + rq = rcu_dereference_check(tags->rqs[bitnr],
> >> + !percpu_ref_is_zero(&hctx->queue->q_usage_counter));
> >
> > do we need to worry about rq->q != hctx->queue here? i.e., could we
> > run into use-after-free on rq->q == hctx->queue check below, since
> > rq->q->q_usage_counter might not be raised? Once we verify rq->q ==
> > hctx->queue, i agree q_usage_counter is sufficient then
>
> That's a great question. I will change the second
> rcu_dereference_check() argument into 'true' and elaborate the comment
> above rcu_dereference_check().
>
> >> -static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
> >> +static bool __bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr,
> >> + void *data)
> >> {
> >> struct bt_tags_iter_data *iter_data = data;
> >> struct blk_mq_tags *tags = iter_data->tags;
> >> @@ -275,7 +286,7 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
> >> if (iter_data->flags & BT_TAG_ITER_STATIC_RQS)
> >> rq = tags->static_rqs[bitnr];
> >> else
> >> - rq = tags->rqs[bitnr];
> >> + rq = rcu_dereference_check(tags->rqs[bitnr], true);
> >
> > lockdep_is_held(&tags->iter_rwsem) ?
>
> I will change the second rcu_dereference_check() argument into the
> following:
>
> rcu_read_lock_held() || lockdep_is_held(&tags->iter_rwsem)
rcu_dereference_check() already has a || rcu_read_lock_held(), fwiw
>
> >> + /*
> >> + * Freeing tags happens with the request queue frozen so the
> >> + * srcu dereference below is protected by the request queue
> >
> > s/srcu/rcu
>
> Thanks, will fix.
>
> Bart.
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
