* [PATCH] block: fix bio-allocation from per-cpu cache
[not found] <CGME20221027101534epcas5p3b8335c05a1003531f1a4488dc27f27ee@epcas5p3.samsung.com>
@ 2022-10-27 10:04 ` Kanchan Joshi
2022-10-27 10:38 ` Pavel Begunkov
2022-10-27 20:44 ` Jens Axboe
0 siblings, 2 replies; 9+ messages in thread
From: Kanchan Joshi @ 2022-10-27 10:04 UTC (permalink / raw)
To: axboe, asml.silence, linux-block; +Cc: Kanchan Joshi
If cache does not have any entry, make sure to detect that and return
failure. Otherwise this leads to null pointer dereference.
Fixes: 13a184e26965 ("block/bio: add pcpu caching for non-polling bio_put")
Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
---
Can be reproduced by:
fio -direct=1 -iodepth=1 -rw=randread -ioengine=io_uring -bs=4k -numjobs=1 -size=4k -filename=/dev/nvme0n1 -hipri=1 -name=block
BUG: KASAN: null-ptr-deref in bio_alloc_bioset.cold+0x2a/0x16a
Read of size 8 at addr 0000000000000000 by task fio/1835
CPU: 5 PID: 1835 Comm: fio Not tainted 6.1.0-rc2+ #226
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g
Call Trace:
<TASK>
dump_stack_lvl+0x34/0x48
print_report+0x490/0x4a1
? __virt_addr_valid+0x28/0x140
? bio_alloc_bioset.cold+0x2a/0x16a
kasan_report+0xb3/0x130
? bio_alloc_bioset.cold+0x2a/0x16a
bio_alloc_bioset.cold+0x2a/0x16a
? bvec_alloc+0xf0/0xf0
? iov_iter_is_aligned+0x130/0x2c0
blkdev_direct_IO.part.0+0x16a/0x8d0
block/bio.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 8f624ffaf3d0..66f088bb3736 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -439,13 +439,14 @@ static struct bio *bio_alloc_percpu_cache(struct block_device *bdev,
cache = per_cpu_ptr(bs->cache, get_cpu());
if (!cache->free_list &&
- READ_ONCE(cache->nr_irq) >= ALLOC_CACHE_THRESHOLD) {
+ READ_ONCE(cache->nr_irq) >= ALLOC_CACHE_THRESHOLD)
bio_alloc_irq_cache_splice(cache);
- if (!cache->free_list) {
- put_cpu();
- return NULL;
- }
+
+ if (!cache->free_list) {
+ put_cpu();
+ return NULL;
}
+
bio = cache->free_list;
cache->free_list = bio->bi_next;
cache->nr--;
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] block: fix bio-allocation from per-cpu cache
2022-10-27 10:04 ` [PATCH] block: fix bio-allocation from per-cpu cache Kanchan Joshi
@ 2022-10-27 10:38 ` Pavel Begunkov
2022-10-27 10:49 ` Kanchan Joshi
2022-10-27 20:44 ` Jens Axboe
1 sibling, 1 reply; 9+ messages in thread
From: Pavel Begunkov @ 2022-10-27 10:38 UTC (permalink / raw)
To: Kanchan Joshi, axboe, linux-block
On 10/27/22 11:04, Kanchan Joshi wrote:
> If cache does not have any entry, make sure to detect that and return
> failure. Otherwise this leads to null pointer dereference.
Damn, it was done right in v2
https://lore.kernel.org/all/9fd04486d972c1f3ef273fa26b4b6bf51a5e4270.1666122465.git.asml.silence@gmail.com/
Perhaps I based v3 on a wrong version. Thanks
> Fixes: 13a184e26965 ("block/bio: add pcpu caching for non-polling bio_put")
> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
> ---
> Can be reproduced by:
> fio -direct=1 -iodepth=1 -rw=randread -ioengine=io_uring -bs=4k -numjobs=1 -size=4k -filename=/dev/nvme0n1 -hipri=1 -name=block
>
> BUG: KASAN: null-ptr-deref in bio_alloc_bioset.cold+0x2a/0x16a
> Read of size 8 at addr 0000000000000000 by task fio/1835
>
> CPU: 5 PID: 1835 Comm: fio Not tainted 6.1.0-rc2+ #226
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g
> Call Trace:
> <TASK>
> dump_stack_lvl+0x34/0x48
> print_report+0x490/0x4a1
> ? __virt_addr_valid+0x28/0x140
> ? bio_alloc_bioset.cold+0x2a/0x16a
> kasan_report+0xb3/0x130
> ? bio_alloc_bioset.cold+0x2a/0x16a
> bio_alloc_bioset.cold+0x2a/0x16a
> ? bvec_alloc+0xf0/0xf0
> ? iov_iter_is_aligned+0x130/0x2c0
> blkdev_direct_IO.part.0+0x16a/0x8d0
>
> block/bio.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 8f624ffaf3d0..66f088bb3736 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -439,13 +439,14 @@ static struct bio *bio_alloc_percpu_cache(struct block_device *bdev,
>
> cache = per_cpu_ptr(bs->cache, get_cpu());
> if (!cache->free_list &&
> - READ_ONCE(cache->nr_irq) >= ALLOC_CACHE_THRESHOLD) {
> + READ_ONCE(cache->nr_irq) >= ALLOC_CACHE_THRESHOLD)
> bio_alloc_irq_cache_splice(cache);
> - if (!cache->free_list) {
> - put_cpu();
> - return NULL;
> - }
> +
> + if (!cache->free_list) {
Let's nest it under the other "if (!cache->free_list)"
> + put_cpu();
> + return NULL;
> }
> +
> bio = cache->free_list;
> cache->free_list = bio->bi_next;
> cache->nr--;
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] block: fix bio-allocation from per-cpu cache
2022-10-27 10:38 ` Pavel Begunkov
@ 2022-10-27 10:49 ` Kanchan Joshi
2022-10-27 11:46 ` Pavel Begunkov
0 siblings, 1 reply; 9+ messages in thread
From: Kanchan Joshi @ 2022-10-27 10:49 UTC (permalink / raw)
To: Pavel Begunkov; +Cc: axboe, linux-block
[-- Attachment #1: Type: text/plain, Size: 2249 bytes --]
On Thu, Oct 27, 2022 at 11:38:50AM +0100, Pavel Begunkov wrote:
>On 10/27/22 11:04, Kanchan Joshi wrote:
>>If cache does not have any entry, make sure to detect that and return
>>failure. Otherwise this leads to null pointer dereference.
>
>Damn, it was done right in v2
>
>https://lore.kernel.org/all/9fd04486d972c1f3ef273fa26b4b6bf51a5e4270.1666122465.git.asml.silence@gmail.com/
>
>Perhaps I based v3 on a wrong version. Thanks
>
>
>>Fixes: 13a184e26965 ("block/bio: add pcpu caching for non-polling bio_put")
>>Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
>>---
>>Can be reproduced by:
>>fio -direct=1 -iodepth=1 -rw=randread -ioengine=io_uring -bs=4k -numjobs=1 -size=4k -filename=/dev/nvme0n1 -hipri=1 -name=block
>>
>>BUG: KASAN: null-ptr-deref in bio_alloc_bioset.cold+0x2a/0x16a
>>Read of size 8 at addr 0000000000000000 by task fio/1835
>>
>>CPU: 5 PID: 1835 Comm: fio Not tainted 6.1.0-rc2+ #226
>>Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g
>>Call Trace:
>> <TASK>
>> dump_stack_lvl+0x34/0x48
>> print_report+0x490/0x4a1
>> ? __virt_addr_valid+0x28/0x140
>> ? bio_alloc_bioset.cold+0x2a/0x16a
>> kasan_report+0xb3/0x130
>> ? bio_alloc_bioset.cold+0x2a/0x16a
>> bio_alloc_bioset.cold+0x2a/0x16a
>> ? bvec_alloc+0xf0/0xf0
>> ? iov_iter_is_aligned+0x130/0x2c0
>> blkdev_direct_IO.part.0+0x16a/0x8d0
>>
>> block/bio.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>>diff --git a/block/bio.c b/block/bio.c
>>index 8f624ffaf3d0..66f088bb3736 100644
>>--- a/block/bio.c
>>+++ b/block/bio.c
>>@@ -439,13 +439,14 @@ static struct bio *bio_alloc_percpu_cache(struct block_device *bdev,
>> cache = per_cpu_ptr(bs->cache, get_cpu());
>> if (!cache->free_list &&
>>- READ_ONCE(cache->nr_irq) >= ALLOC_CACHE_THRESHOLD) {
>>+ READ_ONCE(cache->nr_irq) >= ALLOC_CACHE_THRESHOLD)
>> bio_alloc_irq_cache_splice(cache);
>>- if (!cache->free_list) {
>>- put_cpu();
>>- return NULL;
>>- }
>>+
>>+ if (!cache->free_list) {
>
>Let's nest it under the other "if (!cache->free_list)"
Not sure if I got you. It was under that if condition earlier, and that
part causes trouble.
What you wrote in v2 is another way, but there also we have two checks
on cache->free_list.
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] block: fix bio-allocation from per-cpu cache
2022-10-27 10:49 ` Kanchan Joshi
@ 2022-10-27 11:46 ` Pavel Begunkov
0 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-10-27 11:46 UTC (permalink / raw)
To: Kanchan Joshi; +Cc: axboe, linux-block
On 10/27/22 11:49, Kanchan Joshi wrote:
> On Thu, Oct 27, 2022 at 11:38:50AM +0100, Pavel Begunkov wrote:
>> On 10/27/22 11:04, Kanchan Joshi wrote:
>>> If cache does not have any entry, make sure to detect that and return
>>> failure. Otherwise this leads to null pointer dereference.
>>
>> Damn, it was done right in v2
>>
>> https://lore.kernel.org/all/9fd04486d972c1f3ef273fa26b4b6bf51a5e4270.1666122465.git.asml.silence@gmail.com/
>>
>> Perhaps I based v3 on a wrong version. Thanks
>>
>>
>>> Fixes: 13a184e26965 ("block/bio: add pcpu caching for non-polling bio_put")
>>> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
>>> ---
>>> Can be reproduced by:
>>> fio -direct=1 -iodepth=1 -rw=randread -ioengine=io_uring -bs=4k -numjobs=1 -size=4k -filename=/dev/nvme0n1 -hipri=1 -name=block
>>>
>>> BUG: KASAN: null-ptr-deref in bio_alloc_bioset.cold+0x2a/0x16a
>>> Read of size 8 at addr 0000000000000000 by task fio/1835
>>>
>>> CPU: 5 PID: 1835 Comm: fio Not tainted 6.1.0-rc2+ #226
>>> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g
>>> Call Trace:
>>> <TASK>
>>> dump_stack_lvl+0x34/0x48
>>> print_report+0x490/0x4a1
>>> ? __virt_addr_valid+0x28/0x140
>>> ? bio_alloc_bioset.cold+0x2a/0x16a
>>> kasan_report+0xb3/0x130
>>> ? bio_alloc_bioset.cold+0x2a/0x16a
>>> bio_alloc_bioset.cold+0x2a/0x16a
>>> ? bvec_alloc+0xf0/0xf0
>>> ? iov_iter_is_aligned+0x130/0x2c0
>>> blkdev_direct_IO.part.0+0x16a/0x8d0
>>>
>>> block/bio.c | 11 ++++++-----
>>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/block/bio.c b/block/bio.c
>>> index 8f624ffaf3d0..66f088bb3736 100644
>>> --- a/block/bio.c
>>> +++ b/block/bio.c
>>> @@ -439,13 +439,14 @@ static struct bio *bio_alloc_percpu_cache(struct block_device *bdev,
>>> cache = per_cpu_ptr(bs->cache, get_cpu());
>>> if (!cache->free_list &&
>>> - READ_ONCE(cache->nr_irq) >= ALLOC_CACHE_THRESHOLD) {
>>> + READ_ONCE(cache->nr_irq) >= ALLOC_CACHE_THRESHOLD)
>>> bio_alloc_irq_cache_splice(cache);
>>> - if (!cache->free_list) {
>>> - put_cpu();
>>> - return NULL;
>>> - }
>>> +
>>> + if (!cache->free_list) {
>>
>> Let's nest it under the other "if (!cache->free_list)"
>
> Not sure if I got you. It was under that if condition earlier, and that
> part causes trouble.
Under the free_list check specifically, the threshold would also
go in a separate if,
> What you wrote in v2 is another way, but there also we have two checks
> on cache->free_list.
Your version:
if (cache_empty())
if (check_threshold())
try_replenish_cache(); // splice
if (cache_empty()) // still empty
return NULL;
vs v2:
if (cache_empty()) {
if (check_threshold())
try_replenish_cache(); // splice
if (cache_empty()) // still empty
return NULL;
}
But on the other hand compilers should be smart enough to
optimise repeated checks when the cache already have requests,
so there should be no real difference.
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] block: fix bio-allocation from per-cpu cache
2022-10-27 10:04 ` [PATCH] block: fix bio-allocation from per-cpu cache Kanchan Joshi
2022-10-27 10:38 ` Pavel Begunkov
@ 2022-10-27 20:44 ` Jens Axboe
2022-10-27 20:45 ` Pavel Begunkov
1 sibling, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2022-10-27 20:44 UTC (permalink / raw)
To: Kanchan Joshi, asml.silence, linux-block
On 10/27/22 4:04 AM, Kanchan Joshi wrote:
> If cache does not have any entry, make sure to detect that and return
> failure. Otherwise this leads to null pointer dereference.
>
> Fixes: 13a184e26965 ("block/bio: add pcpu caching for non-polling bio_put")
> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
> ---
> Can be reproduced by:
> fio -direct=1 -iodepth=1 -rw=randread -ioengine=io_uring -bs=4k -numjobs=1 -size=4k -filename=/dev/nvme0n1 -hipri=1 -name=block
>
> BUG: KASAN: null-ptr-deref in bio_alloc_bioset.cold+0x2a/0x16a
> Read of size 8 at addr 0000000000000000 by task fio/1835
>
> CPU: 5 PID: 1835 Comm: fio Not tainted 6.1.0-rc2+ #226
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g
> Call Trace:
> <TASK>
> dump_stack_lvl+0x34/0x48
> print_report+0x490/0x4a1
> ? __virt_addr_valid+0x28/0x140
> ? bio_alloc_bioset.cold+0x2a/0x16a
> kasan_report+0xb3/0x130
> ? bio_alloc_bioset.cold+0x2a/0x16a
> bio_alloc_bioset.cold+0x2a/0x16a
> ? bvec_alloc+0xf0/0xf0
> ? iov_iter_is_aligned+0x130/0x2c0
> blkdev_direct_IO.part.0+0x16a/0x8d0
Was going to apply this, but after running some testing, it does
fix the initial crash but I still get weird corruption crashes
with the series it's fixing.
Pavel, I'm going to drop this series for now.
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] block: fix bio-allocation from per-cpu cache
2022-10-27 20:44 ` Jens Axboe
@ 2022-10-27 20:45 ` Pavel Begunkov
2022-10-27 20:50 ` Pavel Begunkov
2022-10-27 20:52 ` Jens Axboe
0 siblings, 2 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-10-27 20:45 UTC (permalink / raw)
To: Jens Axboe, Kanchan Joshi, linux-block
On 10/27/22 21:44, Jens Axboe wrote:
> On 10/27/22 4:04 AM, Kanchan Joshi wrote:
>> If cache does not have any entry, make sure to detect that and return
>> failure. Otherwise this leads to null pointer dereference.
>>
>> Fixes: 13a184e26965 ("block/bio: add pcpu caching for non-polling bio_put")
>> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
>> ---
>> Can be reproduced by:
>> fio -direct=1 -iodepth=1 -rw=randread -ioengine=io_uring -bs=4k -numjobs=1 -size=4k -filename=/dev/nvme0n1 -hipri=1 -name=block
>>
>> BUG: KASAN: null-ptr-deref in bio_alloc_bioset.cold+0x2a/0x16a
>> Read of size 8 at addr 0000000000000000 by task fio/1835
>>
>> CPU: 5 PID: 1835 Comm: fio Not tainted 6.1.0-rc2+ #226
>> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g
>> Call Trace:
>> <TASK>
>> dump_stack_lvl+0x34/0x48
>> print_report+0x490/0x4a1
>> ? __virt_addr_valid+0x28/0x140
>> ? bio_alloc_bioset.cold+0x2a/0x16a
>> kasan_report+0xb3/0x130
>> ? bio_alloc_bioset.cold+0x2a/0x16a
>> bio_alloc_bioset.cold+0x2a/0x16a
>> ? bvec_alloc+0xf0/0xf0
>> ? iov_iter_is_aligned+0x130/0x2c0
>> blkdev_direct_IO.part.0+0x16a/0x8d0
>
> Was going to apply this, but after running some testing, it does
> fix the initial crash but I still get weird corruption crashes
> with the series it's fixing.
>
> Pavel, I'm going to drop this series for now.
I found one yesterday. Is the issue reproducible?
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] block: fix bio-allocation from per-cpu cache
2022-10-27 20:45 ` Pavel Begunkov
@ 2022-10-27 20:50 ` Pavel Begunkov
2022-10-27 20:52 ` Jens Axboe
1 sibling, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-10-27 20:50 UTC (permalink / raw)
To: Jens Axboe, Kanchan Joshi, linux-block
On 10/27/22 21:45, Pavel Begunkov wrote:
> On 10/27/22 21:44, Jens Axboe wrote:
>> On 10/27/22 4:04 AM, Kanchan Joshi wrote:
>>> If cache does not have any entry, make sure to detect that and return
>>> failure. Otherwise this leads to null pointer dereference.
>>>
>>> Fixes: 13a184e26965 ("block/bio: add pcpu caching for non-polling bio_put")
>>> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
>>> ---
>>> Can be reproduced by:
>>> fio -direct=1 -iodepth=1 -rw=randread -ioengine=io_uring -bs=4k -numjobs=1 -size=4k -filename=/dev/nvme0n1 -hipri=1 -name=block
>>>
>>> BUG: KASAN: null-ptr-deref in bio_alloc_bioset.cold+0x2a/0x16a
>>> Read of size 8 at addr 0000000000000000 by task fio/1835
>>>
>>> CPU: 5 PID: 1835 Comm: fio Not tainted 6.1.0-rc2+ #226
>>> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g
>>> Call Trace:
>>> <TASK>
>>> dump_stack_lvl+0x34/0x48
>>> print_report+0x490/0x4a1
>>> ? __virt_addr_valid+0x28/0x140
>>> ? bio_alloc_bioset.cold+0x2a/0x16a
>>> kasan_report+0xb3/0x130
>>> ? bio_alloc_bioset.cold+0x2a/0x16a
>>> bio_alloc_bioset.cold+0x2a/0x16a
>>> ? bvec_alloc+0xf0/0xf0
>>> ? iov_iter_is_aligned+0x130/0x2c0
>>> blkdev_direct_IO.part.0+0x16a/0x8d0
>>
>> Was going to apply this, but after running some testing, it does
>> fix the initial crash but I still get weird corruption crashes
>> with the series it's fixing.
>>
>> Pavel, I'm going to drop this series for now.
>
> I found one yesterday. Is the issue reproducible?
found one bug *
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] block: fix bio-allocation from per-cpu cache
2022-10-27 20:45 ` Pavel Begunkov
2022-10-27 20:50 ` Pavel Begunkov
@ 2022-10-27 20:52 ` Jens Axboe
2022-10-27 21:35 ` Pavel Begunkov
1 sibling, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2022-10-27 20:52 UTC (permalink / raw)
To: Pavel Begunkov, Kanchan Joshi, linux-block
On 10/27/22 2:45 PM, Pavel Begunkov wrote:
> On 10/27/22 21:44, Jens Axboe wrote:
>> On 10/27/22 4:04 AM, Kanchan Joshi wrote:
>>> If cache does not have any entry, make sure to detect that and return
>>> failure. Otherwise this leads to null pointer dereference.
>>>
>>> Fixes: 13a184e26965 ("block/bio: add pcpu caching for non-polling bio_put")
>>> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
>>> ---
>>> Can be reproduced by:
>>> fio -direct=1 -iodepth=1 -rw=randread -ioengine=io_uring -bs=4k -numjobs=1 -size=4k -filename=/dev/nvme0n1 -hipri=1 -name=block
>>>
>>> BUG: KASAN: null-ptr-deref in bio_alloc_bioset.cold+0x2a/0x16a
>>> Read of size 8 at addr 0000000000000000 by task fio/1835
>>>
>>> CPU: 5 PID: 1835 Comm: fio Not tainted 6.1.0-rc2+ #226
>>> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g
>>> Call Trace:
>>> <TASK>
>>> dump_stack_lvl+0x34/0x48
>>> print_report+0x490/0x4a1
>>> ? __virt_addr_valid+0x28/0x140
>>> ? bio_alloc_bioset.cold+0x2a/0x16a
>>> kasan_report+0xb3/0x130
>>> ? bio_alloc_bioset.cold+0x2a/0x16a
>>> bio_alloc_bioset.cold+0x2a/0x16a
>>> ? bvec_alloc+0xf0/0xf0
>>> ? iov_iter_is_aligned+0x130/0x2c0
>>> blkdev_direct_IO.part.0+0x16a/0x8d0
>>
>> Was going to apply this, but after running some testing, it does
>> fix the initial crash but I still get weird corruption crashes
>> with the series it's fixing.
>>
>> Pavel, I'm going to drop this series for now.
>
> I found one yesterday. Is the issue reproducible?
Oh yeah, triggers in < 1 second for me when running my usual irq
peak bench:
t/io_uring -p0 -d128 -b512 -s32 -c32 -F1 -B1 -R0 -X1 -n24 -P1 /dev/nvme0n1 /dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 /dev/nvme4n1 /dev/nvme5n1 /dev/nvme6n1 /dev/nvme7n1 /dev/nvme8n1 /dev/nvme9n1 /dev/nvme10n1 /dev/nvme11n1 /dev/nvme12n1 /dev/nvme13n1 /dev/nvme14n1 /dev/nvme15n1 /dev/nvme16n1 /dev/nvme17n1 /dev/nvme18n1 /dev/nvme19n1 /dev/nvme20n1 /dev/nvme21n1 /dev/nvme22n1 /dev/nvme23n1
Interestingly, doesn't trigger in qemu with just a single device.
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] block: fix bio-allocation from per-cpu cache
2022-10-27 20:52 ` Jens Axboe
@ 2022-10-27 21:35 ` Pavel Begunkov
0 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-10-27 21:35 UTC (permalink / raw)
To: Jens Axboe, Kanchan Joshi, linux-block
On 10/27/22 21:52, Jens Axboe wrote:
> On 10/27/22 2:45 PM, Pavel Begunkov wrote:
>> On 10/27/22 21:44, Jens Axboe wrote:
>>> On 10/27/22 4:04 AM, Kanchan Joshi wrote:
>>>> If cache does not have any entry, make sure to detect that and return
>>>> failure. Otherwise this leads to null pointer dereference.
>>>>
>>>> Fixes: 13a184e26965 ("block/bio: add pcpu caching for non-polling bio_put")
>>>> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
>>>> ---
>>>> Can be reproduced by:
>>>> fio -direct=1 -iodepth=1 -rw=randread -ioengine=io_uring -bs=4k -numjobs=1 -size=4k -filename=/dev/nvme0n1 -hipri=1 -name=block
>>>>
>>>> BUG: KASAN: null-ptr-deref in bio_alloc_bioset.cold+0x2a/0x16a
>>>> Read of size 8 at addr 0000000000000000 by task fio/1835
>>>>
>>>> CPU: 5 PID: 1835 Comm: fio Not tainted 6.1.0-rc2+ #226
>>>> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.14.0-0-g
>>>> Call Trace:
>>>> <TASK>
>>>> dump_stack_lvl+0x34/0x48
>>>> print_report+0x490/0x4a1
>>>> ? __virt_addr_valid+0x28/0x140
>>>> ? bio_alloc_bioset.cold+0x2a/0x16a
>>>> kasan_report+0xb3/0x130
>>>> ? bio_alloc_bioset.cold+0x2a/0x16a
>>>> bio_alloc_bioset.cold+0x2a/0x16a
>>>> ? bvec_alloc+0xf0/0xf0
>>>> ? iov_iter_is_aligned+0x130/0x2c0
>>>> blkdev_direct_IO.part.0+0x16a/0x8d0
>>>
>>> Was going to apply this, but after running some testing, it does
>>> fix the initial crash but I still get weird corruption crashes
>>> with the series it's fixing.
>>>
>>> Pavel, I'm going to drop this series for now.
>>
>> I found one yesterday. Is the issue reproducible?
>
> Oh yeah, triggers in < 1 second for me when running my usual irq
> peak bench:
>
> t/io_uring -p0 -d128 -b512 -s32 -c32 -F1 -B1 -R0 -X1 -n24 -P1 /dev/nvme0n1 /dev/nvme1n1 /dev/nvme2n1 /dev/nvme3n1 /dev/nvme4n1 /dev/nvme5n1 /dev/nvme6n1 /dev/nvme7n1 /dev/nvme8n1 /dev/nvme9n1 /dev/nvme10n1 /dev/nvme11n1 /dev/nvme12n1 /dev/nvme13n1 /dev/nvme14n1 /dev/nvme15n1 /dev/nvme16n1 /dev/nvme17n1 /dev/nvme18n1 /dev/nvme19n1 /dev/nvme20n1 /dev/nvme21n1 /dev/nvme22n1 /dev/nvme23n1
>
> Interestingly, doesn't trigger in qemu with just a single device.
The bug I mentioned is splicing from in-IRQ put, which modifies
the non-irq list. We need to hit that ALLOC_CACHE_MAX + ALLOC_CACHE_SLACK
in the cache to trigger it, so makes sense you see it only with
very high qd tests, matches the profile.
I'll resend the patch set with a few changes, but would be great
if you can say if sth like below works for you
diff --git a/block/bio.c b/block/bio.c
index 0686a3774157..af715aee239b 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -764,6 +764,12 @@ static inline void bio_put_percpu_cache(struct bio *bio)
struct bio_alloc_cache *cache;
cache = per_cpu_ptr(bio->bi_pool->cache, get_cpu());
+ if (READ_ONCE(cache->nr_irq) + cache->nr > ALLOC_CACHE_MAX) {
+ put_cpu();
+ bio_free(bio);
+ return;
+ }
+
bio_uninit(bio);
if ((bio->bi_opf & REQ_POLLED) && !WARN_ON_ONCE(in_interrupt())) {
@@ -779,10 +785,6 @@ static inline void bio_put_percpu_cache(struct bio *bio)
cache->nr_irq++;
local_irq_restore(flags);
}
-
- if (READ_ONCE(cache->nr_irq) + cache->nr >
- ALLOC_CACHE_MAX + ALLOC_CACHE_SLACK)
- bio_alloc_cache_prune(cache, ALLOC_CACHE_SLACK);
put_cpu();
}
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-10-27 21:36 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20221027101534epcas5p3b8335c05a1003531f1a4488dc27f27ee@epcas5p3.samsung.com>
2022-10-27 10:04 ` [PATCH] block: fix bio-allocation from per-cpu cache Kanchan Joshi
2022-10-27 10:38 ` Pavel Begunkov
2022-10-27 10:49 ` Kanchan Joshi
2022-10-27 11:46 ` Pavel Begunkov
2022-10-27 20:44 ` Jens Axboe
2022-10-27 20:45 ` Pavel Begunkov
2022-10-27 20:50 ` Pavel Begunkov
2022-10-27 20:52 ` Jens Axboe
2022-10-27 21:35 ` Pavel Begunkov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.