Re: [bug report] BUG: KASAN: slab-use-after-free in bfq_setup_cooperator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Jan

在 2023/03/07 18:20, Jan Kara 写道:
On Tue 07-03-23 17:36:19, Yu Kuai wrote:
Hi,

在 2023/03/07 17:13, Shinichiro Kawasaki 写道:
On Mar 07, 2023 / 16:57, Yu Kuai wrote:
[...]
Thanks for the report, can you help to provide the result of add2line of
following?

bfq_setup_cooperator+0x120b/0x1650
bfq_setup_cooperator+0xa41/0x1650

That will help to locate the problem.

Hi, Yu thanks for looking into this. Here are the faddr2line outputs:

$ ./scripts/faddr2line vmlinux bfq_setup_cooperator+0x120b/0x1650
bfq_setup_cooperator+0x120b/0x1650:
bfqq_process_refs at block/bfq-iosched.c:1200
(inlined by) bfq_setup_stable_merge at block/bfq-iosched.c:2855
(inlined by) bfq_setup_cooperator at block/bfq-iosched.c:2941

$ ./scripts/faddr2line vmlinux bfq_setup_cooperator+0xa41/0x1650
bfq_setup_cooperator+0xa41/0x1650:
bfq_setup_cooperator at block/bfq-iosched.c:2939


So, after a quick look at the code, the difference is that fd571df0ac5b
changes the logic when bfqq_proces_refs(stable_merge_bfqq) is 0.

I think perhaps following patch can work, at least 'stable_merge_bfqq'
won't be accessed after bfq_put_stable_ref() in this case:

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 8a8d4441519c..881f74b3a556 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2856,6 +2856,11 @@ bfq_setup_stable_merge(struct bfq_data *bfqd, struct
bfq_queue *bfqq,
                            bfqq_process_refs(stable_merge_bfqq));
         struct bfq_queue *new_bfqq;

+       /* deschedule stable merge, because done or aborted here */
+       bfq_put_stable_ref(stable_merge_bfqq);
+
+       bfqq_data->stable_merge_bfqq = NULL;
+

I don't think this is going to help. stable_merge_bfqq is used just a few
lines below again in bfq_setup_merge(). The problem really is that the
reference from stable merge can be the last one keeping bfqq alive so in
bfq_setup_cooperator() we need to see if stable_merge_bfqq still has
process references (and cancel the merge if not) before dropping our ref.

I was thinking that bfq_setup_merge() will only be called if 'proc_ref'
is not 0, which means that stable_merge_bfqq won't be freed.

So rather doing something like:

		bfqq_data->stable_merge_bfqq = NULL;
		new_bfqq = bfq_setup_stable_merge(bfqd, bfqq,
						  stable_merge_bfqq, bfqq_data);
		bfq_put_stable_ref(stable_merge_bfqq);
		return new_bfqq;

should work in bfq_setup_cooperator().

Yes, this will work.

Thanks,
Kuai

								Honza





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux