[PATCH] blk-mq: Fix missed srcu_barrier() calls before cleanup_srcu_struct()

[PATCH] blk-mq: Fix missed srcu_barrier() calls before cleanup_srcu_struct()

[Zqiang]

In blk_mq_alloc_tag_set(), if blk_mq_alloc_set_map_and_rqs() fails
after some hardware queues have already been allocated, its unwind
path frees the partially allocated maps and tags through invoke the
blk_mq_free_map_and_rqs()->blk_mq_free_tags(), which issues call_srcu()
against set->tags_srcu. control then returns to blk_mq_alloc_tag_set()
and falls through to the out_cleanup_tags_srcu label, which calls
cleanup_srcu_struct() directly.

Calling cleanup_srcu_struct() while call_srcu() callbacks are still
outstanding is illegal, it triggers a WARN and tears down the
srcu_struct while the pending blk_mq_free_tags_callback() still
references it, leading to a use-after-free.

The normal teardown path in blk_mq_free_tag_set() already guards
against this by calling srcu_barrier() before cleanup_srcu_struct(),
but the error path in blk_mq_alloc_tag_set() was missing the same
barrier. this commit therefore also add srcu_barrier(&set->tags_srcu)
call so that all outstanding SRCU callbacks complete before the
srcu_struct is destroyed.

Fixes: ad0d05dbddc1 ("blk-mq: Defer freeing of tags page_list to SRCU callback")
Signed-off-by: Zqiang <qiang.zhang@linux.dev>
---
 block/blk-mq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2c850330a32b..59cd629b37d5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4924,6 +4924,7 @@ int blk_mq_alloc_tag_set(struct blk_mq_tag_set *set)
 	}
 	kfree(set->tags);
 	set->tags = NULL;
+	srcu_barrier(&set->tags_srcu);
 out_cleanup_tags_srcu:
 	cleanup_srcu_struct(&set->tags_srcu);
 out_cleanup_srcu:
-- 
2.17.1

Re: [PATCH] blk-mq: Fix missed srcu_barrier() calls before cleanup_srcu_struct()

[Zqiang]

Friendly ping :) 

Thanks
Zqiang


> 
> In blk_mq_alloc_tag_set(), if blk_mq_alloc_set_map_and_rqs() fails
> after some hardware queues have already been allocated, its unwind
> path frees the partially allocated maps and tags through invoke the
> blk_mq_free_map_and_rqs()->blk_mq_free_tags(), which issues call_srcu()
> against set->tags_srcu. control then returns to blk_mq_alloc_tag_set()
> and falls through to the out_cleanup_tags_srcu label, which calls
> cleanup_srcu_struct() directly.
> 
> Calling cleanup_srcu_struct() while call_srcu() callbacks are still
> outstanding is illegal, it triggers a WARN and tears down the
> srcu_struct while the pending blk_mq_free_tags_callback() still
> references it, leading to a use-after-free.
> 
> The normal teardown path in blk_mq_free_tag_set() already guards
> against this by calling srcu_barrier() before cleanup_srcu_struct(),
> but the error path in blk_mq_alloc_tag_set() was missing the same
> barrier. this commit therefore also add srcu_barrier(&set->tags_srcu)
> call so that all outstanding SRCU callbacks complete before the
> srcu_struct is destroyed.
> 
> Fixes: ad0d05dbddc1 ("blk-mq: Defer freeing of tags page_list to SRCU callback")
> Signed-off-by: Zqiang <qiang.zhang@linux.dev>
> ---
>  block/blk-mq.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 2c850330a32b..59cd629b37d5 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -4924,6 +4924,7 @@ int blk_mq_alloc_tag_set(struct blk_mq_tag_set *set)
>  }
>  kfree(set->tags);
>  set->tags = NULL;
> + srcu_barrier(&set->tags_srcu);
>  out_cleanup_tags_srcu:
>  cleanup_srcu_struct(&set->tags_srcu);
>  out_cleanup_srcu:
> -- 
> 2.17.1
>