Re: v4.20-rc6: Sporadic use-after-free in bt_iter()

[Bart Van Assche <bvanassche@acm.org>]

On Thu, 2018-12-20 at 15:50 -0700, Jens Axboe wrote:
> +static void blk_fq_rcu_free(struct work_struct *work)
> +{
> +	struct blk_flush_queue *fq = container_of(to_rcu_work(work),
> +							struct blk_flush_queue,
> +							rcu_work);
> +
> +	kfree(fq->flush_rq);
> +	kfree(fq);
> +}
> +
>  void blk_free_flush_queue(struct blk_flush_queue *fq)
>  {
>  	/* bio based request queue hasn't flush queue */
>  	if (!fq)
>  		return;
>  
> -	kfree(fq->flush_rq);
> -	kfree(fq);
> +	INIT_RCU_WORK(&fq->rcu_work, blk_fq_rcu_free);
> +	queue_rcu_work(system_wq, &fq->rcu_work);
>  }

Can INIT_RCU_WORK() + queue_rcu_work() be changed into call_rcu()? The latter
namely uses a smaller data structure.

> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
> index 2089c6c62f44..c39b58391ae8 100644
> --- a/block/blk-mq-tag.c
> +++ b/block/blk-mq-tag.c
> @@ -228,13 +228,15 @@ static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
>  
>  	if (!reserved)
>  		bitnr += tags->nr_reserved_tags;
> -	rq = tags->rqs[bitnr];
> +	if (tags->rqs[bitnr].queue != hctx->queue)
> +		return true;

Since blk_mq_tag_set_rq() is not serialized against this function I doubt that
the tags->rqs[bitnr].queue != hctx->queue check helps. Can it be left out?

> +struct rq_tag_entry {
> +       struct request_queue *queue;
> +       struct request *rq;

If the new test can be left out from bt_iter(), can this new data structure be
left out too? In other words, keep the existing approach of only storing the
request pointer and not the queue pointer.

Thanks,

Bart.