blk-throttle: fix throtl_data leak during disk release

Tightening the throttle activation check in blk_throtl_activated() to
require both q->td presence and policy bit set introduced a memory leak
during disk release:

blkg_destroy_all() clears the policy bit first during queue deactivation,
causing subsequent blk_throtl_exit() to skip throtl_data cleanup when
blk_throtl_activated() fails policy check.

Idealy we should avoid modifying blk_throtl_exit() activation check because
it's intuitive that blk-throtl start from blk_throtl_init() and end in
blk_throtl_exit(). However, call blk_throtl_exit() before
blkg_destroy_all() will make a long term deadlock problem easier to
trigger[1], hence fix this problem by checking if q->td is NULL from
blk_throtl_exit(), and remove policy deactivation as well since it's
useless.

[1] https://lore.kernel.org/all/CAHj4cs9p9H5yx+ywsb3CMUdbqGPhM+8tuBvhW=9ADiCjAqza9w@mail.gmail.com/#t

Fixes: bd9fd5be6bc0 ("blk-throttle: fix access race during throttle policy activation")
Reported-by: Yi Zhang <yi.zhang@redhat.com>
Closes: https://lore.kernel.org/all/CAHj4cs-p-ZwBEKigBj7T6hQCOo-H68-kVwCrV6ZvRovrr9Z+HA@mail.gmail.com/
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index f510ae072868ca72686df8c32d30e454beeff9a8..2c5b64b1a724a722f7187f0abd078fe372db90df 100644 (file)
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1842,12 +1842,15 @@ void blk_throtl_exit(struct gendisk *disk)
 {
        struct request_queue *q = disk->queue;
 
-       if (!blk_throtl_activated(q))
+       /*
+        * blkg_destroy_all() already deactivate throtl policy, just check and
+        * free throtl data.
+        */
+       if (!q->td)
                return;
 
        timer_delete_sync(&q->td->service_queue.pending_timer);
        throtl_shutdown_wq(q);
-       blkcg_deactivate_policy(disk, &blkcg_policy_throtl);
        kfree(q->td);
 }