Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport fix #69

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Backport fix #69

wants to merge 14 commits into from

Conversation

Ch3n60x
Copy link
Collaborator

@Ch3n60x Ch3n60x commented Mar 27, 2024

This PR backport vhost lib fixes from DPDK LTS 21.11

wenwumax and others added 14 commits March 26, 2024 05:37
[ upstream commit 9e89b06 ]

In vhost_user_msg_handler(), if vhost message handling
failed, we should check whether the queue is locked and
release the lock before returning. Or, it will cause a
deadlock later.

Fixes: 7f31d4e ("vhost: fix lock on device readiness notification")

Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
Tested-by: Wei Ling <weix.ling@intel.com>
Acked-by: David Marchand <david.marchand@redhat.com>
[ upstream commit 1ef468a ]

VHOST_LOG_* macros don't append a newline.
Add missing ones.

Fixes: e623e0c ("vhost: add reconnect ability")
Fixes: af14759 ("vhost: introduce API to start a specific driver")
Fixes: 2dfeebe ("vhost: check return of mutex initialization")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
[ upstream commit bb15129 ]

device information in the log messages was dropped.

Fixes: 52ade97 ("vhost: fix physical address mapping")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
[ upstream commit 0b2a2ca ]

translate_ring_addresses (via numa_realloc) may change a virtio device and
virtio queue.
The virtqueue object must be refreshed before accessing the lock.

Fixes: 04c27cb ("vhost: fix unsafe vring addresses modifications")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
[ upstream commit 4226aa9 ]

This patch fixes a compilation issue met with GCC 12 on
LoongArch64:

In function ‘mbuf_to_desc’,
    inlined from ‘vhost_enqueue_async_packed’
    inlined from ‘virtio_dev_rx_async_packed’
    inlined from ‘virtio_dev_rx_async_submit_packed’
lib/vhost/virtio_net.c:1159:18: error:
        ‘buf_vec[0].buf_addr’ may be used uninitialized
 1159 |         buf_addr = buf_vec[vec_idx].buf_addr;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
lib/vhost/virtio_net.c:1834:27: note: ‘buf_vec’ declared here
 1834 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
      |                           ^~~~~~~

It happens because the compiler assumes that 'size'
variable in vhost_enqueue_async_packed could wrap to 0 since
'size' is uint32_t and pkt->pkt_len too.

In practice, it would never happen since 'pkt->pkt_len' is
unlikely to be close to UINT32_MAX, but let's just change
'size' to uint64_t to make the compiler happy without
having to add runtime checks.

This patch also fixes similar patterns in three other
places, including one that also produces similar build
issue on ARM64 in vhost_enqueue_single_packed().

Fixes: 873e8da ("vhost: support packed ring in async datapath")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
Tested-by: Amit Prakash Shukla <amitprakashs@marvell.com>
[ upstream commit 830f7e7 ]

Vhost-user library locks all VQ's access lock when processing
vring based messages, such as SET_VRING_KICK and SET_VRING_CALL,
and the data processing thread may already be started, e.g: SPDK
vhost-blk and vhost-scsi will start the data processing thread
when one vring is ready, then deadlock may happen when SPDK is
posting interrupts to VM.  Here, we add a new API which allows
caller to try again later for this case.

Bugzilla ID: 1015
Fixes: c573699 ("vhost: fix missing virtqueue lock protection")

Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
[ upstream commit 6546b60 ]

This variable is not used.

Fixes: abeb865 ("vhost: remove copy threshold for async path")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
[ upstream commit 43ccd55 ]

This patch changes VHOST_USER_SET_VRING_ERR and
VHOST_USER_SET_LOG_FD "not implemented" log levels from
INFO to DEBUG, as implementing these requests is not
mandatory. Having them being displayed at INFO level
may induce some confusion to the end-user.

Fixes: fd29c33 ("vhost: handle unsupported message types in functions")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
Acked-by: Kevin Traynor <ktraynor@redhat.com>
[ upstream commit 218daf1 ]

This patch fixes possible FDs leaks when truncation happens
on either the message buffer or its control data. Indeed,
by returning early, it did not let a chance to retrieve the
FDs passed as ancillary data, and so caused a potential FDs
leak.

This patch fixes this by extracting the FDs from the
ancillary data as long as recvmsg() call succeeded. It also
improves the logs to differentiate between MSG_TRUNC and
MSG_CTRUNC.

Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
[ upstream commit 1c80a40 ]

The net/vhost pmd currently provides a -1 vid when disabling interrupt
after a virtio port got disconnected.

This can be caught when running with ASan.

First, start dpdk-l3fwd-power in interrupt mode with a net/vhost port.

$ ./build-clang/examples/dpdk-l3fwd-power -l0,1 --in-memory \
	-a 0000:00:00.0 \
	--vdev net_vhost0,iface=plop.sock,client=1\
	-- \
	-p 0x1 \
	--interrupt-only \
	--config '(0,0,1)' \
	--parse-ptype 0

Then start testpmd with virtio-user.

$ ./build-clang/app/dpdk-testpmd -l0,2 --single-file-segment --in-memory \
	-a 0000:00:00.0 \
	--vdev net_virtio_user0,path=plop.sock,server=1 \
	-- \
	-i

Finally stop testpmd.
ASan then splats in dpdk-l3fwd-power:

=================================================================
==3641005==ERROR: AddressSanitizer: global-buffer-overflow on address
	0x000005ed0778 at pc 0x000001270f81 bp 0x7fddbd2eee20
	sp 0x7fddbd2eee18
READ of size 8 at 0x000005ed0778 thread T2
    #0 0x1270f80 in get_device .../lib/vhost/vhost.h:801:27
    Mellanox#1 0x1270f80 in rte_vhost_get_vhost_vring .../lib/vhost/vhost.c:951:8
    Mellanox#2 0x3ac95cb in eth_rxq_intr_disable
	.../drivers/net/vhost/rte_eth_vhost.c:647:8
    Mellanox#3 0x170e0bf in rte_eth_dev_rx_intr_disable
	.../lib/ethdev/rte_ethdev.c:5443:25
    Mellanox#4 0xf72ba7 in turn_on_off_intr .../examples/l3fwd-power/main.c:881:4
    Mellanox#5 0xf71045 in main_intr_loop .../examples/l3fwd-power/main.c:1061:6
    Mellanox#6 0x17f9292 in eal_thread_loop
	.../lib/eal/common/eal_common_thread.c:210:9
    Mellanox#7 0x18373f5 in eal_worker_thread_loop .../lib/eal/linux/eal.c:915:2
    Mellanox#8 0x7fddc16ae12c in start_thread (/lib64/libc.so.6+0x8b12c)
	(BuildId: 81daba31ee66dbd63efdc4252a872949d874d136)
    Mellanox#9 0x7fddc172fbbf in __GI___clone3 (/lib64/libc.so.6+0x10cbbf)
	(BuildId: 81daba31ee66dbd63efdc4252a872949d874d136)

0x000005ed0778 is located 8 bytes to the left of global variable
	'vhost_devices' defined in '.../lib/vhost/vhost.c:24'
	(0x5ed0780) of size 8192
0x000005ed0778 is located 20 bytes to the right of global variable
	'vhost_config_log_level' defined in '.../lib/vhost/vhost.c:2174'
	(0x5ed0760) of size 4
SUMMARY: AddressSanitizer: global-buffer-overflow
	.../lib/vhost/vhost.h:801:27 in get_device
Shadow bytes around the buggy address:
  0x000080bd2090: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x000080bd20a0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
  0x000080bd20b0: f9 f9 f9 f9 00 f9 f9 f9 00 f9 f9 f9 00 f9 f9 f9
  0x000080bd20c0: 00 00 00 00 00 00 00 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x000080bd20d0: 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
=>0x000080bd20e0: 00 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 04 f9 f9[f9]
  0x000080bd20f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080bd2100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080bd2110: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080bd2120: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080bd2130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
Thread T2 created by T0 here:
    #0 0xe98996 in __interceptor_pthread_create
	(.examples/dpdk-l3fwd-power+0xe98996)
	(BuildId: d0b984a3b0287b9e0f301b73426fa921aeecca3a)
    Mellanox#1 0x1836767 in eal_worker_thread_create .../lib/eal/linux/eal.c:952:6
    Mellanox#2 0x1834b83 in rte_eal_init .../lib/eal/linux/eal.c:1257:9
    Mellanox#3 0xf68902 in main .../examples/l3fwd-power/main.c:2496:8
    Mellanox#4 0x7fddc164a50f in __libc_start_call_main (/lib64/libc.so.6+0x2750f)
	(BuildId: 81daba31ee66dbd63efdc4252a872949d874d136)

==3641005==ABORTING

More generally, any application passing an incorrect vid would trigger
such an OOB access.

Fixes: 4796ad6 ("examples/vhost: import userspace vhost application")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
[ upstream commit 585283f ]

On failure, read_vhost_message() only closed the message
FDs if the header size was unexpected, but there are other
cases where it is required. For example in the case the
payload size read from the header is greater than the
expected maximum payload size.

This patch fixes this by closing all messages FDs in all
error cases.

Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
[ upstream commit 0445f81 ]

This patch fixes cases where IRQ injection is tried while
the call FD is not valid, which should not happen.

Fixes: b1cce26 ("vhost: add notification for packed ring")
Fixes: e37ff95 ("vhost: support virtqueue interrupt/notification suppression")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
[ upstream commit b3e42d9 ]

Calling vring_invalidate must be done with a (write) lock taken on the
virtqueue.

Fixes: 72d002b ("vhost: fix vring address handling during live migration")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
[ upstream commit 19639c3 ]

In a nested virtualization environment, running dpdk-vdpa
in QEMU-L1 for software live migration will result in a
deadlock between dpdk-vdpa and QEMU-L2 processes.
'rte_vdpa_relay_vring_used'->
'__vhost_iova_to_vva'->
'vhost_user_iotlb_rd_unlock(vq)'->
'vhost_user_iotlb_miss'-> send vhost message
'VHOST_USER_SLAVE_IOTLB_MSG' to QEMU-L2's vdpa socket, then
call 'vhost_user_iotlb_rd_lock(vq)' to hold the read lock
`iotlb_lock`.

But there is no place to release this read lock.

QEMU-L2 get the 'VHOST_USER_SLAVE_IOTLB_MSG', then call
'vhost_user_send_device_iotlb_msg' to send
'VHOST_USER_IOTLB_MSG' messages to dpdk-vdpa.

dpdk-vdpa will call vhost_user_iotlb_cache_insert and will
obtain the write lock `iotlb_lock`, but the read lock
`iotlb_lock` has not been released and will block here.

This patch add lock and unlock function to fix the deadlock.

Fixes: b13ad2d ("vhost: provide helpers for virtio ring relay")

Signed-off-by: Hao Chen <chenh@yusur.tech>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants