From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 738415595 for ; Fri, 15 Jul 2016 13:15:32 +0200 (CEST) Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout2.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0OAC00CF6SLU2B60@mailout2.w1.samsung.com> for dev@dpdk.org; Fri, 15 Jul 2016 12:15:30 +0100 (BST) X-AuditID: cbfec7f4-f796c6d000001486-58-5788c5d2aefc Received: from eusync4.samsung.com ( [203.254.199.214]) by eucpsbgm1.samsung.com (EUCPMTA) with SMTP id 83.EC.05254.2D5C8875; Fri, 15 Jul 2016 12:15:30 +0100 (BST) Received: from imaximets.rnd.samsung.ru ([106.109.129.180]) by eusync4.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0OAC005XOSL8G570@eusync4.samsung.com>; Fri, 15 Jul 2016 12:15:30 +0100 (BST) From: Ilya Maximets To: dev@dpdk.org, Huawei Xie , Yuanhan Liu , Rich Lane Cc: Dyasly Sergey , Heetae Ahn , Jianfeng Tan , Stephen Hemminger , Thomas Monjalon , Ilya Maximets Date: Fri, 15 Jul 2016 14:15:05 +0300 Message-id: <1468581305-11756-3-git-send-email-i.maximets@samsung.com> X-Mailer: git-send-email 2.7.4 In-reply-to: <1468581305-11756-1-git-send-email-i.maximets@samsung.com> References: <1463748604-27251-1-git-send-email-i.maximets@samsung.com> <1468581305-11756-1-git-send-email-i.maximets@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrHLMWRmVeSWpSXmKPExsVy+t/xa7qXjnaEGxyex2fx7tN2Jotpn2+z W7TPPMtkcaX9J7tF9+wvbBab3k1itZg8W8pi8R05iy+bprNZXJ9wgdWBy+Ni/x1GjweXbzJ5 /FqwlNVj8Z6XTB7zTgZ69Jycx+TRt2UVYwB7FJdNSmpOZllqkb5dAlfGnf2n2QpWylRs/HuU qYFxrngXIyeHhICJxOTbb9ghbDGJC/fWs3UxcnEICSxllLi2/xELhNPKJHH/znFGkCo2AR2J U6uPMIIkRARaGSUe/58G1sIs0Mwkse3DE7BZwgJeEqvP/GYFsVkEVCW6f19mAbF5Bdwk3t3r ZoLYJydx81wnM4jNKeAucXb3S6jdzYwSLxtOsU5g5F3AyLCKUTS1NLmgOCk911CvODG3uDQv XS85P3cTIyQUv+xgXHzM6hCjAAejEg+vwNH2cCHWxLLiytxDjBIczEoivG8OdYQL8aYkVlal FuXHF5XmpBYfYpTmYFES5527632IkEB6YklqdmpqQWoRTJaJg1OqgTHekSH+hMTczQ5a6ez/ vvL8snhrceBgYInrW+a3IpPXCZUvEbvNfLFBXCec2WfqwcnyEd5eW789dvf7Mr/xzmSJHZnb Ha+IZjMz9h6PDkx+kW9e9TYjc4d2UZlw/MPVe1IcLazf33F/P/P9nVNTN04SnHq9ZmHRjzLH hrfv+54JythYdV058UuJpTgj0VCLuag4EQB7YMMOQQIAAA== Subject: [dpdk-dev] [PATCH v3 2/2] vhost: do sanity check for ring descriptor address X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Jul 2016 11:15:32 -0000 In current implementation vhost will crash with segmentation fault if malicious or buggy virtio application breaks addresses of descriptors. Before commit 0823c1cb0a73 ("vhost: workaround stale vring base") this crash was reproducible even with normal DPDK application that tries to change number of virtqueues dynamically inside VM. Fix that by checking addresses of descriptors before using. Signed-off-by: Ilya Maximets --- lib/librte_vhost/vhost_rxtx.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c index a9b04df..bc00518 100644 --- a/lib/librte_vhost/vhost_rxtx.c +++ b/lib/librte_vhost/vhost_rxtx.c @@ -147,10 +147,15 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, struct virtio_net_hdr_mrg_rxbuf virtio_hdr = {{0, 0, 0, 0, 0, 0}, 0}; desc = &vq->desc[desc_idx]; - if (unlikely(desc->len < dev->vhost_hlen)) + desc_addr = gpa_to_vva(dev, desc->addr); + /* + * Checking of 'desc_addr' placed outside of 'unlikely' macro to avoid + * performance issue with some versions of gcc (4.8.4 and 5.3.0) which + * otherwise stores offset on the stack instead of in a register. + */ + if (unlikely(desc->len < dev->vhost_hlen) || !desc_addr) return -1; - desc_addr = gpa_to_vva(dev, desc->addr); rte_prefetch0((void *)(uintptr_t)desc_addr); virtio_enqueue_offload(m, &virtio_hdr.hdr); @@ -182,7 +187,10 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, return -1; desc = &vq->desc[desc->next]; - desc_addr = gpa_to_vva(dev, desc->addr); + desc_addr = gpa_to_vva(dev, desc->addr); + if (unlikely(!desc_addr)) + return -1; + desc_offset = 0; desc_avail = desc->len; } @@ -387,10 +395,10 @@ copy_mbuf_to_desc_mergeable(struct virtio_net *dev, struct vhost_virtqueue *vq, LOG_DEBUG(VHOST_DATA, "(%d) current index %d | end index %d\n", dev->vid, cur_idx, end_idx); - if (buf_vec[vec_idx].buf_len < dev->vhost_hlen) + desc_addr = gpa_to_vva(dev, buf_vec[vec_idx].buf_addr); + if (buf_vec[vec_idx].buf_len < dev->vhost_hlen || !desc_addr) return 0; - desc_addr = gpa_to_vva(dev, buf_vec[vec_idx].buf_addr); rte_prefetch0((void *)(uintptr_t)desc_addr); virtio_hdr.num_buffers = end_idx - start_idx; @@ -425,6 +433,8 @@ copy_mbuf_to_desc_mergeable(struct virtio_net *dev, struct vhost_virtqueue *vq, vec_idx++; desc_addr = gpa_to_vva(dev, buf_vec[vec_idx].buf_addr); + if (unlikely(!desc_addr)) + return 0; /* Prefetch buffer address. */ rte_prefetch0((void *)(uintptr_t)desc_addr); @@ -688,6 +698,9 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq, return -1; desc_addr = gpa_to_vva(dev, desc->addr); + if (unlikely(!desc_addr)) + return -1; + hdr = (struct virtio_net_hdr *)((uintptr_t)desc_addr); rte_prefetch0(hdr); @@ -701,6 +714,9 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq, desc = &vq->desc[desc->next]; desc_addr = gpa_to_vva(dev, desc->addr); + if (unlikely(!desc_addr)) + return -1; + rte_prefetch0((void *)(uintptr_t)desc_addr); desc_offset = 0; @@ -737,6 +753,9 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq, desc = &vq->desc[desc->next]; desc_addr = gpa_to_vva(dev, desc->addr); + if (unlikely(!desc_addr)) + return -1; + rte_prefetch0((void *)(uintptr_t)desc_addr); desc_offset = 0; -- 2.7.4