* [PATCH 0/2] vhost fixes for OVS SIGSEGV in PMD @ 2022-08-01 11:53 Claudio Fontana 2022-08-01 11:53 ` [PATCH 1/2] vhost: fix error handling in virtio_dev_tx_split Claudio Fontana 2022-08-01 11:53 ` [PATCH 2/2] vhost: improve error handling in desc_to_mbuf Claudio Fontana 0 siblings, 2 replies; 4+ messages in thread From: Claudio Fontana @ 2022-08-01 11:53 UTC (permalink / raw) To: dev; +Cc: Claudio Fontana Hi, the real meat is in patch 1/2, which fixes a segmentation fault in the OVS PMD thread when resynchronizing with QEMU after the guest application has been killed with SIGKILL. This fixes an issue where the guest DPDK application is able to crash the OVS process on the host. Patch 2/2 is just an improvement in the current error handling. For your review and comments, Claudio Claudio Fontana (2): vhost: fix error handling in virtio_dev_tx_split vhost: improve error handling in desc_to_mbuf lib/vhost/virtio_net.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] vhost: fix error handling in virtio_dev_tx_split 2022-08-01 11:53 [PATCH 0/2] vhost fixes for OVS SIGSEGV in PMD Claudio Fontana @ 2022-08-01 11:53 ` Claudio Fontana 2022-08-01 11:53 ` [PATCH 2/2] vhost: improve error handling in desc_to_mbuf Claudio Fontana 1 sibling, 0 replies; 4+ messages in thread From: Claudio Fontana @ 2022-08-01 11:53 UTC (permalink / raw) To: dev; +Cc: Claudio Fontana in virtio_dev_split we add a check for invalid nr_vec, mainly for nr_vec == 0 (but add a check for BUF_VECTOR_MAX too), and bail out before calling desc_to_mbuf, otherwise in desc_to_mbuf we end up trying to memcpy from a source address buf_vec[0] that is an uninitialized stack variable. This should fix errors that have been reported in multiple occasions from telcos to the DPDK, OVS and QEMU projects, as this affects in particular the openvswitch/DPDK, QEMU vhost-user setup. The back trace looks roughly like this, depending on the specific rte_memcpy selected, etc, in any case the "src" parameter is garbage (in this example containing 0 + dev->host_hlen(12 = 0xc)). Thread 153 "pmd-c88/id:150" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7f64e5e6b700 (LWP 141373)] rte_mov128blocks (n=2048, src=0xc <error: Cannot access memory at 0xc>, dst=0x150da4480) at ../lib/eal/x86/include/rte_memcpy.h:384 (gdb) bt 0 rte_mov128blocks (n=2048, src=0xc, dst=0x150da4480) 1 rte_memcpy_generic (n=2048, src=0xc, dst=0x150da4480) 2 rte_memcpy (n=2048, src=0xc, dst=<optimized out>) 3 sync_fill_seg 4 desc_to_mbuf 5 virtio_dev_tx_split 6 virtio_dev_tx_split_legacy 7 0x00007f676fea0fef in rte_vhost_dequeue_burst 8 0x00007f6772005a62 in netdev_dpdk_vhost_rxq_recv 9 0x00007f6771f38116 in netdev_rxq_recv 10 0x00007f6771f03d96 in dp_netdev_process_rxq_port 11 0x00007f6771f04239 in pmd_thread_main 12 0x00007f6771f92aff in ovsthread_wrapper 13 0x00007f6771c1b6ea in start_thread 14 0x00007f6771933a8f in clone Tested-by: Claudio Fontana <cfontana@suse.de> Signed-off-by: Claudio Fontana <cfontana@suse.de> --- lib/vhost/virtio_net.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 35fa4670fd..0b8db2046e 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -2917,9 +2917,16 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, vq->last_avail_idx + i, &nr_vec, buf_vec, &head_idx, &buf_len, - VHOST_ACCESS_RO) < 0)) + VHOST_ACCESS_RO) < 0)) { + dropped += 1; + i++; break; - + } + if (unlikely(nr_vec < 1 || nr_vec >= BUF_VECTOR_MAX)) { + dropped += 1; + i++; + break; + } update_shadow_used_ring_split(vq, head_idx, 0); err = virtio_dev_pktmbuf_prep(dev, pkts[i], buf_len); -- 2.26.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] vhost: improve error handling in desc_to_mbuf 2022-08-01 11:53 [PATCH 0/2] vhost fixes for OVS SIGSEGV in PMD Claudio Fontana 2022-08-01 11:53 ` [PATCH 1/2] vhost: fix error handling in virtio_dev_tx_split Claudio Fontana @ 2022-08-01 11:53 ` Claudio Fontana 1 sibling, 0 replies; 4+ messages in thread From: Claudio Fontana @ 2022-08-01 11:53 UTC (permalink / raw) To: dev; +Cc: Claudio Fontana check when increasing vec_idx that it is still valid in the (buf_len < dev->vhost_hlen) case too. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- lib/vhost/virtio_net.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 0b8db2046e..6d34feaf73 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -2701,12 +2701,15 @@ desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq, if (unlikely(buf_len < dev->vhost_hlen)) { buf_offset = dev->vhost_hlen - buf_len; vec_idx++; + if (unlikely(vec_idx >= nr_vec)) + goto error; buf_addr = buf_vec[vec_idx].buf_addr; buf_iova = buf_vec[vec_idx].buf_iova; buf_len = buf_vec[vec_idx].buf_len; buf_avail = buf_len - buf_offset; } else if (buf_len == dev->vhost_hlen) { - if (unlikely(++vec_idx >= nr_vec)) + vec_idx++; + if (unlikely(vec_idx >= nr_vec)) goto error; buf_addr = buf_vec[vec_idx].buf_addr; buf_iova = buf_vec[vec_idx].buf_iova; -- 2.26.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 0/2] vhost fixes for OVS SIGSEGV in PMD @ 2022-07-31 20:17 Claudio Fontana 2022-07-31 20:17 ` [PATCH 1/2] vhost: fix error handling in virtio_dev_tx_split Claudio Fontana 0 siblings, 1 reply; 4+ messages in thread From: Claudio Fontana @ 2022-07-31 20:17 UTC (permalink / raw) To: dev; +Cc: Claudio Fontana Hi, the real meat is in patch 1/2, which fixes a segmentation fault in the OVS PMD thread when resynchronizing with QEMU after the guest application has been killed with SIGKILL. This fixes an issue where the guest DPDK application is able to crash the OVS process on the host. Patch 2/2 is just an improvement in the current error handling. For your review and comments, Claudio Claudio Fontana (2): vhost: fix error handling in virtio_dev_tx_split vhost: improve error handling in desc_to_mbuf lib/vhost/virtio_net.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] vhost: fix error handling in virtio_dev_tx_split 2022-07-31 20:17 [PATCH 0/2] vhost fixes for OVS SIGSEGV in PMD Claudio Fontana @ 2022-07-31 20:17 ` Claudio Fontana 0 siblings, 0 replies; 4+ messages in thread From: Claudio Fontana @ 2022-07-31 20:17 UTC (permalink / raw) To: dev; +Cc: Claudio Fontana in virtio_dev_split we add a check for invalid nr_vec, mainly for nr_vec == 0 (but add a check for BUF_VECTOR_MAX too), and bail out before calling desc_to_mbuf, otherwise in desc_to_mbuf we end up trying to memcpy from a source address buf_vec[0] that is an uninitialized stack variable. This should fix errors that have been reported in multiple occasions from telcos to the DPDK, OVS and QEMU projects, as this affects in particular the openvswitch/DPDK, QEMU vhost-user setup. The back trace looks roughly like this, depending on the specific rte_memcpy selected, etc, in any case the "src" parameter is garbage (in this example containing 0 + dev->host_hlen(12 = 0xc)). Thread 153 "pmd-c88/id:150" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7f64e5e6b700 (LWP 141373)] rte_mov128blocks (n=2048, src=0xc <error: Cannot access memory at address 0xc>, dst=0x150da4480) at ../lib/eal/x86/include/rte_memcpy.h:384 (gdb) bt 0 rte_mov128blocks (n=2048, src=0xc, dst=0x150da4480) 1 rte_memcpy_generic (n=2048, src=0xc, dst=0x150da4480) 2 rte_memcpy (n=2048, src=0xc, dst=<optimized out>) 3 sync_fill_seg 4 desc_to_mbuf 5 virtio_dev_tx_split 6 virtio_dev_tx_split_legacy 7 0x00007f676fea0fef in rte_vhost_dequeue_burst 8 0x00007f6772005a62 in netdev_dpdk_vhost_rxq_recv 9 0x00007f6771f38116 in netdev_rxq_recv 10 0x00007f6771f03d96 in dp_netdev_process_rxq_port 11 0x00007f6771f04239 in pmd_thread_main 12 0x00007f6771f92aff in ovsthread_wrapper 13 0x00007f6771c1b6ea in start_thread 14 0x00007f6771933a8f in clone Tested-by: Claudio Fontana <cfontana@suse.de> Signed-off-by: Claudio Fontana <cfontana@suse.de> --- lib/vhost/virtio_net.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 35fa4670fd..0b8db2046e 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -2917,9 +2917,16 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, vq->last_avail_idx + i, &nr_vec, buf_vec, &head_idx, &buf_len, - VHOST_ACCESS_RO) < 0)) + VHOST_ACCESS_RO) < 0)) { + dropped += 1; + i++; break; - + } + if (unlikely(nr_vec < 1 || nr_vec >= BUF_VECTOR_MAX)) { + dropped += 1; + i++; + break; + } update_shadow_used_ring_split(vq, head_idx, 0); err = virtio_dev_pktmbuf_prep(dev, pkts[i], buf_len); -- 2.26.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-01 11:53 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-01 11:53 [PATCH 0/2] vhost fixes for OVS SIGSEGV in PMD Claudio Fontana 2022-08-01 11:53 ` [PATCH 1/2] vhost: fix error handling in virtio_dev_tx_split Claudio Fontana 2022-08-01 11:53 ` [PATCH 2/2] vhost: improve error handling in desc_to_mbuf Claudio Fontana -- strict thread matches above, loose matches on Subject: below -- 2022-07-31 20:17 [PATCH 0/2] vhost fixes for OVS SIGSEGV in PMD Claudio Fontana 2022-07-31 20:17 ` [PATCH 1/2] vhost: fix error handling in virtio_dev_tx_split Claudio Fontana
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).