[PATCH 0/4 v4] bfq: Avoid use-after-free when moving processes between cgroups

[PATCH 0/4 v4] bfq: Avoid use-after-free when moving processes between cgroups

[Jan Kara]

Hello,

here is the third version of my patches to fix use-after-free issues in BFQ
when processes with merged queues get moved to different cgroups. The patches
have survived some beating in my test VM, but so far I fail to reproduce the
original KASAN reports so testing from people who can reproduce them is most
welcome. Kuai, can you please give these patches a run in your setup? Thanks
a lot for your help with fixing this!

Changes since v3:
* Changed handling of bfq group move to handle the case when target of the
  merge has moved.

Changes since v2:
* Improved handling of bfq queue splitting on move between cgroups
* Removed broken change to bfq_put_cooperator()

Changes since v1:
* Added fix for bfq_put_cooperator()
* Added fix to handle move between cgroups in bfq_merge_bio()

								Honza
Previous versions:
Link: http://lore.kernel.org/r/20211223171425.3551-1-jack@suse.cz # v1
Link: http://lore.kernel.org/r/20220105143037.20542-1-jack@suse.cz # v2
Link: http://lore.kernel.org/r/20220112113529.6355-1-jack@suse.cz # v3

[PATCH 1/4] bfq: Avoid false marking of bic as stably merged

[Jan Kara]

bfq_setup_cooperator() can mark bic as stably merged even though it
decides to not merge its bfqqs (when bfq_setup_merge() returns NULL).
Make sure to mark bic as stably merged only if we are really going to
merge bfqqs.

CC: stable@vger.kernel.org
Fixes: 430a67f9d616 ("block, bfq: merge bursts of newly-created queues")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/bfq-iosched.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index fec18118dc30..056399185c2f 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2762,9 +2762,12 @@ bfq_setup_cooperator(struct bfq_data *bfqd, struct bfq_queue *bfqq,
 				struct bfq_queue *new_bfqq =
 					bfq_setup_merge(bfqq, stable_merge_bfqq);
 
-				bic->stably_merged = true;
-				if (new_bfqq && new_bfqq->bic)
-					new_bfqq->bic->stably_merged = true;
+				if (new_bfqq) {
+					bic->stably_merged = true;
+					if (new_bfqq->bic)
+						new_bfqq->bic->stably_merged =
+									true;
+				}
 				return new_bfqq;
 			} else
 				return NULL;
-- 
2.31.1

[PATCH 2/4] bfq: Avoid merging queues with different parents

[Jan Kara]

It can happen that the parent of a bfqq changes between the moment we
decide two queues are worth to merge (and set bic->stable_merge_bfqq)
and the moment bfq_setup_merge() is called. This can happen e.g. because
the process submitted IO for a different cgroup and thus bfqq got
reparented. It can even happen that the bfqq we are merging with has
parent cgroup that is already offline and going to be destroyed in which
case the merge can lead to use-after-free issues such as:

BUG: KASAN: use-after-free in __bfq_deactivate_entity+0x9cb/0xa50
Read of size 8 at addr ffff88800693c0c0 by task runc:[2:INIT]/10544

CPU: 0 PID: 10544 Comm: runc:[2:INIT] Tainted: G            E     5.15.2-0.g5fb85fd-default #1 openSUSE Tumbleweed (unreleased) f1f3b891c72369aebecd2e43e4641a6358867c70
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a-rebuilt.opensuse.org 04/01/2014
Call Trace:
 <IRQ>
 dump_stack_lvl+0x46/0x5a
 print_address_description.constprop.0+0x1f/0x140
 ? __bfq_deactivate_entity+0x9cb/0xa50
 kasan_report.cold+0x7f/0x11b
 ? __bfq_deactivate_entity+0x9cb/0xa50
 __bfq_deactivate_entity+0x9cb/0xa50
 ? update_curr+0x32f/0x5d0
 bfq_deactivate_entity+0xa0/0x1d0
 bfq_del_bfqq_busy+0x28a/0x420
 ? resched_curr+0x116/0x1d0
 ? bfq_requeue_bfqq+0x70/0x70
 ? check_preempt_wakeup+0x52b/0xbc0
 __bfq_bfqq_expire+0x1a2/0x270
 bfq_bfqq_expire+0xd16/0x2160
 ? try_to_wake_up+0x4ee/0x1260
 ? bfq_end_wr_async_queues+0xe0/0xe0
 ? _raw_write_unlock_bh+0x60/0x60
 ? _raw_spin_lock_irq+0x81/0xe0
 bfq_idle_slice_timer+0x109/0x280
 ? bfq_dispatch_request+0x4870/0x4870
 __hrtimer_run_queues+0x37d/0x700
 ? enqueue_hrtimer+0x1b0/0x1b0
 ? kvm_clock_get_cycles+0xd/0x10
 ? ktime_get_update_offsets_now+0x6f/0x280
 hrtimer_interrupt+0x2c8/0x740

Fix the problem by checking that the parent of the two bfqqs we are
merging in bfq_setup_merge() is the same.

Link: https://lore.kernel.org/linux-block/20211125172809.GC19572@quack2.suse.cz/
CC: stable@vger.kernel.org
Fixes: 430a67f9d616 ("block, bfq: merge bursts of newly-created queues")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/bfq-iosched.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 056399185c2f..0da47f2ca781 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2638,6 +2638,14 @@ bfq_setup_merge(struct bfq_queue *bfqq, struct bfq_queue *new_bfqq)
 	if (process_refs == 0 || new_process_refs == 0)
 		return NULL;
 
+	/*
+	 * Make sure merged queues belong to the same parent. Parents could
+	 * have changed since the time we decided the two queues are suitable
+	 * for merging.
+	 */
+	if (new_bfqq->entity.parent != bfqq->entity.parent)
+		return NULL;
+
 	bfq_log_bfqq(bfqq->bfqd, bfqq, "scheduling merge with queue %d",
 		new_bfqq->pid);
 
-- 
2.31.1

[PATCH 3/4] bfq: Split shared queues on move between cgroups

[Jan Kara]

When bfqq is shared by multiple processes it can happen that one of the
processes gets moved to a different cgroup (or just starts submitting IO
for different cgroup). In case that happens we need to split the merged
bfqq as otherwise we will have IO for multiple cgroups in one bfqq and
we will just account IO time to wrong entities etc.

Similarly if the bfqq is scheduled to merge with another bfqq but the
merge didn't happen yet, cancel the merge as it need not be valid
anymore.

CC: stable@vger.kernel.org
Fixes: e21b7a0b9887 ("block, bfq: add full hierarchical scheduling and cgroups support")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/bfq-cgroup.c  | 25 ++++++++++++++++++++++++-
 block/bfq-iosched.c |  2 +-
 block/bfq-iosched.h |  1 +
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index 24a5c5329bcd..dbc117e00783 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -730,8 +730,31 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd,
 
 	if (sync_bfqq) {
 		entity = &sync_bfqq->entity;
-		if (entity->sched_data != &bfqg->sched_data)
+		if (entity->sched_data != &bfqg->sched_data) {
+			/*
+			 * Was the queue we use merged to a different queue?
+			 * Detach process from the queue as merge need not be
+			 * valid anymore. We cannot easily cancel the merge as
+			 * there may be other processes scheduled to this
+			 * queue.
+			 */
+			if (sync_bfqq->new_bfqq) {
+				bfq_put_cooperator(sync_bfqq);
+				bfq_release_process_ref(bfqd, sync_bfqq);
+				bic_set_bfqq(bic, NULL, 1);
+				return bfqg;
+			}
+			/*
+			 * Moving bfqq that is shared with another process?
+			 * Split the queues at the nearest occasion as the
+			 * processes can be in different cgroups now.
+			 */
+			if (bfq_bfqq_coop(sync_bfqq)) {
+				bic->stably_merged = false;
+				bfq_mark_bfqq_split_coop(sync_bfqq);
+			}
 			bfq_bfqq_move(bfqd, sync_bfqq, bfqg);
+		}
 	}
 
 	return bfqg;
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 0da47f2ca781..361d321b012a 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5184,7 +5184,7 @@ static void bfq_put_stable_ref(struct bfq_queue *bfqq)
 	bfq_put_queue(bfqq);
 }
 
-static void bfq_put_cooperator(struct bfq_queue *bfqq)
+void bfq_put_cooperator(struct bfq_queue *bfqq)
 {
 	struct bfq_queue *__bfqq, *next;
 
diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
index a73488eec8a4..6e250db2138e 100644
--- a/block/bfq-iosched.h
+++ b/block/bfq-iosched.h
@@ -976,6 +976,7 @@ void bfq_weights_tree_remove(struct bfq_data *bfqd,
 void bfq_bfqq_expire(struct bfq_data *bfqd, struct bfq_queue *bfqq,
 		     bool compensate, enum bfqq_expiration reason);
 void bfq_put_queue(struct bfq_queue *bfqq);
+void bfq_put_cooperator(struct bfq_queue *bfqq);
 void bfq_end_wr_async_queues(struct bfq_data *bfqd, struct bfq_group *bfqg);
 void bfq_release_process_ref(struct bfq_data *bfqd, struct bfq_queue *bfqq);
 void bfq_schedule_dispatch(struct bfq_data *bfqd);
-- 
2.31.1

[PATCH 4/4] bfq: Update cgroup information before merging bio

[Jan Kara]

When the process is migrated to a different cgroup (or in case of
writeback just starts submitting bios associated with a different
cgroup) bfq_merge_bio() can operate with stale cgroup information in
bic. Thus the bio can be merged to a request from a different cgroup or
it can result in merging of bfqqs for different cgroups or bfqqs of
already dead cgroups and causing possible use-after-free issues. Fix the
problem by updating cgroup information in bfq_merge_bio().

CC: stable@vger.kernel.org
Fixes: e21b7a0b9887 ("block, bfq: add full hierarchical scheduling and cgroups support")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/bfq-cgroup.c  | 40 ++++++++++++++++++++++------------------
 block/bfq-iosched.c | 11 +++++++++--
 2 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index dbc117e00783..f6f5f156b9f2 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -729,30 +729,34 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd,
 	}
 
 	if (sync_bfqq) {
-		entity = &sync_bfqq->entity;
-		if (entity->sched_data != &bfqg->sched_data) {
+		struct bfq_queue *orig_bfqq = sync_bfqq;
+
+		/* Traverse the merge chain to bfqq we will be using */
+		while (sync_bfqq->new_bfqq)
+			sync_bfqq = sync_bfqq->new_bfqq;
+		/*
+		 * Target bfqq got moved to a different cgroup or this process
+		 * started submitting bios for different cgroup?
+		 */
+		if (sync_bfqq->entity.sched_data != &bfqg->sched_data) {
 			/*
 			 * Was the queue we use merged to a different queue?
-			 * Detach process from the queue as merge need not be
-			 * valid anymore. We cannot easily cancel the merge as
-			 * there may be other processes scheduled to this
-			 * queue.
+			 * Detach process from the queue as the merge is not
+			 * valid anymore. We cannot easily just cancel the
+			 * merge (by clearing new_bfqq) as there may be other
+			 * processes using this queue and holding refs to all
+			 * queues below sync_bfqq->new_bfqq. Similarly if the
+			 * merge already happened, we need to detach from bfqq
+			 * now so that we cannot merge bio to a request from
+			 * the old cgroup.
 			 */
-			if (sync_bfqq->new_bfqq) {
-				bfq_put_cooperator(sync_bfqq);
-				bfq_release_process_ref(bfqd, sync_bfqq);
+			if (orig_bfqq != sync_bfqq || bfq_bfqq_coop(sync_bfqq)) {
+				bfq_put_cooperator(orig_bfqq);
+				bfq_release_process_ref(bfqd, orig_bfqq);
 				bic_set_bfqq(bic, NULL, 1);
 				return bfqg;
 			}
-			/*
-			 * Moving bfqq that is shared with another process?
-			 * Split the queues at the nearest occasion as the
-			 * processes can be in different cgroups now.
-			 */
-			if (bfq_bfqq_coop(sync_bfqq)) {
-				bic->stably_merged = false;
-				bfq_mark_bfqq_split_coop(sync_bfqq);
-			}
+			/* We are the only user of this bfqq, just move it */
 			bfq_bfqq_move(bfqd, sync_bfqq, bfqg);
 		}
 	}
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 361d321b012a..8a088d77a0b6 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2337,10 +2337,17 @@ static bool bfq_bio_merge(struct request_queue *q, struct bio *bio,
 
 	spin_lock_irq(&bfqd->lock);
 
-	if (bic)
+	if (bic) {
+		/*
+		 * Make sure cgroup info is uptodate for current process before
+		 * considering the merge.
+		 */
+		bfq_bic_update_cgroup(bic, bio);
+
 		bfqd->bio_bfqq = bic_to_bfqq(bic, op_is_sync(bio->bi_opf));
-	else
+	} else {
 		bfqd->bio_bfqq = NULL;
+	}
 	bfqd->bio_bic = bic;
 
 	ret = blk_mq_sched_try_merge(q, bio, nr_segs, &free);
-- 
2.31.1

Re: [PATCH 0/4 v4] bfq: Avoid use-after-free when moving processes between cgroups

[yukuai (C)]

在 2022/01/15 1:01, Jan Kara 写道:
> Hello,
> 
> here is the third version of my patches to fix use-after-free issues in BFQ
> when processes with merged queues get moved to different cgroups. The patches
> have survived some beating in my test VM, but so far I fail to reproduce the
> original KASAN reports so testing from people who can reproduce them is most
> welcome. Kuai, can you please give these patches a run in your setup? Thanks
> a lot for your help with fixing this!
> 
> Changes since v3:
> * Changed handling of bfq group move to handle the case when target of the
>    merge has moved.
Hi, Jan

The problem can still be reporduced...

Do you implement this in patch 4? If so, I don't understand how you
chieve this.

For example: 3 bfqqs were merged:
q1->q2->q3

If __bfq_bic_change_cgroup() is called with q2, the problem can be
fixed. However, if __bfq_bic_change_cgroup() is called with q3, can
the problem be fixed?

Thanks,
Kuai
> 
> Changes since v2:
> * Improved handling of bfq queue splitting on move between cgroups
> * Removed broken change to bfq_put_cooperator()
> 
> Changes since v1:
> * Added fix for bfq_put_cooperator()
> * Added fix to handle move between cgroups in bfq_merge_bio()
> 
> 								Honza
> Previous versions:
> Link: http://lore.kernel.org/r/20211223171425.3551-1-jack@suse.cz # v1
> Link: http://lore.kernel.org/r/20220105143037.20542-1-jack@suse.cz # v2
> Link: http://lore.kernel.org/r/20220112113529.6355-1-jack@suse.cz # v3
> .
> 

Re: [PATCH 0/4 v4] bfq: Avoid use-after-free when moving processes between cgroups

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 2784 bytes --]

On Mon 17-01-22 16:13:09, yukuai (C) wrote:
> 在 2022/01/15 1:01, Jan Kara 写道:
> > Hello,
> > 
> > here is the third version of my patches to fix use-after-free issues in BFQ
> > when processes with merged queues get moved to different cgroups. The patches
> > have survived some beating in my test VM, but so far I fail to reproduce the
> > original KASAN reports so testing from people who can reproduce them is most
> > welcome. Kuai, can you please give these patches a run in your setup? Thanks
> > a lot for your help with fixing this!
> > 
> > Changes since v3:
> > * Changed handling of bfq group move to handle the case when target of the
> >    merge has moved.
> Hi, Jan
> 
> The problem can still be reporduced...

Drat. Thanks for testing.

> Do you implement this in patch 4? If so, I don't understand how you
> chieve this.

Yes, patch 4 should be handling this.

> For example: 3 bfqqs were merged:
> q1->q2->q3
> 
> If __bfq_bic_change_cgroup() is called with q2, the problem can be
> fixed. However, if __bfq_bic_change_cgroup() is called with q3, can
> the problem be fixed?

Maybe I'm missing something so let's walk through your example where the
bio is submitted for a task attached to q3. Bio is submitted,
__bfq_bic_change_cgroup() is called with sync_bfqq == q3, the loop

               while (sync_bfqq->new_bfqq)
                       sync_bfqq = sync_bfqq->new_bfqq;

won't do anything. Then we check:

               if (sync_bfqq->entity.sched_data != &bfqg->sched_data) {

This should be true because the task got moved and the bio is thus now
submitted for a different cgroup. Then we get to the condition:

                       if (orig_bfqq != sync_bfqq || bfq_bfqq_coop(sync_bfqq))

orig_bfqq == sync_bfqq so that won't help but the idea was that
bfq_bfqq_coop() would trigger in this case so we detach the process from q3
(through bic_set_bfqq(bic, NULL, 1)) and create new queue in the right
cgroup. Eventually, all the processes would detach from q3 and it would get
destroyed. So why do you think this scheme will not work?

And even if q3 is not marked as coop (which should not happen when there
are other queues merged to it), we would move q3 to a cgroup for which IO
is submitted - i.e., a one which is alive.

Hum, but maybe with the above merge setup (q1->q2->q3) if
__bfq_bic_change_cgroup() gets called for q1, q3 is already moved to the root
cgroup by bfq_pd_offline() and bio is for the root cgroup, then we decide
to keep everything as is. But bfq_setup_cooperator() will return q2 and
we queue IO there and q2 may still be pointing to a dead cgroup (but q2
didn't get reparented because it was not in any of the service trees). So
perhaps attached fixup will help?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[-- Attachment #2: bfq-cgroup-fixup.patch --]
[-- Type: text/x-patch, Size: 508 bytes --]

diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index f6f5f156b9f2..3e8f0564ff0b 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -732,7 +732,7 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd,
 		struct bfq_queue *orig_bfqq = sync_bfqq;
 
 		/* Traverse the merge chain to bfqq we will be using */
-		while (sync_bfqq->new_bfqq)
+		if (sync_bfqq->new_bfqq)
 			sync_bfqq = sync_bfqq->new_bfqq;
 		/*
 		 * Target bfqq got moved to a different cgroup or this process

Re: [PATCH 0/4 v4] bfq: Avoid use-after-free when moving processes between cgroups

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 5647 bytes --]

On Mon 17-01-22 22:16:23, yukuai (C) wrote:
> 在 2022/01/17 19:45, Jan Kara 写道:
> > On Mon 17-01-22 16:13:09, yukuai (C) wrote:
> > > 在 2022/01/15 1:01, Jan Kara 写道:
> > > > Hello,
> > > > 
> > > > here is the third version of my patches to fix use-after-free issues in BFQ
> > > > when processes with merged queues get moved to different cgroups. The patches
> > > > have survived some beating in my test VM, but so far I fail to reproduce the
> > > > original KASAN reports so testing from people who can reproduce them is most
> > > > welcome. Kuai, can you please give these patches a run in your setup? Thanks
> > > > a lot for your help with fixing this!
> > > > 
> > > > Changes since v3:
> > > > * Changed handling of bfq group move to handle the case when target of the
> > > >     merge has moved.
> > > Hi, Jan
> > > 
> > > The problem can still be reporduced...
> > 
> > Drat. Thanks for testing.
> > 
> > > Do you implement this in patch 4? If so, I don't understand how you
> > > chieve this.
> > 
> > Yes, patch 4 should be handling this.
> > 
> > > For example: 3 bfqqs were merged:
> > > q1->q2->q3
> > > 
> > > If __bfq_bic_change_cgroup() is called with q2, the problem can be
> > > fixed. However, if __bfq_bic_change_cgroup() is called with q3, can
> > > the problem be fixed?
> > 
> > Maybe I'm missing something so let's walk through your example where the
> > bio is submitted for a task attached to q3. Bio is submitted,
> > __bfq_bic_change_cgroup() is called with sync_bfqq == q3, the loop
> > 
> >                 while (sync_bfqq->new_bfqq)
> >                         sync_bfqq = sync_bfqq->new_bfqq;
> > 
> > won't do anything. Then we check:
> > 
> >                 if (sync_bfqq->entity.sched_data != &bfqg->sched_data) {
> > 
> > This should be true because the task got moved and the bio is thus now
> > submitted for a different cgroup. Then we get to the condition:
> > 
> >                         if (orig_bfqq != sync_bfqq || bfq_bfqq_coop(sync_bfqq))
> > 
> > orig_bfqq == sync_bfqq so that won't help but the idea was that
> > bfq_bfqq_coop() would trigger in this case so we detach the process from q3
> > (through bic_set_bfqq(bic, NULL, 1)) and create new queue in the right
> > cgroup. Eventually, all the processes would detach from q3 and it would get
> > destroyed. So why do you think this scheme will not work?
> 
> Hi, Jan
> 
> I repoduced again with some debug info:

Thanks for your help with debugging!

> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> index f6f5f156b9f2..f62ebc4bbe56 100644
> --- a/block/bfq-cgroup.c
> +++ b/block/bfq-cgroup.c
> @@ -732,8 +732,11 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct
> bfq_data *bfqd,
>                 struct bfq_queue *orig_bfqq = sync_bfqq;
> 
>                 /* Traverse the merge chain to bfqq we will be using */
> -               while (sync_bfqq->new_bfqq)
> +               while (sync_bfqq->new_bfqq) {
> +                       printk("%s: bfqq %px, new_bfqq %px\n", __func__,
> +                                       sync_bfqq, sync_bfqq->new_bfqq);
>                         sync_bfqq = sync_bfqq->new_bfqq;
> +               }
>                 /*
>                  * Target bfqq got moved to a different cgroup or this
> process
>                  * started submitting bios for different cgroup?
> @@ -756,6 +759,8 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct
> bfq_data *bfqd,
>                                 bic_set_bfqq(bic, NULL, 1);

Maybe it would be nice to add a debug message here, saying we are detaching
the process from orig_bfqq. Just that we know that this branch executed.

>                                 return bfqg;
>                         }
> +                       printk("%s: bfqq %px move from %px to %px\n",
> __func__,
> +                               sync_bfqq, bfqq_group(sync_bfqq), bfqg);
>                         /* We are the only user of this bfqq, just move it
> */
>                         bfq_bfqq_move(bfqd, sync_bfqq, bfqg);

...

> -       if (bfqq->new_bfqq)
> +       if (bfqq->new_bfqq) {
> +               if (bfqq->entity.parent != bfqq->new_bfqq->entity.parent) {
> +                       printk("%s: bfqq %px(%px) new_bfqq %px(%px)\n",
> __func__,
> +                               bfqq, bfqq_group(bfqq), bfqq->new_bfqq,
> +                               bfqq_group(bfqq->new_bfqq));
> +                       BUG_ON(1);

OK, so I can see why this triggered. We have:

Set new_bfqq for bfqq *eec0:
[   50.207263] bfq_setup_merge: set bfqq ffff88816b16eec0 new_bfqq ffff8881133e1340 parent ffff88814380b000/ffff88814380b000
Move new_bfqq to a root cgroup:
[   50.485933] bfq_reparent_leaf_entity: bfqq ffff8881133e1340 move from ffff88814380b000 to ffff88810b1f1000
Submit bio for root cgroup through *eec0:
[   50.519910] __bfq_bic_change_cgroup: bfqq ffff88816b16eec0, new_bfqq ffff8881133e1340
__bfq_bic_change_cgroup() does nothing as the bio is for the cgroup to which
target queue belongs.
[   50.520640] bfq_setup_cooperator: bfqq ffff88816b16eec0(ffff88814380b000) new_bfqq ffff8881133e1340(ffff88810b1f1000)
The BUG_ON trips. 

So at this moment the BUG_ON was just too eager to trigger as we would be
submitting IO to a bfq queue belonging to an appropriate cgroup. But I
guess it does not make sence to keep unfinished merges over cgroup
migration and __bfq_bic_change_cgroup() should have detached task from its
bfqq anyway. Can you please try running with a new version of patch 4 which
is attached? And perhaps with your debug patch as well... Thanks!

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[-- Attachment #2: 0001-bfq-Update-cgroup-information-before-merging-bio.patch --]
[-- Type: text/x-patch, Size: 3683 bytes --]

From 2c32c74eccc94f1f5c689132423cbe5fc9616871 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Wed, 5 Jan 2022 14:21:04 +0100
Subject: [PATCH] bfq: Update cgroup information before merging bio

When the process is migrated to a different cgroup (or in case of
writeback just starts submitting bios associated with a different
cgroup) bfq_merge_bio() can operate with stale cgroup information in
bic. Thus the bio can be merged to a request from a different cgroup or
it can result in merging of bfqqs for different cgroups or bfqqs of
already dead cgroups and causing possible use-after-free issues. Fix the
problem by updating cgroup information in bfq_merge_bio().

CC: stable@vger.kernel.org
Fixes: e21b7a0b9887 ("block, bfq: add full hierarchical scheduling and cgroups support")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/bfq-cgroup.c  | 37 +++++++++++++++++++++----------------
 block/bfq-iosched.c | 11 +++++++++--
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index dbc117e00783..de4db8a0d796 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -729,30 +729,35 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct bfq_data *bfqd,
 	}
 
 	if (sync_bfqq) {
-		entity = &sync_bfqq->entity;
-		if (entity->sched_data != &bfqg->sched_data) {
+		struct bfq_queue *target_bfqq = NULL;
+
+		if (sync_bfqq->new_bfqq)
+			target_bfqq = sync_bfqq->new_bfqq;
+		/*
+		 * Target bfqq got moved to a different cgroup or this process
+		 * started submitting bios for different cgroup?
+		 */
+		if ((target_bfqq &&
+		     sync_bfqq->entity.parent != target_bfqq->entity.parent) ||
+		    sync_bfqq->entity.sched_data != &bfqg->sched_data) {
 			/*
 			 * Was the queue we use merged to a different queue?
-			 * Detach process from the queue as merge need not be
-			 * valid anymore. We cannot easily cancel the merge as
-			 * there may be other processes scheduled to this
-			 * queue.
+			 * Detach process from the queue as the merge is not
+			 * valid anymore. We cannot easily just cancel the
+			 * merge (by clearing new_bfqq) as there may be other
+			 * processes using this queue and holding refs to all
+			 * queues below sync_bfqq->new_bfqq. Similarly if the
+			 * merge already happened, we need to detach from bfqq
+			 * now so that we cannot merge bio to a request from
+			 * the old cgroup.
 			 */
-			if (sync_bfqq->new_bfqq) {
+			if (target_bfqq || bfq_bfqq_coop(sync_bfqq)) {
 				bfq_put_cooperator(sync_bfqq);
 				bfq_release_process_ref(bfqd, sync_bfqq);
 				bic_set_bfqq(bic, NULL, 1);
 				return bfqg;
 			}
-			/*
-			 * Moving bfqq that is shared with another process?
-			 * Split the queues at the nearest occasion as the
-			 * processes can be in different cgroups now.
-			 */
-			if (bfq_bfqq_coop(sync_bfqq)) {
-				bic->stably_merged = false;
-				bfq_mark_bfqq_split_coop(sync_bfqq);
-			}
+			/* We are the only user of this bfqq, just move it */
 			bfq_bfqq_move(bfqd, sync_bfqq, bfqg);
 		}
 	}
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 361d321b012a..8a088d77a0b6 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2337,10 +2337,17 @@ static bool bfq_bio_merge(struct request_queue *q, struct bio *bio,
 
 	spin_lock_irq(&bfqd->lock);
 
-	if (bic)
+	if (bic) {
+		/*
+		 * Make sure cgroup info is uptodate for current process before
+		 * considering the merge.
+		 */
+		bfq_bic_update_cgroup(bic, bio);
+
 		bfqd->bio_bfqq = bic_to_bfqq(bic, op_is_sync(bio->bi_opf));
-	else
+	} else {
 		bfqd->bio_bfqq = NULL;
+	}
 	bfqd->bio_bic = bic;
 
 	ret = blk_mq_sched_try_merge(q, bio, nr_segs, &free);
-- 
2.31.1

Re: [PATCH 0/4 v4] bfq: Avoid use-after-free when moving processes between cgroups

[Jan Kara]

On Tue 18-01-22 11:00:43, yukuai (C) wrote:
> 在 2022/01/17 23:46, Jan Kara 写道:
> > On Mon 17-01-22 22:16:23, yukuai (C) wrote:
> > > 在 2022/01/17 19:45, Jan Kara 写道:
> > > > On Mon 17-01-22 16:13:09, yukuai (C) wrote:
> > > > > 在 2022/01/15 1:01, Jan Kara 写道:
> > > > > > Hello,
> > > > > > 
> > > > > > here is the third version of my patches to fix use-after-free issues in BFQ
> > > > > > when processes with merged queues get moved to different cgroups. The patches
> > > > > > have survived some beating in my test VM, but so far I fail to reproduce the
> > > > > > original KASAN reports so testing from people who can reproduce them is most
> > > > > > welcome. Kuai, can you please give these patches a run in your setup? Thanks
> > > > > > a lot for your help with fixing this!
> > > > > > 
> > > > > > Changes since v3:
> > > > > > * Changed handling of bfq group move to handle the case when target of the
> > > > > >      merge has moved.
> > > > > Hi, Jan
> > > > > 
> > > > > The problem can still be reporduced...
> > > > 
> > > > Drat. Thanks for testing.
> > > > 
> > > > > Do you implement this in patch 4? If so, I don't understand how you
> > > > > chieve this.
> > > > 
> > > > Yes, patch 4 should be handling this.
> > > > 
> > > > > For example: 3 bfqqs were merged:
> > > > > q1->q2->q3
> > > > > 
> > > > > If __bfq_bic_change_cgroup() is called with q2, the problem can be
> > > > > fixed. However, if __bfq_bic_change_cgroup() is called with q3, can
> > > > > the problem be fixed?
> > > > 
> > > > Maybe I'm missing something so let's walk through your example where the
> > > > bio is submitted for a task attached to q3. Bio is submitted,
> > > > __bfq_bic_change_cgroup() is called with sync_bfqq == q3, the loop
> > > > 
> > > >                  while (sync_bfqq->new_bfqq)
> > > >                          sync_bfqq = sync_bfqq->new_bfqq;
> > > > 
> > > > won't do anything. Then we check:
> > > > 
> > > >                  if (sync_bfqq->entity.sched_data != &bfqg->sched_data) {
> > > > 
> > > > This should be true because the task got moved and the bio is thus now
> > > > submitted for a different cgroup. Then we get to the condition:
> > > > 
> > > >                          if (orig_bfqq != sync_bfqq || bfq_bfqq_coop(sync_bfqq))
> > > > 
> > > > orig_bfqq == sync_bfqq so that won't help but the idea was that
> > > > bfq_bfqq_coop() would trigger in this case so we detach the process from q3
> > > > (through bic_set_bfqq(bic, NULL, 1)) and create new queue in the right
> > > > cgroup. Eventually, all the processes would detach from q3 and it would get
> > > > destroyed. So why do you think this scheme will not work?
> > > 
> > > Hi, Jan
> > > 
> > > I repoduced again with some debug info:
> > 
> > Thanks for your help with debugging!
> > 
> > > diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> > > index f6f5f156b9f2..f62ebc4bbe56 100644
> > > --- a/block/bfq-cgroup.c
> > > +++ b/block/bfq-cgroup.c
> > > @@ -732,8 +732,11 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct
> > > bfq_data *bfqd,
> > >                  struct bfq_queue *orig_bfqq = sync_bfqq;
> > > 
> > >                  /* Traverse the merge chain to bfqq we will be using */
> > > -               while (sync_bfqq->new_bfqq)
> > > +               while (sync_bfqq->new_bfqq) {
> > > +                       printk("%s: bfqq %px, new_bfqq %px\n", __func__,
> > > +                                       sync_bfqq, sync_bfqq->new_bfqq);
> > >                          sync_bfqq = sync_bfqq->new_bfqq;
> > > +               }
> > >                  /*
> > >                   * Target bfqq got moved to a different cgroup or this
> > > process
> > >                   * started submitting bios for different cgroup?
> > > @@ -756,6 +759,8 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct
> > > bfq_data *bfqd,
> > >                                  bic_set_bfqq(bic, NULL, 1);
> > 
> > Maybe it would be nice to add a debug message here, saying we are detaching
> > the process from orig_bfqq. Just that we know that this branch executed.
> > 
> > >                                  return bfqg;
> > >                          }
> > > +                       printk("%s: bfqq %px move from %px to %px\n",
> > > __func__,
> > > +                               sync_bfqq, bfqq_group(sync_bfqq), bfqg);
> > >                          /* We are the only user of this bfqq, just move it
> > > */
> > >                          bfq_bfqq_move(bfqd, sync_bfqq, bfqg);
> > 
> > ...
> > 
> > > -       if (bfqq->new_bfqq)
> > > +       if (bfqq->new_bfqq) {
> > > +               if (bfqq->entity.parent != bfqq->new_bfqq->entity.parent) {
> > > +                       printk("%s: bfqq %px(%px) new_bfqq %px(%px)\n",
> > > __func__,
> > > +                               bfqq, bfqq_group(bfqq), bfqq->new_bfqq,
> > > +                               bfqq_group(bfqq->new_bfqq));
> > > +                       BUG_ON(1);
> > 
> > OK, so I can see why this triggered. We have:
> > 
> > Set new_bfqq for bfqq *eec0:
> > [   50.207263] bfq_setup_merge: set bfqq ffff88816b16eec0 new_bfqq ffff8881133e1340 parent ffff88814380b000/ffff88814380b000
> > Move new_bfqq to a root cgroup:
> > [   50.485933] bfq_reparent_leaf_entity: bfqq ffff8881133e1340 move from ffff88814380b000 to ffff88810b1f1000
> > Submit bio for root cgroup through *eec0:
> > [   50.519910] __bfq_bic_change_cgroup: bfqq ffff88816b16eec0, new_bfqq ffff8881133e1340
> > __bfq_bic_change_cgroup() does nothing as the bio is for the cgroup to which
> > target queue belongs.
> > [   50.520640] bfq_setup_cooperator: bfqq ffff88816b16eec0(ffff88814380b000) new_bfqq ffff8881133e1340(ffff88810b1f1000)
> > The BUG_ON trips.
> > 
> > So at this moment the BUG_ON was just too eager to trigger as we would be
> > submitting IO to a bfq queue belonging to an appropriate cgroup. But I
> > guess it does not make sence to keep unfinished merges over cgroup
> > migration and __bfq_bic_change_cgroup() should have detached task from its
> > bfqq anyway. Can you please try running with a new version of patch 4 which
> > is attached? And perhaps with your debug patch as well... Thanks!
> 
> Hi
> 
> I reporduced again with the following:
> 
> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> index f6f5f156b9f2..1afb9127ca84 100644
> --- a/block/bfq-cgroup.c
> +++ b/block/bfq-cgroup.c
> @@ -732,8 +732,11 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct
> bfq_data *bfqd,
>                 struct bfq_queue *orig_bfqq = sync_bfqq;
> 
>                 /* Traverse the merge chain to bfqq we will be using */
> -               while (sync_bfqq->new_bfqq)
> +               if (sync_bfqq->new_bfqq) {
> +                       printk("%s: bfqq %px, new_bfqq %px\n", __func__,
> +                                       sync_bfqq, sync_bfqq->new_bfqq);
>                         sync_bfqq = sync_bfqq->new_bfqq;
> +               }
>                 /*
>                  * Target bfqq got moved to a different cgroup or this
> process
>                  * started submitting bios for different cgroup?
> @@ -754,8 +757,11 @@ static struct bfq_group *__bfq_bic_change_cgroup(struct
> bfq_data *bfqd,
>                                 bfq_put_cooperator(orig_bfqq);
>                                 bfq_release_process_ref(bfqd, orig_bfqq);
>                                 bic_set_bfqq(bic, NULL, 1);
> +                               printk("%s: bfqq %px detached\n", __func__,
> orig_bfqq);
>                                 return bfqg;
>                         }
> +                       printk("%s: bfqq %px move from %px to %px\n",
> __func__,
> +                               sync_bfqq, bfqq_group(sync_bfqq), bfqg);
>                         /* We are the only user of this bfqq, just move it
> */
>                         bfq_bfqq_move(bfqd, sync_bfqq, bfqg);
>                 }
> @@ -875,7 +881,10 @@ static void bfq_reparent_leaf_entity(struct bfq_data
> *bfqd,
>         }
> 
>         bfqq = bfq_entity_to_bfqq(child_entity);
> +       printk("%s: bfqq %px move from %px to %px\n", __func__,
> +                       bfqq, bfqq_group(bfqq), bfqd->root_group);
>         bfq_bfqq_move(bfqd, bfqq, bfqd->root_group);
> +
>  }
> 
>  /**
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 07be51bc229b..c6b439df28ad 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -2797,6 +2797,8 @@ bfq_setup_merge(struct bfq_queue *bfqq, struct
> bfq_queue *new_bfqq)
>          * are likely to increase the throughput.
>          */
>         bfqq->new_bfqq = new_bfqq;
> +       printk("%s: set bfqq %px new_bfqq %px parent %px/%px \n", __func__,
> bfqq,
> +                       new_bfqq, bfqq_group(bfqq), bfqq_group(new_bfqq));
>         new_bfqq->ref += process_refs;
>         return new_bfqq;
>  }
> @@ -2963,8 +2965,16 @@ bfq_setup_cooperator(struct bfq_data *bfqd, struct
> bfq_queue *bfqq,
>         if (bfq_too_late_for_merging(bfqq))
>                 return NULL;
> 
> -       if (bfqq->new_bfqq)
> +       if (bfqq->new_bfqq) {
> +               if (bfqq->entity.parent != bfqq->new_bfqq->entity.parent &&
> +                   bfqq_group(bfqq->new_bfqq) != bfqd->root_group) {
> +                       printk("%s: bfqq %px(%px) new_bfqq %px(%px)\n",
> __func__,
> +                               bfqq, bfqq_group(bfqq), bfqq->new_bfqq,
> +                               bfqq_group(bfqq->new_bfqq));
> +                       BUG_ON(1);
> +               }
>                 return bfqq->new_bfqq;
> +       }
> 
>         if (!io_struct || unlikely(bfqq == &bfqd->oom_bfqq))
>                 return NULL;
> @@ -6928,6 +6938,8 @@ static void __bfq_put_async_bfqq(struct bfq_data
> *bfqd,
> 
>         bfq_log(bfqd, "put_async_bfqq: %p", bfqq);
>         if (bfqq) {
> +               printk("%s: bfqq %px move from %px to %px\n", __func__,
> +                               bfqq, bfqq_group(bfqq), bfqd->root_group);
>                 bfq_bfqq_move(bfqd, bfqq, bfqd->root_group);
> 
>                 bfq_log_bfqq(bfqd, bfqq, "put_async_bfqq: putting %p, %d",

This was not exactly what I wanted but now I've realized I've sent you a
wrong patch. My fault and thanks for testing!

> The result is attached, one thing that is weird: the uaf is triggered
> before the BUG_ON.

Yes, that is interesting. The BUG_ON at the end indeed shows a case where
there was a longer chain of merges bfq_pd_offline() moved some of the
queues from the merge chain to the root cgroup but not all. The state was

ffff888169d57440 -> ffff88810b593180 -> ffff88812cfc9340
(root)              (root)              (orig cgroup)

Then something strange happened:

[   92.692545] __bfq_bic_change_cgroup: bfqq ffff888169d57440, new_bfqq ffff88810b593180
[   92.693374] bfq_setup_cooperator: bfqq ffff88810b593180(ffff88816d167000) new_bfqq ffff88812cfc9340(ffff88817ab36000)

We can see __bfq_bic_change_cgroup() got called for queue ffff888169d57440
but then not for ffff88810b593180 while bfq_setup_cooperator() got called
for ffff88810b593180. Not sure what happened inside the BFQ. Anyway, the
presence of these merge chains and the fact that bfq_pd_offline() can move
arbitrary subset to different cgroup shows that we should be more careful.
I've changed __bfq_bic_change_cgroup() to check the whole merge chain and
detach from bfqq if anything does not match. I'll send it as a new
revision. Can you please test it? Thanks!

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 2/4] bfq: Avoid merging queues with different parents

[Jan Kara]

It can happen that the parent of a bfqq changes between the moment we
decide two queues are worth to merge (and set bic->stable_merge_bfqq)
and the moment bfq_setup_merge() is called. This can happen e.g. because
the process submitted IO for a different cgroup and thus bfqq got
reparented. It can even happen that the bfqq we are merging with has
parent cgroup that is already offline and going to be destroyed in which
case the merge can lead to use-after-free issues such as:

BUG: KASAN: use-after-free in __bfq_deactivate_entity+0x9cb/0xa50
Read of size 8 at addr ffff88800693c0c0 by task runc:[2:INIT]/10544

CPU: 0 PID: 10544 Comm: runc:[2:INIT] Tainted: G            E     5.15.2-0.g5fb85fd-default #1 openSUSE Tumbleweed (unreleased) f1f3b891c72369aebecd2e43e4641a6358867c70
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a-rebuilt.opensuse.org 04/01/2014
Call Trace:
 <IRQ>
 dump_stack_lvl+0x46/0x5a
 print_address_description.constprop.0+0x1f/0x140
 ? __bfq_deactivate_entity+0x9cb/0xa50
 kasan_report.cold+0x7f/0x11b
 ? __bfq_deactivate_entity+0x9cb/0xa50
 __bfq_deactivate_entity+0x9cb/0xa50
 ? update_curr+0x32f/0x5d0
 bfq_deactivate_entity+0xa0/0x1d0
 bfq_del_bfqq_busy+0x28a/0x420
 ? resched_curr+0x116/0x1d0
 ? bfq_requeue_bfqq+0x70/0x70
 ? check_preempt_wakeup+0x52b/0xbc0
 __bfq_bfqq_expire+0x1a2/0x270
 bfq_bfqq_expire+0xd16/0x2160
 ? try_to_wake_up+0x4ee/0x1260
 ? bfq_end_wr_async_queues+0xe0/0xe0
 ? _raw_write_unlock_bh+0x60/0x60
 ? _raw_spin_lock_irq+0x81/0xe0
 bfq_idle_slice_timer+0x109/0x280
 ? bfq_dispatch_request+0x4870/0x4870
 __hrtimer_run_queues+0x37d/0x700
 ? enqueue_hrtimer+0x1b0/0x1b0
 ? kvm_clock_get_cycles+0xd/0x10
 ? ktime_get_update_offsets_now+0x6f/0x280
 hrtimer_interrupt+0x2c8/0x740

Fix the problem by checking that the parent of the two bfqqs we are
merging in bfq_setup_merge() is the same.

Link: https://lore.kernel.org/linux-block/20211125172809.GC19572@quack2.suse.cz/
CC: stable@vger.kernel.org
Fixes: 430a67f9d616 ("block, bfq: merge bursts of newly-created queues")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/bfq-iosched.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 056399185c2f..0da47f2ca781 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2638,6 +2638,14 @@ bfq_setup_merge(struct bfq_queue *bfqq, struct bfq_queue *new_bfqq)
 	if (process_refs == 0 || new_process_refs == 0)
 		return NULL;
 
+	/*
+	 * Make sure merged queues belong to the same parent. Parents could
+	 * have changed since the time we decided the two queues are suitable
+	 * for merging.
+	 */
+	if (new_bfqq->entity.parent != bfqq->entity.parent)
+		return NULL;
+
 	bfq_log_bfqq(bfqq->bfqd, bfqq, "scheduling merge with queue %d",
 		new_bfqq->pid);
 
-- 
2.31.1

[PATCH 2/4] bfq: Avoid merging queues with different parents

[Jan Kara]

It can happen that the parent of a bfqq changes between the moment we
decide two queues are worth to merge (and set bic->stable_merge_bfqq)
and the moment bfq_setup_merge() is called. This can happen e.g. because
the process submitted IO for a different cgroup and thus bfqq got
reparented. It can even happen that the bfqq we are merging with has
parent cgroup that is already offline and going to be destroyed in which
case the merge can lead to use-after-free issues such as:

BUG: KASAN: use-after-free in __bfq_deactivate_entity+0x9cb/0xa50
Read of size 8 at addr ffff88800693c0c0 by task runc:[2:INIT]/10544

CPU: 0 PID: 10544 Comm: runc:[2:INIT] Tainted: G            E     5.15.2-0.g5fb85fd-default #1 openSUSE Tumbleweed (unreleased) f1f3b891c72369aebecd2e43e4641a6358867c70
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a-rebuilt.opensuse.org 04/01/2014
Call Trace:
 <IRQ>
 dump_stack_lvl+0x46/0x5a
 print_address_description.constprop.0+0x1f/0x140
 ? __bfq_deactivate_entity+0x9cb/0xa50
 kasan_report.cold+0x7f/0x11b
 ? __bfq_deactivate_entity+0x9cb/0xa50
 __bfq_deactivate_entity+0x9cb/0xa50
 ? update_curr+0x32f/0x5d0
 bfq_deactivate_entity+0xa0/0x1d0
 bfq_del_bfqq_busy+0x28a/0x420
 ? resched_curr+0x116/0x1d0
 ? bfq_requeue_bfqq+0x70/0x70
 ? check_preempt_wakeup+0x52b/0xbc0
 __bfq_bfqq_expire+0x1a2/0x270
 bfq_bfqq_expire+0xd16/0x2160
 ? try_to_wake_up+0x4ee/0x1260
 ? bfq_end_wr_async_queues+0xe0/0xe0
 ? _raw_write_unlock_bh+0x60/0x60
 ? _raw_spin_lock_irq+0x81/0xe0
 bfq_idle_slice_timer+0x109/0x280
 ? bfq_dispatch_request+0x4870/0x4870
 __hrtimer_run_queues+0x37d/0x700
 ? enqueue_hrtimer+0x1b0/0x1b0
 ? kvm_clock_get_cycles+0xd/0x10
 ? ktime_get_update_offsets_now+0x6f/0x280
 hrtimer_interrupt+0x2c8/0x740

Fix the problem by checking that the parent of the two bfqqs we are
merging in bfq_setup_merge() is the same.

Link: https://lore.kernel.org/linux-block/20211125172809.GC19572@quack2.suse.cz/
CC: stable@vger.kernel.org
Fixes: 430a67f9d616 ("block, bfq: merge bursts of newly-created queues")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/bfq-iosched.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 056399185c2f..0da47f2ca781 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2638,6 +2638,14 @@ bfq_setup_merge(struct bfq_queue *bfqq, struct bfq_queue *new_bfqq)
 	if (process_refs == 0 || new_process_refs == 0)
 		return NULL;
 
+	/*
+	 * Make sure merged queues belong to the same parent. Parents could
+	 * have changed since the time we decided the two queues are suitable
+	 * for merging.
+	 */
+	if (new_bfqq->entity.parent != bfqq->entity.parent)
+		return NULL;
+
 	bfq_log_bfqq(bfqq->bfqd, bfqq, "scheduling merge with queue %d",
 		new_bfqq->pid);
 
-- 
2.31.1