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@xxxxxxxxx/
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@xxxxxxxxxxx>
---
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.
