summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLuca Vizzarro <luca.vizzarro@arm.com>2024-04-10 16:21:01 +0100
committerMaxime Coquelin <maxime.coquelin@redhat.com>2024-06-07 15:26:07 +0200
commita16f688e142d721b1f5fcc9ad43fa87c652b170d (patch)
treee658542d674b469520710293dd5087e38923e72e
parent9bd8346d6432bc71ee56597d969580407623c390 (diff)
vhost: fix build with GCC 13
This patch resolves a build error with GCC 13 and arm/aarch32 as targets: In function ‘mbuf_to_desc’, inlined from ‘vhost_enqueue_async_packed’ at ../lib/vhost/virtio_net.c:1828:6, inlined from ‘virtio_dev_rx_async_packed’ at ../lib/vhost/virtio_net.c:1842:6, inlined from ‘virtio_dev_rx_async_submit_packed’ at ../lib/vhost/virtio_net.c:1900:7: ../lib/vhost/virtio_net.c:1159:18: error: ‘buf_vec[0].buf_addr’ may be used uninitialized [-Werror=maybe-uninitialized] 1159 | buf_addr = buf_vec[vec_idx].buf_addr; | ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ <snip> ../lib/vhost/virtio_net.c:1160:18: error: ‘buf_vec[0].buf_iova’ may be used uninitialized [-Werror=maybe-uninitialized] 1160 | buf_iova = buf_vec[vec_idx].buf_iova; | ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ <snip> ../lib/vhost/virtio_net.c:1161:35: error: ‘buf_vec[0].buf_len’ may be used uninitialized [-Werror=maybe-uninitialized] 1161 | buf_len = buf_vec[vec_idx].buf_len; | ~~~~~~~~~~~~~~~~^~~~~~~~ GCC complains about the possible runtime path where the while loop which fills buf_vec (in vhost_enqueue_async_packed) is not run. As a consequence it correctly thinks that buf_vec is not initialized while being accessed anyways. This scenario is actually very unlikely as the only way this can occur is if size has overflowed to 0. Meaning that the total packet length would be close to UINT64_MAX (or actually UINT32_MAX). At first glance, the code suggests that this may never happen as the type of size has been changed to 64-bit. For a 32-bit architecture such as arm (e.g. armv7-a) and aarch32, this still happens because the operand types (pkt->pkt_len and sizeof) are 32-bit wide, performing 32-bit arithmetic first (where the overflow can happen) and widening to 64-bit later. The proposed fix simply guarantees to the compiler that the scope which fills buf_vec is accessed at least once, while not disrupting the actual logic. This is based on the assumption that size will always be greater than 0, as suggested by the sizeof, and the packet length will never be as big as UINT32_MAX, and causing an overflow. Fixes: 873e8dad6f49 ("vhost: support packed ring in async datapath") Cc: stable@dpdk.org Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com> Reviewed-by: Nick Connolly <nick.connolly@arm.com> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
-rw-r--r--.mailmap2
-rw-r--r--lib/vhost/virtio_net.c4
2 files changed, 3 insertions, 3 deletions
diff --git a/.mailmap b/.mailmap
index 111dbb0ac3..ed63605a15 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1030,7 +1030,7 @@ Netanel Belgazal <netanel@amazon.com>
Netanel Gonen <netanelg@mellanox.com>
Niall Power <niall.power@intel.com>
Nicholas Pratte <npratte@iol.unh.edu>
-Nick Connolly <nick.connolly@mayadata.io>
+Nick Connolly <nick.connolly@arm.com> <nick.connolly@mayadata.io>
Nick Nunley <nicholas.d.nunley@intel.com>
Niclas Storm <niclas.storm@ericsson.com>
Nicolas Chautru <nicolas.chautru@intel.com>
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index b406b5d7d9..370402d849 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1935,7 +1935,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
else
max_tries = 1;
- while (size > 0) {
+ do {
/*
* if we tried all available ring items, and still
* can't get enough buf, it means something abnormal
@@ -1962,7 +1962,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
avail_idx += desc_count;
if (avail_idx >= vq->size)
avail_idx -= vq->size;
- }
+ } while (size > 0);
if (unlikely(mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec, *nr_buffers, true) < 0))
return -1;