Skip to content

Commit

Permalink
vhost: fix a theoretical race in device cleanup
Browse files Browse the repository at this point in the history
vhost_zerocopy_callback accesses VQ right after it drops a ubuf
reference.  In theory, this could race with device removal which waits
on the ubuf kref, and crash on use after free.

Do all accesses within rcu read side critical section, and synchronize
on release.

Since callbacks are always invoked from bh, synchronize_rcu_bh seems
enough and will help release complete a bit faster.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Michael S. Tsirkin authored and David S. Miller committed Feb 13, 2014
1 parent 0ad8b48 commit b0c057c
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions drivers/vhost/net.c
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,8 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
struct vhost_virtqueue *vq = ubufs->vq;
int cnt;

rcu_read_lock_bh();

/* set len to mark this desc buffers done DMA */
vq->heads[ubuf->desc].len = success ?
VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN;
Expand All @@ -322,6 +324,8 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
*/
if (cnt <= 1 || !(cnt % 16))
vhost_poll_queue(&vq->poll);

rcu_read_unlock_bh();
}

/* Expects to be always run from workqueue - which acts as
Expand Down Expand Up @@ -799,6 +803,8 @@ static int vhost_net_release(struct inode *inode, struct file *f)
fput(tx_sock->file);
if (rx_sock)
fput(rx_sock->file);
/* Make sure no callbacks are outstanding */
synchronize_rcu_bh();
/* We do an extra flush before freeing memory,
* since jobs can re-queue themselves. */
vhost_net_flush(n);
Expand Down

0 comments on commit b0c057c

Please sign in to comment.