On Thu, May 06, 2021 at 07:54:40AM +0100, Christoph Hellwig wrote: > On Wed, May 05, 2021 at 10:58:53PM +0800, Ming Lei wrote: > > Grab rq->refcount before calling ->fn in blk_mq_tagset_busy_iter(), and > > this way will prevent the request from being re-used when ->fn is > > running. The approach is same as what we do during handling timeout. > > > > Fix request UAF related with completion race or queue releasing: > > I guess UAF is supposed to mean use-after-free? Please just spell that > out. OK. > > > + rq = blk_mq_find_and_get_req(tags, bitnr); > > /* > > * We can hit rq == NULL here, because the tagging functions > > * test and set the bit before assigning ->rqs[]. > > */ > > + if (!rq) > > + return true; > > Nit: I'd find this much earier to follow if the blk_mq_find_and_get_req > and thus assignment to rq was after the block comment. OK. > > > struct blk_mq_tags *tags = iter_data->tags; > > bool reserved = iter_data->flags & BT_TAG_ITER_RESERVED; > > struct request *rq; > > + bool ret = true; > > + bool iter_static_rqs = !!(iter_data->flags & BT_TAG_ITER_STATIC_RQS); > > > > if (!reserved) > > bitnr += tags->nr_reserved_tags; > > @@ -272,16 +288,19 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data) > > * We can hit rq == NULL here, because the tagging functions > > * test and set the bit before assigning ->rqs[]. > > */ > > + if (iter_static_rqs) > > I think this local variable just obsfucates what is going on here. It has to be one local variable, otherwise sparse may complain since iter_data->flags may be thought as being changed during the period. Thanks, Ming
