DPDK patches and discussions
 help / color / mirror / Atom feed
From: Niall Meade <niall.meade@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@amd.com>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	Roman Zhukov <roman.zhukov@arknetworks.am>
Cc: dev@dpdk.org, Niall Meade <niall.meade@intel.com>
Subject: [PATCH v2 1/1] ethdev: fix int overflow in descriptor count logic
Date: Fri, 27 Sep 2024 13:23:29 +0000	[thread overview]
Message-ID: <20240927132329.296038-1-niall.meade@intel.com> (raw)

Addressed a specific overflow issue in the eth_dev_adjust_nb_desc()
function where the uint16_t variable nb_desc would overflow when its
value was greater than (2^16 - nb_align). This overflow caused nb_desc
to incorrectly wrap around between 0 and nb_align-1, leading to the
function setting nb_desc to nb_min instead of the expected nb_max.

To give an example, let nb_desc=UINT16_MAX, nb_align=32, nb_max=4096 and
nb_min=64. RTE_ALIGN_CEIL(nb_desc, nb_align) calls
RTE_ALIGN_FLOOR(nb_desc + nb_align - 1, nb_align). This results in an
overflow of nb_desc, leading to nb_desc being set to 30 and then 0 when
the macros return. As a result of this, nb_desc is then set to nb_min
later on.

The resolution involves upcasting nb_desc to a uint32_t before the
RTE_ALIGN_CEIL macro is applied. This change ensures that the subsequent
call to RTE_ALIGN_FLOOR(nb_desc + (nb_align - 1), nb_align) does not
result in an overflow, as it would when nb_desc is a uint16_t. By using
a uint32_t for these operations, the correct behavior is maintained
without the risk of overflow.

Fixes: 0f67fc3baeb9 ("ethdev: add function to adjust number of descriptors")

Signed-off-by: Niall Meade <niall.meade@intel.com>

---
v2:
* add example of issue to commit message
---
 .mailmap                |  1 +
 lib/ethdev/rte_ethdev.c | 12 +++++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/.mailmap b/.mailmap
index 4a508bafad..c1941e78bb 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1053,6 +1053,7 @@ Nelson Escobar <neescoba@cisco.com>
 Nemanja Marjanovic <nemanja.marjanovic@intel.com>
 Netanel Belgazal <netanel@amazon.com>
 Netanel Gonen <netanelg@mellanox.com>
+Niall Meade <niall.meade@intel.com>
 Niall Power <niall.power@intel.com>
 Nicholas Pratte <npratte@iol.unh.edu>
 Nick Connolly <nick.connolly@arm.com> <nick.connolly@mayadata.io>
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f1c658f49e..f978283edf 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6577,13 +6577,19 @@ static void
 eth_dev_adjust_nb_desc(uint16_t *nb_desc,
 		const struct rte_eth_desc_lim *desc_lim)
 {
+	/* Upcast to uint32 to avoid potential overflow with RTE_ALIGN_CEIL(). */
+	uint32_t nb_desc_32 = *nb_desc;
+
 	if (desc_lim->nb_align != 0)
-		*nb_desc = RTE_ALIGN_CEIL(*nb_desc, desc_lim->nb_align);
+		nb_desc_32 = RTE_ALIGN_CEIL(nb_desc_32, desc_lim->nb_align);
 
 	if (desc_lim->nb_max != 0)
-		*nb_desc = RTE_MIN(*nb_desc, desc_lim->nb_max);
+		nb_desc_32 = RTE_MIN(nb_desc_32, desc_lim->nb_max);
+
+	nb_desc_32 = RTE_MAX(nb_desc_32, desc_lim->nb_min);
 
-	*nb_desc = RTE_MAX(*nb_desc, desc_lim->nb_min);
+	/* Assign clipped u32 back to u16. */
+	*nb_desc = nb_desc_32;
 }
 
 int
-- 
2.34.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.


             reply	other threads:[~2024-09-27 22:14 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-27 13:23 Niall Meade [this message]
2024-09-27 23:24 ` Ferruh Yigit

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=20240927132329.296038-1-niall.meade@intel.com \
    --to=niall.meade@intel.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=roman.zhukov@arknetworks.am \
    --cc=thomas@monjalon.net \
    /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).