All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Cc: linux-block@vger.kernel.org, linux-nvme@lists.infradead.org,
	axboe@fb.com, hch@lst.de, kbusch@kernel.org
Subject: Re: [PATCH] nvme-core: check bdev value for NULL
Date: Wed, 27 Jan 2021 08:13:44 +0100	[thread overview]
Message-ID: <20210127071344.GA21223@lst.de> (raw)
In-Reply-To: <20210127053738.4922-1-chaitanya.kulkarni@wdc.com>

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Jens, can you pick this up as it fixes an issue I introduced in
for-5.12/block?

On Tue, Jan 26, 2021 at 09:37:38PM -0800, Chaitanya Kulkarni wrote:
> The nvme-core sets the bdev to NULL when admin comamnd is issued from
> IOCTL in the following path e.g. nvme list :-
> 
> block_ioctl()
>  blkdev_ioctl()
>   nvme_ioctl()
>    nvme_user_cmd()
>     nvme_submit_user_cmd()
> 
> The commit 309dca309fc3 ("block: store a block_device pointer in struct bio")
> now uses bdev unconditionally in the macro bio_set_dev() and assumes
> that bdev value is not NULL which results in the following crash in
> since thats where bdev is actually accessed :-
> 
> void bio_associate_blkg_from_css(struct bio *bio,
> 				 struct cgroup_subsys_state *css)
> {
> 	if (bio->bi_blkg)
> 		blkg_put(bio->bi_blkg);
> 
> 	if (css && css->parent) {
> 		bio->bi_blkg = blkg_tryget_closest(bio, css);
> 	} else {
> -------------->	blkg_get(bio->bi_bdev->bd_disk->queue->root_blkg);
> 		bio->bi_blkg = bio->bi_bdev->bd_disk->queue->root_blkg;
> 	}
> }
> EXPORT_SYMBOL_GPL(bio_associate_blkg_from_css);
> 
> <1>[  345.385947] BUG: kernel NULL pointer dereference, address: 0000000000000690
> <1>[  345.387103] #PF: supervisor read access in kernel mode
> <1>[  345.387894] #PF: error_code(0x0000) - not-present page
> <6>[  345.388756] PGD 162a2b067 P4D 162a2b067 PUD 1633eb067 PMD 0
> <4>[  345.389625] Oops: 0000 [#1] SMP NOPTI
> <4>[  345.390206] CPU: 15 PID: 4100 Comm: nvme Tainted: G           OE     5.11.0-rc5blk+ #141
> <4>[  345.391377] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba52764
> <4>[  345.393074] RIP: 0010:bio_associate_blkg_from_css.cold.47+0x58/0x21f
> 
> <4>[  345.396362] RSP: 0018:ffffc90000dbbce8 EFLAGS: 00010246
> <4>[  345.397078] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000027
> <4>[  345.398114] RDX: 0000000000000000 RSI: ffff888813be91f0 RDI: ffff888813be91f8
> <4>[  345.399039] RBP: ffffc90000dbbd30 R08: 0000000000000001 R09: 0000000000000001
> <4>[  345.399950] R10: 0000000064c66670 R11: 00000000ef955201 R12: ffff888812d32800
> <4>[  345.401031] R13: 0000000000000000 R14: ffff888113e51540 R15: ffff888113e51540
> <4>[  345.401976] FS:  00007f3747f1d780(0000) GS:ffff888813a00000(0000) knlGS:0000000000000000
> <4>[  345.402997] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4>[  345.403737] CR2: 0000000000000690 CR3: 000000081a4bc000 CR4: 00000000003506e0
> <4>[  345.404685] Call Trace:
> <4>[  345.405031]  bio_associate_blkg+0x71/0x1c0
> <4>[  345.405649]  nvme_submit_user_cmd+0x1aa/0x38e [nvme_core]
> <4>[  345.406348]  nvme_user_cmd.isra.73.cold.98+0x54/0x92 [nvme_core]
> <4>[  345.407117]  nvme_ioctl+0x226/0x260 [nvme_core]
> <4>[  345.407707]  blkdev_ioctl+0x1c8/0x2b0
> <4>[  345.408183]  block_ioctl+0x3f/0x50
> <4>[  345.408627]  __x64_sys_ioctl+0x84/0xc0
> <4>[  345.409117]  do_syscall_64+0x33/0x40
> <4>[  345.409592]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> <4>[  345.410233] RIP: 0033:0x7f3747632107
> 
> <4>[  345.413125] RSP: 002b:00007ffe461b6648 EFLAGS: 00000206 ORIG_RAX: 0000000000000010
> <4>[  345.414086] RAX: ffffffffffffffda RBX: 00000000007b7fd0 RCX: 00007f3747632107
> <4>[  345.414998] RDX: 00007ffe461b6650 RSI: 00000000c0484e41 RDI: 0000000000000004
> <4>[  345.415966] RBP: 0000000000000004 R08: 00000000007b7fe8 R09: 00000000007b9080
> <4>[  345.416883] R10: 00007ffe461b62c0 R11: 0000000000000206 R12: 00000000007b7fd0
> <4>[  345.417808] R13: 0000000000000000 R14: 0000000000000003 R15: 0000000000000000
> 
> Add a NULL check before we set the bdev for bio.
> 
> This issue is found on block/for-next tree.
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>  drivers/nvme/host/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index ba5df80881ea..1a3cdc6b1036 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1133,7 +1133,8 @@ static int nvme_submit_user_cmd(struct request_queue *q,
>  		if (ret)
>  			goto out;
>  		bio = req->bio;
> -		bio_set_dev(bio, bdev);
> +		if (bdev)
> +			bio_set_dev(bio, bdev);
>  		if (bdev && meta_buffer && meta_len) {
>  			meta = nvme_add_user_metadata(bio, meta_buffer, meta_len,
>  					meta_seed, write);
> -- 
> 2.22.1
---end quoted text---

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Cc: linux-block@vger.kernel.org, axboe@fb.com, kbusch@kernel.org,
	hch@lst.de, linux-nvme@lists.infradead.org
Subject: Re: [PATCH] nvme-core: check bdev value for NULL
Date: Wed, 27 Jan 2021 08:13:44 +0100	[thread overview]
Message-ID: <20210127071344.GA21223@lst.de> (raw)
In-Reply-To: <20210127053738.4922-1-chaitanya.kulkarni@wdc.com>

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Jens, can you pick this up as it fixes an issue I introduced in
for-5.12/block?

On Tue, Jan 26, 2021 at 09:37:38PM -0800, Chaitanya Kulkarni wrote:
> The nvme-core sets the bdev to NULL when admin comamnd is issued from
> IOCTL in the following path e.g. nvme list :-
> 
> block_ioctl()
>  blkdev_ioctl()
>   nvme_ioctl()
>    nvme_user_cmd()
>     nvme_submit_user_cmd()
> 
> The commit 309dca309fc3 ("block: store a block_device pointer in struct bio")
> now uses bdev unconditionally in the macro bio_set_dev() and assumes
> that bdev value is not NULL which results in the following crash in
> since thats where bdev is actually accessed :-
> 
> void bio_associate_blkg_from_css(struct bio *bio,
> 				 struct cgroup_subsys_state *css)
> {
> 	if (bio->bi_blkg)
> 		blkg_put(bio->bi_blkg);
> 
> 	if (css && css->parent) {
> 		bio->bi_blkg = blkg_tryget_closest(bio, css);
> 	} else {
> -------------->	blkg_get(bio->bi_bdev->bd_disk->queue->root_blkg);
> 		bio->bi_blkg = bio->bi_bdev->bd_disk->queue->root_blkg;
> 	}
> }
> EXPORT_SYMBOL_GPL(bio_associate_blkg_from_css);
> 
> <1>[  345.385947] BUG: kernel NULL pointer dereference, address: 0000000000000690
> <1>[  345.387103] #PF: supervisor read access in kernel mode
> <1>[  345.387894] #PF: error_code(0x0000) - not-present page
> <6>[  345.388756] PGD 162a2b067 P4D 162a2b067 PUD 1633eb067 PMD 0
> <4>[  345.389625] Oops: 0000 [#1] SMP NOPTI
> <4>[  345.390206] CPU: 15 PID: 4100 Comm: nvme Tainted: G           OE     5.11.0-rc5blk+ #141
> <4>[  345.391377] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba52764
> <4>[  345.393074] RIP: 0010:bio_associate_blkg_from_css.cold.47+0x58/0x21f
> 
> <4>[  345.396362] RSP: 0018:ffffc90000dbbce8 EFLAGS: 00010246
> <4>[  345.397078] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000027
> <4>[  345.398114] RDX: 0000000000000000 RSI: ffff888813be91f0 RDI: ffff888813be91f8
> <4>[  345.399039] RBP: ffffc90000dbbd30 R08: 0000000000000001 R09: 0000000000000001
> <4>[  345.399950] R10: 0000000064c66670 R11: 00000000ef955201 R12: ffff888812d32800
> <4>[  345.401031] R13: 0000000000000000 R14: ffff888113e51540 R15: ffff888113e51540
> <4>[  345.401976] FS:  00007f3747f1d780(0000) GS:ffff888813a00000(0000) knlGS:0000000000000000
> <4>[  345.402997] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4>[  345.403737] CR2: 0000000000000690 CR3: 000000081a4bc000 CR4: 00000000003506e0
> <4>[  345.404685] Call Trace:
> <4>[  345.405031]  bio_associate_blkg+0x71/0x1c0
> <4>[  345.405649]  nvme_submit_user_cmd+0x1aa/0x38e [nvme_core]
> <4>[  345.406348]  nvme_user_cmd.isra.73.cold.98+0x54/0x92 [nvme_core]
> <4>[  345.407117]  nvme_ioctl+0x226/0x260 [nvme_core]
> <4>[  345.407707]  blkdev_ioctl+0x1c8/0x2b0
> <4>[  345.408183]  block_ioctl+0x3f/0x50
> <4>[  345.408627]  __x64_sys_ioctl+0x84/0xc0
> <4>[  345.409117]  do_syscall_64+0x33/0x40
> <4>[  345.409592]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> <4>[  345.410233] RIP: 0033:0x7f3747632107
> 
> <4>[  345.413125] RSP: 002b:00007ffe461b6648 EFLAGS: 00000206 ORIG_RAX: 0000000000000010
> <4>[  345.414086] RAX: ffffffffffffffda RBX: 00000000007b7fd0 RCX: 00007f3747632107
> <4>[  345.414998] RDX: 00007ffe461b6650 RSI: 00000000c0484e41 RDI: 0000000000000004
> <4>[  345.415966] RBP: 0000000000000004 R08: 00000000007b7fe8 R09: 00000000007b9080
> <4>[  345.416883] R10: 00007ffe461b62c0 R11: 0000000000000206 R12: 00000000007b7fd0
> <4>[  345.417808] R13: 0000000000000000 R14: 0000000000000003 R15: 0000000000000000
> 
> Add a NULL check before we set the bdev for bio.
> 
> This issue is found on block/for-next tree.
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>  drivers/nvme/host/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index ba5df80881ea..1a3cdc6b1036 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1133,7 +1133,8 @@ static int nvme_submit_user_cmd(struct request_queue *q,
>  		if (ret)
>  			goto out;
>  		bio = req->bio;
> -		bio_set_dev(bio, bdev);
> +		if (bdev)
> +			bio_set_dev(bio, bdev);
>  		if (bdev && meta_buffer && meta_len) {
>  			meta = nvme_add_user_metadata(bio, meta_buffer, meta_len,
>  					meta_seed, write);
> -- 
> 2.22.1
---end quoted text---

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

  reply	other threads:[~2021-01-27  7:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-27  5:37 [PATCH] nvme-core: check bdev value for NULL Chaitanya Kulkarni
2021-01-27  5:37 ` Chaitanya Kulkarni
2021-01-27  7:13 ` Christoph Hellwig [this message]
2021-01-27  7:13   ` Christoph Hellwig
2021-01-27 17:09   ` Jens Axboe
2021-01-27 17:09     ` Jens Axboe

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=20210127071344.GA21223@lst.de \
    --to=hch@lst.de \
    --cc=axboe@fb.com \
    --cc=chaitanya.kulkarni@wdc.com \
    --cc=kbusch@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    /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.