patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Luca Vizzarro <luca.vizzarro@arm.com>
To: dev@dpdk.org
Cc: luca.boccassi@gmail.com, paul.szczepanek@arm.com,
	nick.connolly@arm.com, Luca Vizzarro <luca.vizzarro@arm.com>,
	stable@dpdk.org
Subject: [PATCH 1/1] vhost: fix GCC 13 build error
Date: Wed, 10 Apr 2024 16:21:01 +0100	[thread overview]
Message-ID: <20240410152101.3211244-2-luca.vizzarro@arm.com> (raw)
In-Reply-To: <20240410152101.3211244-1-luca.vizzarro@arm.com>

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>
---
 lib/vhost/virtio_net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 1359c5fb1f..6a2ca295f5 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;
-- 
2.34.1


  reply	other threads:[~2024-04-10 15:22 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-10 15:21 [PATCH 0/1] fix GCC 13 build errors on 32-bit targets Luca Vizzarro
2024-04-10 15:21 ` Luca Vizzarro [this message]
2024-04-25 13:44   ` [PATCH 1/1] vhost: fix GCC 13 build error Maxime Coquelin
2024-06-12  8:29 ` [PATCH 0/1] fix GCC 13 build errors on 32-bit targets Maxime Coquelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240410152101.3211244-2-luca.vizzarro@arm.com \
    --to=luca.vizzarro@arm.com \
    --cc=dev@dpdk.org \
    --cc=luca.boccassi@gmail.com \
    --cc=nick.connolly@arm.com \
    --cc=paul.szczepanek@arm.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).