diff options
author | Jason Wang <jasowang@redhat.com> | 2019-01-28 02:05:05 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2019-01-29 01:53:09 -0500 |
commit | b46a0bf78ad7b150ef5910da83859f7f5a514ffd (patch) | |
tree | e9a0c3681a146d2e1053b57f8a9a8c4e7f735db6 | |
parent | bfe2599dd2f958de54ccfb11b209797e737a99b5 (diff) |
vhost: fix OOB in get_rx_bufs()
After batched used ring updating was introduced in commit e2b3b35eb989
("vhost_net: batch used ring update in rx"). We tend to batch heads in
vq->heads for more than one packet. But the quota passed to
get_rx_bufs() was not correctly limited, which can result a OOB write
in vq->heads.
headcount = get_rx_bufs(vq, vq->heads + nvq->done_idx,
vhost_len, &in, vq_log, &log,
likely(mergeable) ? UIO_MAXIOV : 1);
UIO_MAXIOV was still used which is wrong since we could have batched
used in vq->heads, this will cause OOB if the next buffer needs more
than 960 (1024 (UIO_MAXIOV) - 64 (VHOST_NET_BATCH)) heads after we've
batched 64 (VHOST_NET_BATCH) heads:
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
=============================================================================
BUG kmalloc-8k (Tainted: G B ): Redzone overwritten
-----------------------------------------------------------------------------
INFO: 0x00000000fd93b7a2-0x00000000f0713384. First byte 0xa9 instead of 0xcc
INFO: Allocated in alloc_pd+0x22/0x60 age=3933677 cpu=2 pid=2674
kmem_cache_alloc_trace+0xbb/0x140
alloc_pd+0x22/0x60
gen8_ppgtt_create+0x11d/0x5f0
i915_ppgtt_create+0x16/0x80
i915_gem_create_context+0x248/0x390
i915_gem_context_create_ioctl+0x4b/0xe0
drm_ioctl_kernel+0xa5/0xf0
drm_ioctl+0x2ed/0x3a0
do_vfs_ioctl+0x9f/0x620
ksys_ioctl+0x6b/0x80
__x64_sys_ioctl+0x11/0x20
do_syscall_64+0x43/0xf0
entry_SYSCALL_64_after_hwframe+0x44/0xa9
INFO: Slab 0x00000000d13e87af objects=3 used=3 fp=0x (null) flags=0x200000000010201
INFO: Object 0x0000000003278802 @offset=17064 fp=0x00000000e2e6652b
Fixing this by allocating UIO_MAXIOV + VHOST_NET_BATCH iovs for
vhost-net. This is done through set the limitation through
vhost_dev_init(), then set_owner can allocate the number of iov in a
per device manner.
This fixes CVE-2018-16880.
Fixes: e2b3b35eb989 ("vhost_net: batch used ring update in rx")
Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | drivers/vhost/net.c | 3 | ||||
-rw-r--r-- | drivers/vhost/scsi.c | 2 | ||||
-rw-r--r-- | drivers/vhost/vhost.c | 7 | ||||
-rw-r--r-- | drivers/vhost/vhost.h | 4 | ||||
-rw-r--r-- | drivers/vhost/vsock.c | 2 |
5 files changed, 11 insertions, 7 deletions
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index bca86bf7189f..df51a35cf537 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c | |||
@@ -1337,7 +1337,8 @@ static int vhost_net_open(struct inode *inode, struct file *f) | |||
1337 | n->vqs[i].rx_ring = NULL; | 1337 | n->vqs[i].rx_ring = NULL; |
1338 | vhost_net_buf_init(&n->vqs[i].rxq); | 1338 | vhost_net_buf_init(&n->vqs[i].rxq); |
1339 | } | 1339 | } |
1340 | vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX); | 1340 | vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX, |
1341 | UIO_MAXIOV + VHOST_NET_BATCH); | ||
1341 | 1342 | ||
1342 | vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, EPOLLOUT, dev); | 1343 | vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, EPOLLOUT, dev); |
1343 | vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, EPOLLIN, dev); | 1344 | vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, EPOLLIN, dev); |
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index 344684f3e2e4..23593cb23dd0 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c | |||
@@ -1627,7 +1627,7 @@ static int vhost_scsi_open(struct inode *inode, struct file *f) | |||
1627 | vqs[i] = &vs->vqs[i].vq; | 1627 | vqs[i] = &vs->vqs[i].vq; |
1628 | vs->vqs[i].vq.handle_kick = vhost_scsi_handle_kick; | 1628 | vs->vqs[i].vq.handle_kick = vhost_scsi_handle_kick; |
1629 | } | 1629 | } |
1630 | vhost_dev_init(&vs->dev, vqs, VHOST_SCSI_MAX_VQ); | 1630 | vhost_dev_init(&vs->dev, vqs, VHOST_SCSI_MAX_VQ, UIO_MAXIOV); |
1631 | 1631 | ||
1632 | vhost_scsi_init_inflight(vs, NULL); | 1632 | vhost_scsi_init_inflight(vs, NULL); |
1633 | 1633 | ||
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 15a216cdd507..24a129fcdd61 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c | |||
@@ -390,9 +390,9 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev) | |||
390 | vq->indirect = kmalloc_array(UIO_MAXIOV, | 390 | vq->indirect = kmalloc_array(UIO_MAXIOV, |
391 | sizeof(*vq->indirect), | 391 | sizeof(*vq->indirect), |
392 | GFP_KERNEL); | 392 | GFP_KERNEL); |
393 | vq->log = kmalloc_array(UIO_MAXIOV, sizeof(*vq->log), | 393 | vq->log = kmalloc_array(dev->iov_limit, sizeof(*vq->log), |
394 | GFP_KERNEL); | 394 | GFP_KERNEL); |
395 | vq->heads = kmalloc_array(UIO_MAXIOV, sizeof(*vq->heads), | 395 | vq->heads = kmalloc_array(dev->iov_limit, sizeof(*vq->heads), |
396 | GFP_KERNEL); | 396 | GFP_KERNEL); |
397 | if (!vq->indirect || !vq->log || !vq->heads) | 397 | if (!vq->indirect || !vq->log || !vq->heads) |
398 | goto err_nomem; | 398 | goto err_nomem; |
@@ -414,7 +414,7 @@ static void vhost_dev_free_iovecs(struct vhost_dev *dev) | |||
414 | } | 414 | } |
415 | 415 | ||
416 | void vhost_dev_init(struct vhost_dev *dev, | 416 | void vhost_dev_init(struct vhost_dev *dev, |
417 | struct vhost_virtqueue **vqs, int nvqs) | 417 | struct vhost_virtqueue **vqs, int nvqs, int iov_limit) |
418 | { | 418 | { |
419 | struct vhost_virtqueue *vq; | 419 | struct vhost_virtqueue *vq; |
420 | int i; | 420 | int i; |
@@ -427,6 +427,7 @@ void vhost_dev_init(struct vhost_dev *dev, | |||
427 | dev->iotlb = NULL; | 427 | dev->iotlb = NULL; |
428 | dev->mm = NULL; | 428 | dev->mm = NULL; |
429 | dev->worker = NULL; | 429 | dev->worker = NULL; |
430 | dev->iov_limit = iov_limit; | ||
430 | init_llist_head(&dev->work_list); | 431 | init_llist_head(&dev->work_list); |
431 | init_waitqueue_head(&dev->wait); | 432 | init_waitqueue_head(&dev->wait); |
432 | INIT_LIST_HEAD(&dev->read_list); | 433 | INIT_LIST_HEAD(&dev->read_list); |
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 1b675dad5e05..9490e7ddb340 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h | |||
@@ -170,9 +170,11 @@ struct vhost_dev { | |||
170 | struct list_head read_list; | 170 | struct list_head read_list; |
171 | struct list_head pending_list; | 171 | struct list_head pending_list; |
172 | wait_queue_head_t wait; | 172 | wait_queue_head_t wait; |
173 | int iov_limit; | ||
173 | }; | 174 | }; |
174 | 175 | ||
175 | void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs, int nvqs); | 176 | void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs, |
177 | int nvqs, int iov_limit); | ||
176 | long vhost_dev_set_owner(struct vhost_dev *dev); | 178 | long vhost_dev_set_owner(struct vhost_dev *dev); |
177 | bool vhost_dev_has_owner(struct vhost_dev *dev); | 179 | bool vhost_dev_has_owner(struct vhost_dev *dev); |
178 | long vhost_dev_check_owner(struct vhost_dev *); | 180 | long vhost_dev_check_owner(struct vhost_dev *); |
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c index 3fbc068eaa9b..bb5fc0e9fbc2 100644 --- a/drivers/vhost/vsock.c +++ b/drivers/vhost/vsock.c | |||
@@ -531,7 +531,7 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file) | |||
531 | vsock->vqs[VSOCK_VQ_TX].handle_kick = vhost_vsock_handle_tx_kick; | 531 | vsock->vqs[VSOCK_VQ_TX].handle_kick = vhost_vsock_handle_tx_kick; |
532 | vsock->vqs[VSOCK_VQ_RX].handle_kick = vhost_vsock_handle_rx_kick; | 532 | vsock->vqs[VSOCK_VQ_RX].handle_kick = vhost_vsock_handle_rx_kick; |
533 | 533 | ||
534 | vhost_dev_init(&vsock->dev, vqs, ARRAY_SIZE(vsock->vqs)); | 534 | vhost_dev_init(&vsock->dev, vqs, ARRAY_SIZE(vsock->vqs), UIO_MAXIOV); |
535 | 535 | ||
536 | file->private_data = vsock; | 536 | file->private_data = vsock; |
537 | spin_lock_init(&vsock->send_pkt_list_lock); | 537 | spin_lock_init(&vsock->send_pkt_list_lock); |