forked from DPDK/dpdk
-
Notifications
You must be signed in to change notification settings - Fork 11
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
Ch3n60x
wants to merge
14
commits into
Mellanox:main
Choose a base branch
from
Ch3n60x:backport_fix
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Backport fix #69
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[ 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 <[email protected]> Reviewed-by: Chenbo Xia <[email protected]> Tested-by: Wei Ling <[email protected]> Acked-by: David Marchand <[email protected]>
[ 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 <[email protected]> Reviewed-by: Chenbo Xia <[email protected]>
[ upstream commit bb15129 ] device information in the log messages was dropped. Fixes: 52ade97 ("vhost: fix physical address mapping") Signed-off-by: David Marchand <[email protected]> Reviewed-by: Chenbo Xia <[email protected]>
[ 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 <[email protected]> Reviewed-by: Maxime Coquelin <[email protected]>
[ 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 <[email protected]> Reviewed-by: David Marchand <[email protected]> Tested-by: Amit Prakash Shukla <[email protected]>
[ 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 <[email protected]> Reviewed-by: Chenbo Xia <[email protected]>
[ upstream commit 6546b60 ] This variable is not used. Fixes: abeb865 ("vhost: remove copy threshold for async path") Signed-off-by: David Marchand <[email protected]> Reviewed-by: Maxime Coquelin <[email protected]> Acked-by: Tyler Retzlaff <[email protected]>
[ 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 <[email protected]> Reviewed-by: Chenbo Xia <[email protected]> Acked-by: Kevin Traynor <[email protected]>
[ 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 <[email protected]> Reviewed-by: David Marchand <[email protected]> Reviewed-by: Chenbo Xia <[email protected]>
[ 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 <[email protected]> Reviewed-by: Maxime Coquelin <[email protected]>
[ 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 <[email protected]> Signed-off-by: David Marchand <[email protected]> Reviewed-by: Chenbo Xia <[email protected]>
[ 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 <[email protected]> Signed-off-by: Eelco Chaudron <[email protected]> Reviewed-by: Chenbo Xia <[email protected]>
[ 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 <[email protected]> Acked-by: Eelco Chaudron <[email protected]> Reviewed-by: Maxime Coquelin <[email protected]>
[ 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 <[email protected]> Reviewed-by: Maxime Coquelin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR backport vhost lib fixes from DPDK LTS 21.11