Skip to content

Commit b46a0bf

Browse files
jasowangdavem330
authored andcommitted
vhost: fix OOB in get_rx_bufs()
After batched used ring updating was introduced in commit e2b3b35 ("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 <[email protected]> ============================================================================= 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: e2b3b35 ("vhost_net: batch used ring update in rx") Signed-off-by: Jason Wang <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent bfe2599 commit b46a0bf

File tree

5 files changed

+11
-7
lines changed

5 files changed

+11
-7
lines changed

drivers/vhost/net.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1337,7 +1337,8 @@ static int vhost_net_open(struct inode *inode, struct file *f)
13371337
n->vqs[i].rx_ring = NULL;
13381338
vhost_net_buf_init(&n->vqs[i].rxq);
13391339
}
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);
13411342

13421343
vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, EPOLLOUT, dev);
13431344
vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, EPOLLIN, dev);

drivers/vhost/scsi.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1627,7 +1627,7 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
16271627
vqs[i] = &vs->vqs[i].vq;
16281628
vs->vqs[i].vq.handle_kick = vhost_scsi_handle_kick;
16291629
}
1630-
vhost_dev_init(&vs->dev, vqs, VHOST_SCSI_MAX_VQ);
1630+
vhost_dev_init(&vs->dev, vqs, VHOST_SCSI_MAX_VQ, UIO_MAXIOV);
16311631

16321632
vhost_scsi_init_inflight(vs, NULL);
16331633

drivers/vhost/vhost.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -390,9 +390,9 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
390390
vq->indirect = kmalloc_array(UIO_MAXIOV,
391391
sizeof(*vq->indirect),
392392
GFP_KERNEL);
393-
vq->log = kmalloc_array(UIO_MAXIOV, sizeof(*vq->log),
393+
vq->log = kmalloc_array(dev->iov_limit, sizeof(*vq->log),
394394
GFP_KERNEL);
395-
vq->heads = kmalloc_array(UIO_MAXIOV, sizeof(*vq->heads),
395+
vq->heads = kmalloc_array(dev->iov_limit, sizeof(*vq->heads),
396396
GFP_KERNEL);
397397
if (!vq->indirect || !vq->log || !vq->heads)
398398
goto err_nomem;
@@ -414,7 +414,7 @@ static void vhost_dev_free_iovecs(struct vhost_dev *dev)
414414
}
415415

416416
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)
418418
{
419419
struct vhost_virtqueue *vq;
420420
int i;
@@ -427,6 +427,7 @@ void vhost_dev_init(struct vhost_dev *dev,
427427
dev->iotlb = NULL;
428428
dev->mm = NULL;
429429
dev->worker = NULL;
430+
dev->iov_limit = iov_limit;
430431
init_llist_head(&dev->work_list);
431432
init_waitqueue_head(&dev->wait);
432433
INIT_LIST_HEAD(&dev->read_list);

drivers/vhost/vhost.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,11 @@ struct vhost_dev {
170170
struct list_head read_list;
171171
struct list_head pending_list;
172172
wait_queue_head_t wait;
173+
int iov_limit;
173174
};
174175

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);
176178
long vhost_dev_set_owner(struct vhost_dev *dev);
177179
bool vhost_dev_has_owner(struct vhost_dev *dev);
178180
long vhost_dev_check_owner(struct vhost_dev *);

drivers/vhost/vsock.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
531531
vsock->vqs[VSOCK_VQ_TX].handle_kick = vhost_vsock_handle_tx_kick;
532532
vsock->vqs[VSOCK_VQ_RX].handle_kick = vhost_vsock_handle_rx_kick;
533533

534-
vhost_dev_init(&vsock->dev, vqs, ARRAY_SIZE(vsock->vqs));
534+
vhost_dev_init(&vsock->dev, vqs, ARRAY_SIZE(vsock->vqs), UIO_MAXIOV);
535535

536536
file->private_data = vsock;
537537
spin_lock_init(&vsock->send_pkt_list_lock);

0 commit comments

Comments
 (0)