[1/2] vhost: fix error handling in virtio_dev_tx_split

Message ID 20220731201738.11733-2-cfontana@suse.de (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series vhost fixes for OVS SIGSEGV in PMD |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Claudio Fontana July 31, 2022, 8:17 p.m. UTC
  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(-)
  

Patch

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);