[PATCH BUGFIX 0/1] block, bfq: fix bug 214503

[PATCH BUGFIX 0/1] block, bfq: fix bug 214503

[Paolo Valente]

Hi Jens,
this patch fixes bug 214503 [1]. Commit 2d52c58b9c9b ("block, bfq:
honor already-setup queue merges") has not to do with this bug, and
can be restored.

Thanks,
Paolo

Paolo Valente (1):
  block, bfq: reset last_bfqq_created on group change

 block/bfq-cgroup.c | 6 ++++++
 1 file changed, 6 insertions(+)

--
2.20.1

[PATCH BUGFIX 1/1] block, bfq: reset last_bfqq_created on group change

[Paolo Valente]

Since commit 430a67f9d616 ("block, bfq: merge bursts of newly-created
queues"), BFQ maintains a per-group pointer to the last bfq_queue
created. If such a queue, say bfqq, happens to move to a different
group, then bfqq is no more a valid last bfq_queue created for its
previous group. That pointer must then be cleared. Not resetting such
a pointer may also cause UAF, if bfqq happens to also be freed after
being moved to a different group. This commit performs this missing
reset. As such it fixes commit 430a67f9d616 ("block, bfq: merge bursts
of newly-created queues").

Such a missing reset is most likely the cause of the crash reported in [1].
With some analysis, we found that this crash was due to the
above UAF. And such UAF did go away with this commit applied [1].

Anyway, before this commit, that crash happened to be triggered in
conjunction with commit 2d52c58b9c9b ("block, bfq: honor already-setup
queue merges"). The latter was then reverted by commit ebc69e897e17
("Revert "block, bfq: honor already-setup queue merges""). Yet commit
2d52c58b9c9b ("block, bfq: honor already-setup queue merges") contains
no error related with the above UAF, and can then be restored.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=214503

Fixes: 430a67f9d616 ("block, bfq: merge bursts of newly-created queues")
Tested-by: Grzegorz Kowal <custos.mentis@gmail.com>
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/bfq-cgroup.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index e2f14508f2d6..85b8e1c3a762 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -666,6 +666,12 @@ void bfq_bfqq_move(struct bfq_data *bfqd, struct bfq_queue *bfqq,
 		bfq_put_idle_entity(bfq_entity_service_tree(entity), entity);
 	bfqg_and_blkg_put(bfqq_group(bfqq));
 
+	if (entity->parent &&
+	    entity->parent->last_bfqq_created == bfqq)
+		entity->parent->last_bfqq_created = NULL;
+	else if (bfqd->last_bfqq_created == bfqq)
+		bfqd->last_bfqq_created = NULL;
+
 	entity->parent = bfqg->my_entity;
 	entity->sched_data = &bfqg->sched_data;
 	/* pin down bfqg and its associated blkg  */
-- 
2.20.1

Re: [PATCH BUGFIX 0/1] block, bfq: fix bug 214503

[Jens Axboe]

On Fri, 15 Oct 2021 16:43:35 +0200, Paolo Valente wrote:
> this patch fixes bug 214503 [1]. Commit 2d52c58b9c9b ("block, bfq:
> honor already-setup queue merges") has not to do with this bug, and
> can be restored.
> 
> Thanks,
> Paolo
> 
> [...]

Applied, thanks!

[1/1] block, bfq: reset last_bfqq_created on group change
      commit: d29bd41428cfff9b582c248db14a47e2be8457a8

Best regards,
-- 
Jens Axboe

Re: [PATCH BUGFIX 0/1] block, bfq: fix bug 214503

[Jens Axboe]

On Fri, 15 Oct 2021 16:43:35 +0200, Paolo Valente wrote:
> this patch fixes bug 214503 [1]. Commit 2d52c58b9c9b ("block, bfq:
> honor already-setup queue merges") has not to do with this bug, and
> can be restored.
> 
> Thanks,
> Paolo
> 
> [...]

Applied, thanks!

[1/1] block, bfq: reset last_bfqq_created on group change
      (no commit info)

Best regards,
-- 
Jens Axboe