All of lore.kernel.org
 help / color / mirror / Atom feed
From: yu kuai <yukuai3@huawei.com>
To: <axboe@kernel.dk>, <ming.lei@redhat.com>, <bvanassche@acm.org>
Cc: <yukuai3@huawei.com>, <yi.zhang@huawei.com>, <yuyufen@huawei.com>,
	<linux-block@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: [RFC] block: fix access of uninitialized pointer address in bt_for_each()
Date: Fri, 17 Apr 2020 20:51:34 +0800	[thread overview]
Message-ID: <20200417125134.45117-1-yukuai3@huawei.com> (raw)

I recently got a KASAN warning like this in our 4.19 kernel:

 ==================================================================
 BUG: KASAN: slab-out-of-bounds in bt_for_each+0x1dc/0x2c0
 Read of size 8 at addr ffff8000c0865000 by task sh/2023305

 Call trace:
 dump_backtrace+0x0/0x310
 show_stack+0x28/0x38
 dump_stack+0xd8/0x108
 print_address_description+0x68/0x2d0
 kasan_report+0x124/0x2e0
 __asan_load8+0x88/0xb0
 bt_for_each+0x1dc/0x2c0
 blk_mq_queue_tag_busy_iter+0x1f0/0x3e8
 blk_mq_in_flight+0xb4/0xe0
 part_in_flight+0x124/0x178
 part_round_stats+0x128/0x3b0
 blk_account_io_start+0x2b4/0x3f0
 blk_mq_bio_to_request+0x170/0x258
 blk_mq_make_request+0x734/0xdd8
 generic_make_request+0x388/0x740
 submit_bio+0xd8/0x3d0
 ext4_io_submit+0xb4/0xe0 [ext4]
 ext4_writepages+0xb44/0x1c00 [ext4]
 do_writepages+0xc8/0x1f8
 __filemap_fdatawrite_range+0x200/0x2a0
 filemap_flush+0x30/0x40
 ext4_alloc_da_blocks+0x54/0x200 [ext4]
 ext4_release_file+0xfc/0x150 [ext4]
 __fput+0x15c/0x3a8
 ____fput+0x24/0x30
 task_work_run+0x1a4/0x208
 do_notify_resume+0x1a8/0x1c0
 work_pending+0x8/0x10

 Allocated by task 3515778:
 kasan_kmalloc+0xe0/0x190
 kmem_cache_alloc_trace+0x18c/0x418
 alloc_pipe_info+0x74/0x240
 create_pipe_files+0x74/0x2f8
 __do_pipe_flags+0x48/0x168
 do_pipe2+0xa0/0x1d0
 __arm64_sys_pipe2+0x3c/0x50
 el0_svc_common+0xb4/0x1d8
 el0_svc_handler+0x50/0xa8
 el0_svc+0x8/0xc

 Freed by task 3515778:
 __kasan_slab_free+0x120/0x228
 kasan_slab_free+0x10/0x18
 kfree+0x88/0x3d8
 free_pipe_info+0x150/0x178
 put_pipe_info+0x138/0x1c0
 pipe_release+0xe8/0x120
 __fput+0x15c/0x3a8
 ____fput+0x24/0x30
 task_work_run+0x1a4/0x208
 do_notify_resume+0x1a8/0x1c0
 work_pending+0x8/0x10

 The buggy address belongs to the object at ffff8000c0864f00#012 which belongs to the cache kmalloc-256 of size 256
 The buggy address is located 0 bytes to the right of#012 256-byte region [ffff8000c0864f00, ffff8000c0865000)
 The buggy address belongs to the page:
 page:ffff7e0003021900 count:1 mapcount:0 mapping:ffff80036d00fc00 index:0x0 compound_mapcount: 0
 flags: 0xffffe0000008100(slab|head)
 raw: 0ffffe0000008100 ffff7e0003617f88 ffff7e000d1a6208 ffff80036d00fc00
 raw: 0000000000000000 0000000000150015 00000001ffffffff 0000000000000000
 page dumped because: kasan: bad access detected

 Memory state around the buggy address:
 ffff8000c0864f00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff8000c0864f80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 >ffff8000c0865000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ^
 ffff8000c0865080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff8000c0865100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ==================================================================

After looking into it, I think it's because bt_for_each() accessed
uninitialized pointer address:

thread1			thread2
blk_mq_alloc_tag_set
...
blk_mq_init_queue
...
submit_bio(bio1)	submit_bio(bio2)
 blk_mq_get_request
  blk_mq_get_tag
  			 ...
  			 bt_for_each
  			  bt_iter
  			   rq = tags->rqs[b]
  			    rq->q		----> here
  blk_mq_rq_ctx_init
   tags->rqs[a] = rq

blk_mq_get_tag() is called before blk_mq_rq_ctx_init(), which leaves a
window for bt_for_each() to access 'tags->rqs[tag]->q' before
'tags->rqs[tag]' is set in blk_mq_rq_ctx_init(). While blk_mq_init_tags()
is using 'kcalloc()' for 'tags->rqs'. And I think the problem exist in
mainline, too.

The problem haven't been reporduced unless I manually sleep a while
before 'tags->rqs[tag]' is set:

@@ -275,6 +275,7 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
        struct blk_mq_tags *tags = blk_mq_tags_from_data(data);
        struct request *rq = tags->static_rqs[tag];
        req_flags_t rq_flags = 0;
+       static int debug_count = 0;

        if (data->flags & BLK_MQ_REQ_INTERNAL) {
                rq->tag = -1;
@@ -286,6 +287,12 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
                }
                rq->tag = tag;
                rq->internal_tag = -1;
+               if (!strcmp(dev_name(data->q->backing_dev_info->dev), "250:0")) {
+                       if (debug_count == 0) {
+                               debug_count++;
+                               msleep(5000);
+                       }
+               }
                data->hctx->tags->rqs[rq->tag] = rq;
        }

BTW, I noticed there is a similar problem that haven't been solved yet:
https://lore.kernel.org/linux-block/1545261885.185366.488.camel@acm.org/

I'm trying to fix the problem by replacing 'kcalloc' as 'kzalloc' for
'tags->rqs', and set 'tags->rqs[tag]' to 'NULL' before putting the tag.

---
 block/blk-mq.c | 3 ++-
 block/blk-mq.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 7ed16ed13976..48b74d0085c7 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -485,6 +485,7 @@ static void __blk_mq_free_request(struct request *rq)
 	struct blk_mq_hw_ctx *hctx = blk_mq_map_queue(q, ctx->cpu);
 	const int sched_tag = rq->internal_tag;
 
+	hctx->tags->rqs[rq->tag] = NULL;
 	if (rq->tag != -1)
 		blk_mq_put_tag(hctx, hctx->tags, ctx, rq->tag);
 	if (sched_tag != -1)
@@ -1999,7 +2000,7 @@ struct blk_mq_tags *blk_mq_alloc_rq_map(struct blk_mq_tag_set *set,
 	if (!tags)
 		return NULL;
 
-	tags->rqs = kcalloc_node(nr_tags, sizeof(struct request *),
+	tags->rqs = kzalloc_node(nr_tags, sizeof(struct request *),
 				 GFP_NOIO | __GFP_NOWARN | __GFP_NORETRY,
 				 node);
 	if (!tags->rqs) {
diff --git a/block/blk-mq.h b/block/blk-mq.h
index a6094c27b827..2a55292d3d51 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -196,6 +196,7 @@ static inline void blk_mq_put_driver_tag_hctx(struct blk_mq_hw_ctx *hctx,
 	if (rq->tag == -1 || rq->internal_tag == -1)
 		return;
 
+	hctx->tags->rqs[rq->tag] = NULL;
 	__blk_mq_put_driver_tag(hctx, rq);
 }
 
@@ -207,6 +208,7 @@ static inline void blk_mq_put_driver_tag(struct request *rq)
 		return;
 
 	hctx = blk_mq_map_queue(rq->q, rq->mq_ctx->cpu);
+	hctx->tags->rqs[rq->tag] = NULL;
 	__blk_mq_put_driver_tag(hctx, rq);
 }
 
-- 
2.21.1


             reply	other threads:[~2020-04-17 12:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-17 12:51 yu kuai [this message]
2020-04-17 14:26 ` [RFC] block: fix access of uninitialized pointer address in bt_for_each() Bart Van Assche
2020-04-18  3:24   ` yukuai (C)
2020-04-18  9:42   ` yukuai (C)
2020-04-18 15:26     ` Bart Van Assche
2020-04-18  2:11 ` Ming Lei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200417125134.45117-1-yukuai3@huawei.com \
    --to=yukuai3@huawei.com \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=yi.zhang@huawei.com \
    --cc=yuyufen@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.