Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Guenter Roeck <linux@roeck-us.net>
To:     Paolo Valente <paolo.valente@linaro.org>,
        Jens Axboe <axboe@kernel.dk>
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        Guenter Roeck <linux@roeck-us.net>,
        Hsin-Yi Wang <hsinyi@google.com>,
        Nicolas Boichat <drinkcat@chromium.org>,
        Doug Anderson <dianders@chromium.org>
Subject: [PATCH] bfq: Check if bfqq is NULL in bfq_insert_request
Date:   Mon, 22 Jul 2019 10:30:48 -0700
Message-Id: <1563816648-12057-1-git-send-email-linux@roeck-us.net>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

In bfq_insert_request(), bfqq is initialized with:
	bfqq = bfq_init_rq(rq);
In bfq_init_rq(), we find:
	if (unlikely(!rq->elv.icq))
		return NULL;
Indeed, rq->elv.icq can be NULL if the memory allocation in
create_task_io_context() failed.

A comment in bfq_insert_request() suggests that bfqq is supposed to be
non-NULL if 'at_head || blk_rq_is_passthrough(rq)' is false. Yet, as
debugging and practical experience shows, this is not the case in the
above situation.

This results in the following crash.

Unable to handle kernel NULL pointer dereference
	at virtual address 00000000000001b0
...
Call trace:
 bfq_setup_cooperator+0x44/0x134
 bfq_insert_requests+0x10c/0x630
 blk_mq_sched_insert_requests+0x60/0xb4
 blk_mq_flush_plug_list+0x290/0x2d4
 blk_flush_plug_list+0xe0/0x230
 blk_finish_plug+0x30/0x40
 generic_writepages+0x60/0x94
 blkdev_writepages+0x24/0x30
 do_writepages+0x74/0xac
 __filemap_fdatawrite_range+0x94/0xc8
 file_write_and_wait_range+0x44/0xa0
 blkdev_fsync+0x38/0x68
 vfs_fsync_range+0x68/0x80
 do_fsync+0x44/0x80

The problem is relatively easy to reproduce by running an image with
failslab enabled, such as with:

cd /sys/kernel/debug/failslab
echo 10 > probability
echo 300 > times

Avoid the problem by checking if bfqq is NULL before using it. With the
NULL check in place, requests with missing io context are queued
immediately, and the crash is no longer seen.

Fixes: 18e5a57d79878 ("block, bfq: postpone rq preparation to insert or merge")
Reported-by: Hsin-Yi Wang  <hsinyi@google.com>
Cc: Hsin-Yi Wang <hsinyi@google.com>
Cc: Nicolas Boichat <drinkcat@chromium.org>
Cc: Doug Anderson <dianders@chromium.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 block/bfq-iosched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 72860325245a..56f3f4227010 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5417,7 +5417,7 @@ static void bfq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
 
 	spin_lock_irq(&bfqd->lock);
 	bfqq = bfq_init_rq(rq);
-	if (at_head || blk_rq_is_passthrough(rq)) {
+	if (!bfqq || at_head || blk_rq_is_passthrough(rq)) {
 		if (at_head)
 			list_add(&rq->queuelist, &bfqd->dispatch);
 		else
-- 
2.7.4