On Sun, Apr 28, 2019 at 02:14:26PM +0200, Christoph Hellwig wrote: > On Sun, Apr 28, 2019 at 04:14:06PM +0800, Ming Lei wrote: > > In normal queue cleanup path, hctx is released after request queue > > is freed, see blk_mq_release(). > > > > However, in __blk_mq_update_nr_hw_queues(), hctx may be freed because > > of hw queues shrinking. This way is easy to cause use-after-free, > > because: one implicit rule is that it is safe to call almost all block > > layer APIs if the request queue is alive; and one hctx may be retrieved > > by one API, then the hctx can be freed by blk_mq_update_nr_hw_queues(); > > finally use-after-free is triggered. > > > > Fixes this issue by always freeing hctx after releasing request queue. > > If some hctxs are removed in blk_mq_update_nr_hw_queues(), introduce > > a per-queue list to hold them, then try to resuse these hctxs if numa > > node is matched. > > This seems a little odd. Wouldn't it be much simpler to just keep > the hctx where it is, that is leave the queue_hw_ctx[] pointer in tact, > but have a flag marking it dead?
There are several issues with that solution: 1) q->nr_hw_queues becomes not same with number of the real active hw queues 2) if one hctx is only marked as dead and not freed, after several times of updating nr_hw_queues, more and more hctx instances will be wasted. 3) q->queue_hw_ctx[] has to be re-allocated when nr_hw_queues is increased. So I think this patch should be simpler, either in concept or implementation. Thanks, Ming
