diff mbox

virtio_blk: fix panic in initialization error path

Message ID 4bc0f759ce198dc36e9b678a3c8f69bfef5cb728.1483990999.git.osandov@fb.com (mailing list archive)
State New, archived
Headers show

Commit Message

Omar Sandoval Jan. 9, 2017, 7:44 p.m. UTC
From: Omar Sandoval <osandov@fb.com>

If blk_mq_init_queue() returns an error, it gets assigned to
vblk->disk->queue. Then, when we call put_disk(), we end up calling
blk_put_queue() with the ERR_PTR, causing a bad dereference. Fix it by
only assigning to vblk->disk->queue on success.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 drivers/block/virtio_blk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jeff Moyer Jan. 9, 2017, 7:55 p.m. UTC | #1
Omar Sandoval <osandov@osandov.com> writes:

> From: Omar Sandoval <osandov@fb.com>
>
> If blk_mq_init_queue() returns an error, it gets assigned to
> vblk->disk->queue. Then, when we call put_disk(), we end up calling
> blk_put_queue() with the ERR_PTR, causing a bad dereference. Fix it by
> only assigning to vblk->disk->queue on success.
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Wang Jan. 10, 2017, 2:47 a.m. UTC | #2
On 2017年01月10日 03:44, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> If blk_mq_init_queue() returns an error, it gets assigned to
> vblk->disk->queue. Then, when we call put_disk(), we end up calling
> blk_put_queue() with the ERR_PTR, causing a bad dereference. Fix it by
> only assigning to vblk->disk->queue on success.
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
>   drivers/block/virtio_blk.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 5545a679abd8..8587361e5356 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -628,11 +628,12 @@ static int virtblk_probe(struct virtio_device *vdev)
>   	if (err)
>   		goto out_put_disk;
>   
> -	q = vblk->disk->queue = blk_mq_init_queue(&vblk->tag_set);
> +	q = blk_mq_init_queue(&vblk->tag_set);
>   	if (IS_ERR(q)) {
>   		err = -ENOMEM;
>   		goto out_free_tags;
>   	}
> +	vblk->disk->queue = q;
>   
>   	q->queuedata = vblk;
>   

Acked-by: Jason Wang <jasowang@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael S. Tsirkin Jan. 10, 2017, 4:10 a.m. UTC | #3
On Mon, Jan 09, 2017 at 11:44:12AM -0800, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> If blk_mq_init_queue() returns an error, it gets assigned to
> vblk->disk->queue. Then, when we call put_disk(), we end up calling
> blk_put_queue() with the ERR_PTR, causing a bad dereference. Fix it by
> only assigning to vblk->disk->queue on success.
> 
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Jens, do you mind picking this one up as well, since
you have one virtio-blk patch already?


> ---
>  drivers/block/virtio_blk.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 5545a679abd8..8587361e5356 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -628,11 +628,12 @@ static int virtblk_probe(struct virtio_device *vdev)
>  	if (err)
>  		goto out_put_disk;
>  
> -	q = vblk->disk->queue = blk_mq_init_queue(&vblk->tag_set);
> +	q = blk_mq_init_queue(&vblk->tag_set);
>  	if (IS_ERR(q)) {
>  		err = -ENOMEM;
>  		goto out_free_tags;
>  	}
> +	vblk->disk->queue = q;
>  
>  	q->queuedata = vblk;
>  
> -- 
> 2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jens Axboe Jan. 10, 2017, 4:11 a.m. UTC | #4
On 01/09/2017 09:10 PM, Michael S. Tsirkin wrote:
> On Mon, Jan 09, 2017 at 11:44:12AM -0800, Omar Sandoval wrote:
>> From: Omar Sandoval <osandov@fb.com>
>>
>> If blk_mq_init_queue() returns an error, it gets assigned to
>> vblk->disk->queue. Then, when we call put_disk(), we end up calling
>> blk_put_queue() with the ERR_PTR, causing a bad dereference. Fix it by
>> only assigning to vblk->disk->queue on success.
>>
>> Signed-off-by: Omar Sandoval <osandov@fb.com>
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Jens, do you mind picking this one up as well, since
> you have one virtio-blk patch already?

No problem, in fact I already queued it up.
diff mbox

Patch

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5545a679abd8..8587361e5356 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -628,11 +628,12 @@  static int virtblk_probe(struct virtio_device *vdev)
 	if (err)
 		goto out_put_disk;
 
-	q = vblk->disk->queue = blk_mq_init_queue(&vblk->tag_set);
+	q = blk_mq_init_queue(&vblk->tag_set);
 	if (IS_ERR(q)) {
 		err = -ENOMEM;
 		goto out_free_tags;
 	}
+	vblk->disk->queue = q;
 
 	q->queuedata = vblk;