From 2a2904fa9cc44493bcea495bab944b032b24f7cb Mon Sep 17 00:00:00 2001 From: Tiwei Bie Date: Fri, 22 Feb 2019 10:42:08 +0800 Subject: [PATCH] vhost: fix potential use-after-free for memory region Reclaim outstanding zmbufs first before freeing memory regions, otherwise there could be use-after-free. Fixes: b0a985d1f340 ("vhost: add dequeue zero copy") Cc: stable@dpdk.org Signed-off-by: Tiwei Bie Reviewed-by: Maxime Coquelin --- lib/librte_vhost/vhost.h | 6 +++++ lib/librte_vhost/vhost_user.c | 46 +++++++++++++++++++++++++---------- lib/librte_vhost/virtio_net.c | 6 ----- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h index 044651b199..f008ec43bf 100644 --- a/lib/librte_vhost/vhost.h +++ b/lib/librte_vhost/vhost.h @@ -769,4 +769,10 @@ mbuf_is_consumed(struct rte_mbuf *m) return true; } +static __rte_always_inline void +put_zmbuf(struct zcopy_mbuf *zmbuf) +{ + zmbuf->in_use = 0; +} + #endif /* _VHOST_NET_CDEV_H_ */ diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 6d82535149..36c0c676db 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -93,15 +93,47 @@ get_blk_size(int fd) return ret == -1 ? (uint64_t)-1 : (uint64_t)stat.st_blksize; } +/* + * Reclaim all the outstanding zmbufs for a virtqueue. + */ +static void +drain_zmbuf_list(struct vhost_virtqueue *vq) +{ + struct zcopy_mbuf *zmbuf, *next; + + for (zmbuf = TAILQ_FIRST(&vq->zmbuf_list); + zmbuf != NULL; zmbuf = next) { + next = TAILQ_NEXT(zmbuf, next); + + while (!mbuf_is_consumed(zmbuf->mbuf)) + usleep(1000); + + TAILQ_REMOVE(&vq->zmbuf_list, zmbuf, next); + restore_mbuf(zmbuf->mbuf); + rte_pktmbuf_free(zmbuf->mbuf); + put_zmbuf(zmbuf); + vq->nr_zmbuf -= 1; + } +} + static void free_mem_region(struct virtio_net *dev) { uint32_t i; struct rte_vhost_mem_region *reg; + struct vhost_virtqueue *vq; if (!dev || !dev->mem) return; + if (dev->dequeue_zero_copy) { + for (i = 0; i < dev->nr_vring; i++) { + vq = dev->virtqueue[i]; + if (vq) + drain_zmbuf_list(vq); + } + } + for (i = 0; i < dev->mem->nregions; i++) { reg = &dev->mem->regions[i]; if (reg->host_user_addr) { @@ -1212,19 +1244,7 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, struct VhostUserMsg *msg, static void free_zmbufs(struct vhost_virtqueue *vq) { - struct zcopy_mbuf *zmbuf, *next; - - for (zmbuf = TAILQ_FIRST(&vq->zmbuf_list); - zmbuf != NULL; zmbuf = next) { - next = TAILQ_NEXT(zmbuf, next); - - while (!mbuf_is_consumed(zmbuf->mbuf)) - usleep(1000); - - restore_mbuf(zmbuf->mbuf); - rte_pktmbuf_free(zmbuf->mbuf); - TAILQ_REMOVE(&vq->zmbuf_list, zmbuf, next); - } + drain_zmbuf_list(vq); rte_free(vq->zmbufs); } diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index 40a2923640..a6a33a1013 100644 --- a/lib/librte_vhost/virtio_net.c +++ b/lib/librte_vhost/virtio_net.c @@ -1063,12 +1063,6 @@ vhost_dequeue_offload(struct virtio_net_hdr *hdr, struct rte_mbuf *m) } } -static __rte_always_inline void -put_zmbuf(struct zcopy_mbuf *zmbuf) -{ - zmbuf->in_use = 0; -} - static __rte_always_inline int copy_desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq, struct buf_vector *buf_vec, uint16_t nr_vec, -- 2.20.1