Re: [PATCH v3 1/2] vhost: check for nr_vec == 0 in desc_to_mbuf, mbuf_to_desc

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 8/2/22 02:49, Claudio Fontana wrote:
> in virtio_dev_split we cannot currently call desc_to_mbuf with
> nr_vec == 0, or we end up trying to rte_memcpy from a source address
> buf_vec[0] that is an uninitialized stack variable.
> 
> Improve this in general by having desc_to_mbuf and mbuf_to_desc
> return -1 when called with an invalid nr_vec == 0, which should
> fix any other instance of this problem.
> 
> 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 when the
> guest DPDK application abruptly goes away via SIGKILL and then
> reconnects.
> 
> 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, 8 insertions(+), 3 deletions(-)

This patch is also no more necessary since CVE-2022-2132 has been fixed.
Latests LTS versions and upstream main branch contain the fixes:

dc1516e260a0 ("vhost: fix header spanned across more than two descriptors")
71bd0cc536ad ("vhost: discard too small descriptor chains")

desc_to_mbuf callers now check that the descriptors are at least the
size of the virtio_net header, so nr_vec cannot be 0 in desc_to_mbuf.

Since I cannot reproduce, if you are willing to try please let us know
the results.

Maxime