Skip to content

Commit

Permalink
virtio_blk: fix config handler race
Browse files Browse the repository at this point in the history
Fix a theoretical race related to config work
handler: a config interrupt might happen
after we flush config work but before we
reset the device. It will then cause the
config work to run during or after reset.

Two problems with this:
- if this runs after device is gone we will get use after free
- access of config while reset is in progress is racy
(as layout is changing).

As a solution
1. flush after reset when we know there will be no more interrupts
2. add a flag to disable config access before reset

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
  • Loading branch information
Michael S. Tsirkin authored and Rusty Russell committed Jan 12, 2012
1 parent e93300b commit 4678d6f
Showing 1 changed file with 21 additions and 1 deletion.
22 changes: 21 additions & 1 deletion drivers/block/virtio_blk.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <linux/blkdev.h>
#include <linux/hdreg.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/virtio.h>
#include <linux/virtio_blk.h>
#include <linux/scatterlist.h>
Expand Down Expand Up @@ -36,6 +37,12 @@ struct virtio_blk
/* Process context for config space updates */
struct work_struct config_work;

/* Lock for config space updates */
struct mutex config_lock;

/* enable config space updates */
bool config_enable;

/* What host tells us, plus 2 for header & tailer. */
unsigned int sg_elems;

Expand Down Expand Up @@ -318,6 +325,10 @@ static void virtblk_config_changed_work(struct work_struct *work)
char cap_str_2[10], cap_str_10[10];
u64 capacity, size;

mutex_lock(&vblk->config_lock);
if (!vblk->config_enable)
goto done;

/* Host must always specify the capacity. */
vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity),
&capacity, sizeof(capacity));
Expand All @@ -340,6 +351,8 @@ static void virtblk_config_changed_work(struct work_struct *work)
cap_str_10, cap_str_2);

set_capacity(vblk->disk, capacity);
done:
mutex_unlock(&vblk->config_lock);
}

static void virtblk_config_changed(struct virtio_device *vdev)
Expand Down Expand Up @@ -388,7 +401,9 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
vblk->vdev = vdev;
vblk->sg_elems = sg_elems;
sg_init_table(vblk->sg, vblk->sg_elems);
mutex_init(&vblk->config_lock);
INIT_WORK(&vblk->config_work, virtblk_config_changed_work);
vblk->config_enable = true;

/* We expect one virtqueue, for output. */
vblk->vq = virtio_find_single_vq(vdev, blk_done, "requests");
Expand Down Expand Up @@ -542,14 +557,19 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
struct virtio_blk *vblk = vdev->priv;
int index = vblk->index;

flush_work(&vblk->config_work);
/* Prevent config work handler from accessing the device. */
mutex_lock(&vblk->config_lock);
vblk->config_enable = false;
mutex_unlock(&vblk->config_lock);

/* Nothing should be pending. */
BUG_ON(!list_empty(&vblk->reqs));

/* Stop all the virtqueues. */
vdev->config->reset(vdev);

flush_work(&vblk->config_work);

del_gendisk(vblk->disk);
blk_cleanup_queue(vblk->disk->queue);
put_disk(vblk->disk);
Expand Down

0 comments on commit 4678d6f

Please sign in to comment.