All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: dev@dpdk.org
Cc: Stephen Hemminger <stephen@networkplumber.org>,
	stable@dpdk.org, Akhil Goyal <gakhil@marvell.com>,
	Anoob Joseph <anoobj@marvell.com>,
	Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
Subject: [PATCH 4/8] security: harden telemetry parameter parsing
Date: Fri,  5 Jun 2026 13:51:01 -0700	[thread overview]
Message-ID: <20260605205253.520196-5-stephen@networkplumber.org> (raw)
In-Reply-To: <20260605205253.520196-1-stephen@networkplumber.org>

The cryptodev security telemetry handlers parsed dev_id/capa_id with
strtoul() and no overflow or range check, so an out-of-range dev_id
(e.g. 256) silently truncated to a valid device in
rte_cryptodev_is_valid_dev(). isdigit() was also called on a plain
(signed) char, which is undefined for high-bit input.
The parser was also using strtok() which is not thread safe.

Use a validated parse helper and reject malformed input rather than
logging and continuing. This also drops the thread-unsafe strtok() in
the crypto_caps handler.

Fixes: 259ca6d1617f ("security: add telemetry endpoint for capabilities")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/security/rte_security.c | 41 ++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c
index c47fe44da0..0d89f8af3f 100644
--- a/lib/security/rte_security.c
+++ b/lib/security/rte_security.c
@@ -7,6 +7,8 @@
 #include <stdalign.h>
 #include <ctype.h>
 #include <stdlib.h>
+#include <errno.h>
+#include <limits.h>
 
 #include <eal_export.h>
 #include <rte_cryptodev.h>
@@ -474,6 +476,25 @@ security_capabilities_from_dev_id(int dev_id, const void **caps)
 	return 0;
 }
 
+/* Parse an unsigned integer parameter, returning the value or -EINVAL.
+ * 'max' must be <= INT_MAX.
+ */
+static int
+telemetry_parse_uint(const char *str, char **end, unsigned long max)
+{
+	unsigned long val;
+
+	if (str == NULL || !isdigit((unsigned char)*str))
+		return -EINVAL;
+
+	errno = 0;
+	val = strtoul(str, end, 0);
+	if (errno != 0 || val > max)
+		return -EINVAL;
+
+	return (int)val;
+}
+
 static int
 security_handle_cryptodev_sec_caps(const char *cmd __rte_unused, const char *params,
 				   struct rte_tel_data *d)
@@ -485,13 +506,10 @@ security_handle_cryptodev_sec_caps(const char *cmd __rte_unused, const char *par
 	int dev_id;
 	int rc;
 
-	if (!params || strlen(params) == 0 || !isdigit(*params))
+	dev_id = telemetry_parse_uint(params, &end_param, RTE_CRYPTO_MAX_DEVS - 1);
+	if (dev_id < 0 || *end_param != '\0')
 		return -EINVAL;
 
-	dev_id = strtoul(params, &end_param, 0);
-	if (*end_param != '\0')
-		CDEV_LOG_ERR("Extra parameters passed to command, ignoring");
-
 	rc = security_capabilities_from_dev_id(dev_id, (void *)&capabilities);
 	if (rc < 0)
 		return rc;
@@ -513,24 +531,19 @@ security_handle_cryptodev_crypto_caps(const char *cmd __rte_unused, const char *
 {
 	const struct rte_security_capability *capabilities;
 	struct rte_tel_data *crypto_caps;
-	const char *capa_param;
 	int dev_id, capa_id;
 	int crypto_caps_n;
 	char *end_param;
 	int rc;
 
-	if (!params || strlen(params) == 0 || !isdigit(*params))
+	dev_id = telemetry_parse_uint(params, &end_param, RTE_CRYPTO_MAX_DEVS - 1);
+	if (dev_id < 0 || *end_param != ',')
 		return -EINVAL;
 
-	dev_id = strtoul(params, &end_param, 0);
-	capa_param = strtok(end_param, ",");
-	if (!capa_param || strlen(capa_param) == 0 || !isdigit(*capa_param))
+	capa_id = telemetry_parse_uint(end_param + 1, &end_param, INT_MAX);
+	if (capa_id < 0 || *end_param != '\0')
 		return -EINVAL;
 
-	capa_id = strtoul(capa_param, &end_param, 0);
-	if (*end_param != '\0')
-		CDEV_LOG_ERR("Extra parameters passed to command, ignoring");
-
 	rc = security_capabilities_from_dev_id(dev_id, (void *)&capabilities);
 	if (rc < 0)
 		return rc;
-- 
2.53.0


  parent reply	other threads:[~2026-06-05 20:53 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-05 20:50 [PATCH 0/8] telemetry: thread-safe and bounded parameter parsing Stephen Hemminger
2026-06-05 20:50 ` [PATCH 1/8] telemetry: fix thread-unsafe command parsing Stephen Hemminger
2026-06-08  1:25   ` fengchengwen
2026-06-08  7:49   ` Bruce Richardson
2026-06-05 20:50 ` [PATCH 2/8] ethdev: make telemetry parameter parsing thread-safe Stephen Hemminger
2026-06-08  1:26   ` fengchengwen
2026-06-05 20:51 ` [PATCH 3/8] dmadev: validate telemetry parameters Stephen Hemminger
2026-06-08  1:20   ` fengchengwen
2026-06-05 20:51 ` Stephen Hemminger [this message]
2026-06-05 20:51 ` [PATCH 5/8] eventdev: remove strtok from telemetry handlers Stephen Hemminger
2026-06-05 20:51 ` [PATCH 6/8] eventdev/eth_rx: fix thread-unsafe telemetry parsing Stephen Hemminger
2026-06-05 20:51 ` [PATCH 7/8] eventdev/eth_rx: reject out-of-range telemetry adapter ID Stephen Hemminger
2026-06-05 20:51 ` [PATCH 8/8] eventdev/timer: reject out-of-range ID Stephen Hemminger
2026-06-06  6:08 ` [PATCH 0/8] telemetry: thread-safe and bounded parameter parsing Stephen Hemminger
2026-06-08  7:55 ` Bruce Richardson

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=20260605205253.520196-5-stephen@networkplumber.org \
    --to=stephen@networkplumber.org \
    --cc=anoobj@marvell.com \
    --cc=dev@dpdk.org \
    --cc=gakhil@marvell.com \
    --cc=gmuthukrishn@marvell.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.