[PATCH 0/8] telemetry: thread-safe and bounded parameter parsing

[PATCH 0/8] telemetry: thread-safe and bounded parameter parsing

[Stephen Hemminger]

While looking into extending telemetry for other uses, I noticed a
pattern of unsafe string handling in the command handlers. They run one
thread per client connection but parse parameters with non-reentrant
strtok(), and convert ids with atoi()/unchecked strtoul() that silently
truncate or alias out-of-range values; in eth_rx the strtok()
continuation chain can also dereference freed memory.

This series covers the library code (telemetry, ethdev, dmadev, security,
eventdev, eth_rx, timer). A follow-up is needed for the same strtok()
use in drivers.

They are marked for stable: the races and the use-after-free are real and
the changes are low-risk to backport. But severity is low since telemetry is
not a remote interface, but these are the kind of issues likely to
be found by AI security scanning tools.

In future, atoi() and strtok() look worth adding to the forbidden
tokens list in devtools/checkpatches.sh.

Stephen Hemminger (8):
  telemetry: fix thread-unsafe command parsing
  ethdev: make telemetry parameter parsing thread-safe
  dmadev: validate telemetry parameters
  security: harden telemetry parameter parsing
  eventdev: remove strtok from telemetry handlers
  eventdev/eth_rx: fix thread-unsafe telemetry parsing
  eventdev/eth_rx: reject out-of-range telemetry adapter ID
  eventdev/timer: reject out-of-range ID

 lib/dmadev/rte_dmadev.c                 |  44 +++++---
 lib/ethdev/rte_ethdev_telemetry.c       |  12 ++-
 lib/eventdev/rte_event_eth_rx_adapter.c |  97 ++++++++---------
 lib/eventdev/rte_event_timer_adapter.c  |  22 ++--
 lib/eventdev/rte_eventdev.c             | 136 +++++++++++-------------
 lib/security/rte_security.c             |  41 ++++---
 lib/telemetry/telemetry.c               |   5 +-
 7 files changed, 186 insertions(+), 171 deletions(-)

-- 
2.53.0

[PATCH 1/8] telemetry: fix thread-unsafe command parsing

[Stephen Hemminger]

The telemetry client_handler() runs in a detached thread per connection,
and up to MAX_CONNECTIONS instances can run concurrently.
The function strtok() keeps parser state in a static variable
shared across all threads, so concurrent clients corrupt each other's
command parsing. Use strtok_r() with a local saveptr.

Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/telemetry/telemetry.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index b109d076d4..e591c1e283 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -415,8 +415,9 @@ client_handler(void *sock_id)
 	int bytes = read(s, buffer, sizeof(buffer) - 1);
 	while (bytes > 0) {
 		buffer[bytes] = 0;
-		const char *cmd = strtok(buffer, ",");
-		const char *param = strtok(NULL, "\0");
+		char *saveptr = NULL;
+		const char *cmd = strtok_r(buffer, ",", &saveptr);
+		const char *param = strtok_r(NULL, "\0", &saveptr);
 		struct cmd_callback cb = {.fn = unknown_command};
 		int i;
 
-- 
2.53.0

[PATCH 2/8] ethdev: make telemetry parameter parsing thread-safe

[Stephen Hemminger]

The ethdev telemetry handlers run in a per-connection thread.
Two of the parameter parsers used strtok(), which keeps its state in a
process-global static shared across threads.
Use strtok_r() with a local save pointer.

Also pass an unsigned char to isdigit(),
which is undefined for characters with high-bit set.

Fixes: 9e7533aeb80a ("ethdev: add telemetry command for TM level capabilities")
Fixes: f38f62650f7b ("ethdev: add Rx queue telemetry query")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/ethdev/rte_ethdev_telemetry.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/ethdev/rte_ethdev_telemetry.c b/lib/ethdev/rte_ethdev_telemetry.c
index a910864bc5..ca7f4681c9 100644
--- a/lib/ethdev/rte_ethdev_telemetry.c
+++ b/lib/ethdev/rte_ethdev_telemetry.c
@@ -32,7 +32,7 @@ eth_dev_parse_port_params(const char *params, uint16_t *port_id,
 	uint64_t pi;
 
 	if (params == NULL || strlen(params) == 0 ||
-		!isdigit(*params) || port_id == NULL)
+		!isdigit((unsigned char)*params) || port_id == NULL)
 		return -EINVAL;
 
 	pi = strtoul(params, end_param, 0);
@@ -459,6 +459,7 @@ ethdev_parse_queue_params(const char *params, bool is_rx,
 	const char *qid_param;
 	uint16_t nb_queues;
 	char *end_param;
+	char *saveptr = NULL;
 	uint64_t qid;
 	int ret;
 
@@ -471,8 +472,8 @@ ethdev_parse_queue_params(const char *params, bool is_rx,
 	if (nb_queues == 1 && *end_param == '\0')
 		qid = 0;
 	else {
-		qid_param = strtok(end_param, ",");
-		if (!qid_param || strlen(qid_param) == 0 || !isdigit(*qid_param))
+		qid_param = strtok_r(end_param, ",", &saveptr);
+		if (!qid_param || strlen(qid_param) == 0 || !isdigit((unsigned char)*qid_param))
 			return -EINVAL;
 
 		qid = strtoul(qid_param, &end_param, 0);
@@ -1207,10 +1208,11 @@ static int
 eth_dev_parse_tm_params(char *params, uint32_t *result)
 {
 	const char *splited_param;
+	char *saveptr = NULL;
 	uint64_t ret;
 
-	splited_param = strtok(params, ",");
-	if (!splited_param || strlen(splited_param) == 0 || !isdigit(*splited_param))
+	splited_param = strtok_r(params, ",", &saveptr);
+	if (!splited_param || strlen(splited_param) == 0 || !isdigit((unsigned char)*splited_param))
 		return -EINVAL;
 
 	ret = strtoul(splited_param, &params, 0);
-- 
2.53.0

[PATCH 3/8] dmadev: validate telemetry parameters

[Stephen Hemminger]

Tighten parsing of the dmadev telemetry device and vchan parameters:
reject non-numeric and out-of-range ids through a bounded helper rather
than narrowing strtoul()'s result to int and leaning on the downstream
int16_t/uint16_t API to revalidate. This also drops the thread-unsafe
strtok() in the stats handler.

Fixes: 39b5ab60df30 ("dmadev: add telemetry")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/dmadev/rte_dmadev.c | 44 ++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
index b75b4f9bd1..822bb7c89f 100644
--- a/lib/dmadev/rte_dmadev.c
+++ b/lib/dmadev/rte_dmadev.c
@@ -4,6 +4,7 @@
  */
 
 #include <ctype.h>
+#include <errno.h>
 #include <inttypes.h>
 #include <stdlib.h>
 
@@ -1157,6 +1158,25 @@ dmadev_handle_dev_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+/* Parse an unsigned integer telemetry parameter, returning the value or
+ * -EINVAL.  'max' must be <= INT_MAX.
+ */
+static int
+dmadev_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;
+}
+
 #define ADD_CAPA(td, dc, c) rte_tel_data_add_dict_int(td, dma_capability_name(c), !!(dc & c))
 
 static int
@@ -1169,10 +1189,9 @@ dmadev_handle_dev_info(const char *cmd __rte_unused,
 	uint64_t dev_capa;
 	char *end_param;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	dev_id = dmadev_parse_uint(params, &end_param, INT16_MAX);
+	if (dev_id < 0)
 		return -EINVAL;
-
-	dev_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
 
@@ -1227,13 +1246,11 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
 	struct rte_dma_stats dma_stats;
 	int dev_id, ret, vchan_id;
 	char *end_param;
-	const char *vchan_param;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	dev_id = dmadev_parse_uint(params, &end_param, INT16_MAX);
+	if (dev_id < 0)
 		return -EINVAL;
 
-	dev_id = strtoul(params, &end_param, 0);
-
 	/* Function info_get validates dev_id so we don't need to. */
 	ret = rte_dma_info_get(dev_id, &dma_info);
 	if (ret < 0)
@@ -1245,11 +1262,11 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
 	if (dma_info.nb_vchans == 1 && *end_param == '\0')
 		vchan_id = 0;
 	else {
-		vchan_param = strtok(end_param, ",");
-		if (!vchan_param || strlen(vchan_param) == 0 || !isdigit(*vchan_param))
+		if (*end_param != ',')
+			return -EINVAL;
+		vchan_id = dmadev_parse_uint(end_param + 1, &end_param, UINT16_MAX);
+		if (vchan_id < 0)
 			return -EINVAL;
-
-		vchan_id = strtoul(vchan_param, &end_param, 0);
 	}
 	if (*end_param != '\0')
 		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
@@ -1276,10 +1293,9 @@ dmadev_handle_dev_dump(const char *cmd __rte_unused,
 	int dev_id, ret;
 	FILE *f;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	dev_id = dmadev_parse_uint(params, &end_param, INT16_MAX);
+	if (dev_id < 0)
 		return -EINVAL;
-
-	dev_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
 
-- 
2.53.0

[PATCH 4/8] security: harden telemetry parameter parsing

[Stephen Hemminger]

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

[PATCH 5/8] eventdev: remove strtok from telemetry handlers

[Stephen Hemminger]

The eventdev telemetry command handlers parsed parameters with strtok()
which is not thread safe. The code also has silent truncation
issues since it assigns result of strtoul() to smaller types.

Introduce common helper that checks parameter string
and gets a device id. Make sure and range check values
to the appropriate upper bound for that parameter.

Fixes: feaea0573429 ("eventdev: add device info telemetry command")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eventdev/rte_eventdev.c | 136 +++++++++++++++++-------------------
 1 file changed, 64 insertions(+), 72 deletions(-)

diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 572cd5bd7d..a6f6b6879e 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -1790,6 +1790,34 @@ handle_dev_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+/* Get dev ID from parameter string */
+static int
+parse_dev_id(const char *params, uint8_t *dev_id, char **next_param)
+{
+	char *end_param;
+	unsigned long id;
+
+	if (params == NULL || *params == '\0' ||
+	    !isdigit((unsigned char)*params))
+		return -1;
+
+	id = strtoul(params, &end_param, 10);
+	if (next_param != NULL) {
+		*next_param = end_param;
+	} else if (*end_param != '\0') {
+		RTE_EDEV_LOG_DEBUG(
+			"Extra parameters passed to eventdev telemetry command, ignoring");
+	}
+
+	if (id >= RTE_EVENT_MAX_DEVS) {
+		RTE_EDEV_LOG_DEBUG(
+			"Invalid device id out of range %lu", id);
+		return -1;
+	}
+	*dev_id = id;
+	return 0;
+}
+
 static int
 handle_dev_info(const char *cmd __rte_unused,
 		const char *params,
@@ -1797,16 +1825,10 @@ handle_dev_info(const char *cmd __rte_unused,
 {
 	uint8_t dev_id;
 	struct rte_eventdev *dev;
-	char *end_param;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (parse_dev_id(params, &dev_id, NULL) < 0)
 		return -1;
 
-	dev_id = strtoul(params, &end_param, 10);
-	if (*end_param != '\0')
-		RTE_EDEV_LOG_DEBUG(
-			"Extra parameters passed to eventdev telemetry command, ignoring");
-
 	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
 	dev = &rte_eventdevs[dev_id];
 
@@ -1833,16 +1855,10 @@ handle_port_list(const char *cmd __rte_unused,
 	int i;
 	uint8_t dev_id;
 	struct rte_eventdev *dev;
-	char *end_param;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (parse_dev_id(params, &dev_id, NULL) < 0)
 		return -1;
 
-	dev_id = strtoul(params, &end_param, 10);
-	if (*end_param != '\0')
-		RTE_EDEV_LOG_DEBUG(
-			"Extra parameters passed to eventdev telemetry command, ignoring");
-
 	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
 	dev = &rte_eventdevs[dev_id];
 
@@ -1861,16 +1877,10 @@ handle_queue_list(const char *cmd __rte_unused,
 	int i;
 	uint8_t dev_id;
 	struct rte_eventdev *dev;
-	char *end_param;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (parse_dev_id(params, &dev_id, NULL) != 0)
 		return -1;
 
-	dev_id = strtoul(params, &end_param, 10);
-	if (*end_param != '\0')
-		RTE_EDEV_LOG_DEBUG(
-			"Extra parameters passed to eventdev telemetry command, ignoring");
-
 	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
 	dev = &rte_eventdevs[dev_id];
 
@@ -1886,27 +1896,28 @@ handle_queue_links(const char *cmd __rte_unused,
 		   const char *params,
 		   struct rte_tel_data *d)
 {
-	int i, ret, port_id = 0;
+	int i, ret;
 	char *end_param;
 	uint8_t dev_id;
 	uint8_t queues[RTE_EVENT_MAX_QUEUES_PER_DEV];
 	uint8_t priorities[RTE_EVENT_MAX_QUEUES_PER_DEV];
-	const char *p_param;
+	unsigned long port_id;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (parse_dev_id(params, &dev_id, &end_param) != 0)
 		return -1;
 
-	/* Get dev ID from parameter string */
-	dev_id = strtoul(params, &end_param, 10);
 	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
 
-	p_param = strtok(end_param, ",");
-	if (p_param == NULL || strlen(p_param) == 0 || !isdigit(*p_param))
+	if (*end_param != ',' || !isdigit((unsigned char)end_param[1]))
 		return -1;
 
-	port_id = strtoul(p_param, &end_param, 10);
-	p_param = strtok(NULL, "\0");
-	if (p_param != NULL)
+	port_id = strtoul(end_param + 1, &end_param, 10);
+	if (port_id >= RTE_EVENT_MAX_PORTS_PER_DEV) {
+		RTE_EDEV_LOG_DEBUG("Invalid port id out of range %lu", port_id);
+		return -1;
+	}
+
+	if (*end_param != '\0')
 		RTE_EDEV_LOG_DEBUG(
 			"Extra parameters passed to eventdev telemetry command, ignoring");
 
@@ -1998,19 +2009,12 @@ handle_dev_xstats(const char *cmd __rte_unused,
 		  const char *params,
 		  struct rte_tel_data *d)
 {
-	int dev_id;
+	uint8_t dev_id;
 	enum rte_event_dev_xstats_mode mode;
-	char *end_param;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (parse_dev_id(params, &dev_id, NULL) != 0)
 		return -1;
 
-	/* Get dev ID from parameter string */
-	dev_id = strtoul(params, &end_param, 10);
-	if (*end_param != '\0')
-		RTE_EDEV_LOG_DEBUG(
-			"Extra parameters passed to eventdev telemetry command, ignoring");
-
 	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
 
 	mode = RTE_EVENT_DEV_XSTATS_DEVICE;
@@ -2022,29 +2026,25 @@ handle_port_xstats(const char *cmd __rte_unused,
 		   const char *params,
 		   struct rte_tel_data *d)
 {
-	int dev_id;
-	int port_queue_id = 0;
+	uint8_t dev_id;
 	enum rte_event_dev_xstats_mode mode;
+	unsigned long port_queue_id;
 	char *end_param;
-	const char *p_param;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (parse_dev_id(params, &dev_id, &end_param) != 0)
 		return -1;
-
-	/* Get dev ID from parameter string */
-	dev_id = strtoul(params, &end_param, 10);
 	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
 
-	p_param = strtok(end_param, ",");
 	mode = RTE_EVENT_DEV_XSTATS_PORT;
 
-	if (p_param == NULL || strlen(p_param) == 0 || !isdigit(*p_param))
+	if (*end_param != ',' || !isdigit((unsigned char)end_param[1]))
 		return -1;
 
-	port_queue_id = strtoul(p_param, &end_param, 10);
+	port_queue_id = strtoul(end_param + 1, &end_param, 10);
+	if (port_queue_id >= RTE_EVENT_MAX_PORTS_PER_DEV)
+		return -1;
 
-	p_param = strtok(NULL, "\0");
-	if (p_param != NULL)
+	if (*end_param != '\0')
 		RTE_EDEV_LOG_DEBUG(
 			"Extra parameters passed to eventdev telemetry command, ignoring");
 
@@ -2056,29 +2056,26 @@ handle_queue_xstats(const char *cmd __rte_unused,
 		    const char *params,
 		    struct rte_tel_data *d)
 {
-	int dev_id;
-	int port_queue_id = 0;
+	uint8_t dev_id;
+	unsigned long port_queue_id;
 	enum rte_event_dev_xstats_mode mode;
 	char *end_param;
-	const char *p_param;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (parse_dev_id(params, &dev_id, &end_param) != 0)
 		return -1;
 
-	/* Get dev ID from parameter string */
-	dev_id = strtoul(params, &end_param, 10);
 	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
 
-	p_param = strtok(end_param, ",");
 	mode = RTE_EVENT_DEV_XSTATS_QUEUE;
 
-	if (p_param == NULL || strlen(p_param) == 0 || !isdigit(*p_param))
+	if (*end_param != ',' || !isdigit((unsigned char)end_param[1]))
 		return -1;
 
-	port_queue_id = strtoul(p_param, &end_param, 10);
+	port_queue_id = strtoul(end_param + 1, &end_param, 10);
+	if (port_queue_id >= RTE_EVENT_MAX_QUEUES_PER_DEV)
+		return -1;
 
-	p_param = strtok(NULL, "\0");
-	if (p_param != NULL)
+	if (*end_param != '\0')
 		RTE_EDEV_LOG_DEBUG(
 			"Extra parameters passed to eventdev telemetry command, ignoring");
 
@@ -2090,19 +2087,14 @@ handle_dev_dump(const char *cmd __rte_unused,
 		const char *params,
 		struct rte_tel_data *d)
 {
-	char *buf, *end_param;
-	int dev_id, ret;
+	char *buf;
+	uint8_t dev_id;
+	int ret;
 	FILE *f;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (parse_dev_id(params, &dev_id, NULL) != 0)
 		return -1;
 
-	/* Get dev ID from parameter string */
-	dev_id = strtoul(params, &end_param, 10);
-	if (*end_param != '\0')
-		RTE_EDEV_LOG_DEBUG(
-			"Extra parameters passed to eventdev telemetry command, ignoring");
-
 	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
 
 	buf = calloc(RTE_TEL_MAX_SINGLE_STRING_LEN, sizeof(char));
-- 
2.53.0

[PATCH 6/8] eventdev/eth_rx: fix thread-unsafe telemetry parsing

[Stephen Hemminger]

The eth Rx adapter telemetry handlers parse multi-field parameter
strings with a strtok()/strtok(NULL, ...) continuation chain.
Since strtok() holds its parser state in a process-global static,
and telemetry callbacks run in a separate thread per client connection.
With concurrent clients the continuation calls read another thread's
state - and since each handler strdup()s and frees its own buffer,
a stale continuation can dereference freed memory.
Thread the parse through a local save pointer with strtok_r().

Also passing was passing a char to isdigit(), which is undefined
in C standard for high-bit input.

Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eventdev/rte_event_eth_rx_adapter.c | 52 ++++++++++++-------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index 2183adce6f..96a4a0d926 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -308,7 +308,7 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
 } while (0)
 
 #define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
-	if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
+	if ((token) == NULL || strlen(token) == 0 || !isdigit((unsigned char)*token)) { \
 		RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token"); \
 		ret = retval; \
 		goto error; \
@@ -3764,7 +3764,7 @@ handle_rxa_stats(const char *cmd __rte_unused,
 	uint8_t rx_adapter_id;
 	struct rte_event_eth_rx_adapter_stats rx_adptr_stats;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
 		return -1;
 
 	/* Get Rx adapter ID from parameter string */
@@ -3804,7 +3804,7 @@ handle_rxa_stats_reset(const char *cmd __rte_unused,
 {
 	uint8_t rx_adapter_id;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
 		return -1;
 
 	/* Get Rx adapter ID from parameter string */
@@ -3829,29 +3829,29 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
 	uint16_t rx_queue_id;
 	uint16_t eth_dev_id;
 	int ret = -1;
-	char *token, *l_params;
+	char *token, *l_params, *saveptr = NULL;
 	struct rte_event_eth_rx_adapter_queue_conf queue_conf;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
 		return -1;
 
 	/* Get Rx adapter ID from parameter string */
 	l_params = strdup(params);
 	if (l_params == NULL)
 		return -ENOMEM;
-	token = strtok(l_params, ",");
+	token = strtok_r(l_params, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 	rx_adapter_id = strtoul(token, NULL, 10);
 	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
 
-	token = strtok(NULL, ",");
+	token = strtok_r(NULL, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 
 	/* Get device ID from parameter string */
 	eth_dev_id = strtoul(token, NULL, 10);
 	RTE_EVENT_ETH_RX_ADAPTER_PORTID_VALID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
 
-	token = strtok(NULL, ",");
+	token = strtok_r(NULL, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 
 	/* Get Rx queue ID from parameter string */
@@ -3862,7 +3862,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
 		goto error;
 	}
 
-	token = strtok(NULL, "\0");
+	token = strtok_r(NULL, "\0", &saveptr);
 	if (token != NULL)
 		RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
 				 " telemetry command, ignoring");
@@ -3902,29 +3902,29 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
 	uint16_t rx_queue_id;
 	uint16_t eth_dev_id;
 	int ret = -1;
-	char *token, *l_params;
+	char *token, *l_params, *saveptr = NULL;
 	struct rte_event_eth_rx_adapter_queue_stats q_stats;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
 		return -1;
 
 	/* Get Rx adapter ID from parameter string */
 	l_params = strdup(params);
 	if (l_params == NULL)
 		return -ENOMEM;
-	token = strtok(l_params, ",");
+	token = strtok_r(l_params, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 	rx_adapter_id = strtoul(token, NULL, 10);
 	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
 
-	token = strtok(NULL, ",");
+	token = strtok_r(NULL, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 
 	/* Get device ID from parameter string */
 	eth_dev_id = strtoul(token, NULL, 10);
 	RTE_EVENT_ETH_RX_ADAPTER_PORTID_VALID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
 
-	token = strtok(NULL, ",");
+	token = strtok_r(NULL, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 
 	/* Get Rx queue ID from parameter string */
@@ -3935,7 +3935,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
 		goto error;
 	}
 
-	token = strtok(NULL, "\0");
+	token = strtok_r(NULL, "\0", &saveptr);
 	if (token != NULL)
 		RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
 				 " telemetry command, ignoring");
@@ -3974,28 +3974,28 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
 	uint16_t rx_queue_id;
 	uint16_t eth_dev_id;
 	int ret = -1;
-	char *token, *l_params;
+	char *token, *l_params, *saveptr = NULL;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
 		return -1;
 
 	/* Get Rx adapter ID from parameter string */
 	l_params = strdup(params);
 	if (l_params == NULL)
 		return -ENOMEM;
-	token = strtok(l_params, ",");
+	token = strtok_r(l_params, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 	rx_adapter_id = strtoul(token, NULL, 10);
 	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
 
-	token = strtok(NULL, ",");
+	token = strtok_r(NULL, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 
 	/* Get device ID from parameter string */
 	eth_dev_id = strtoul(token, NULL, 10);
 	RTE_EVENT_ETH_RX_ADAPTER_PORTID_VALID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
 
-	token = strtok(NULL, ",");
+	token = strtok_r(NULL, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 
 	/* Get Rx queue ID from parameter string */
@@ -4006,7 +4006,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
 		goto error;
 	}
 
-	token = strtok(NULL, "\0");
+	token = strtok_r(NULL, "\0", &saveptr);
 	if (token != NULL)
 		RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
 				 " telemetry command, ignoring");
@@ -4036,22 +4036,22 @@ handle_rxa_instance_get(const char *cmd __rte_unused,
 	uint16_t rx_queue_id;
 	uint16_t eth_dev_id;
 	int ret = -1;
-	char *token, *l_params;
+	char *token, *l_params, *saveptr = NULL;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
 		return -1;
 
 	l_params = strdup(params);
 	if (l_params == NULL)
 		return -ENOMEM;
-	token = strtok(l_params, ",");
+	token = strtok_r(l_params, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 
 	/* Get device ID from parameter string */
 	eth_dev_id = strtoul(token, NULL, 10);
 	RTE_EVENT_ETH_RX_ADAPTER_PORTID_VALID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
 
-	token = strtok(NULL, ",");
+	token = strtok_r(NULL, ",", &saveptr);
 	RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
 
 	/* Get Rx queue ID from parameter string */
@@ -4062,7 +4062,7 @@ handle_rxa_instance_get(const char *cmd __rte_unused,
 		goto error;
 	}
 
-	token = strtok(NULL, "\0");
+	token = strtok_r(NULL, "\0", &saveptr);
 	if (token != NULL)
 		RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
 				 " telemetry command, ignoring");
-- 
2.53.0

[PATCH 7/8] eventdev/eth_rx: reject out-of-range telemetry adapter ID

[Stephen Hemminger]

The eventdev rx adapter code was using atoi() to parse numeric
parameters which does not handle out-of-range or extra garbage
on input. Tighten the code to only accept valid numbers.

Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eventdev/rte_event_eth_rx_adapter.c | 45 +++++++++++--------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index 96a4a0d926..635bd6014b 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -270,8 +270,8 @@ rxa_timestamp_dynfield(struct rte_mbuf *mbuf)
 		event_eth_rx_timestamp_dynfield_offset, rte_mbuf_timestamp_t *);
 }
 
-static inline int
-rxa_validate_id(uint8_t id)
+static inline bool
+rxa_validate_id(unsigned long id)
 {
 	return id < RTE_EVENT_ETH_RX_ADAPTER_MAX_INSTANCE;
 }
@@ -294,14 +294,14 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
 
 #define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, retval) do { \
 	if (!rxa_validate_id(id)) { \
-		RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d", id); \
+		RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %lu", (unsigned long)id); \
 		return retval; \
 	} \
 } while (0)
 
 #define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
 	if (!rxa_validate_id(id)) { \
-		RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d", id); \
+		RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %lu", (unsigned long)id); \
 		ret = retval; \
 		goto error; \
 	} \
@@ -316,8 +316,8 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
 } while (0)
 
 #define RTE_EVENT_ETH_RX_ADAPTER_PORTID_VALID_OR_GOTO_ERR_RET(port_id, retval) do { \
-	if (!rte_eth_dev_is_valid_port(port_id)) { \
-		RTE_EDEV_LOG_ERR("Invalid port_id=%u", port_id); \
+	if (port_id >= RTE_MAX_ETHPORTS || !rte_eth_dev_is_valid_port(port_id)) { \
+		RTE_EDEV_LOG_ERR("Invalid port_id=%lu", (unsigned long)port_id); \
 		ret = retval; \
 		goto error; \
 	} \
@@ -3761,14 +3761,14 @@ handle_rxa_stats(const char *cmd __rte_unused,
 		 const char *params,
 		 struct rte_tel_data *d)
 {
-	uint8_t rx_adapter_id;
+	unsigned long rx_adapter_id;
 	struct rte_event_eth_rx_adapter_stats rx_adptr_stats;
 
 	if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
 		return -1;
 
 	/* Get Rx adapter ID from parameter string */
-	rx_adapter_id = atoi(params);
+	rx_adapter_id = strtoul(params, NULL, 10);
 	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
 
 	/* Get Rx adapter stats */
@@ -3802,13 +3802,13 @@ handle_rxa_stats_reset(const char *cmd __rte_unused,
 		       const char *params,
 		       struct rte_tel_data *d __rte_unused)
 {
-	uint8_t rx_adapter_id;
+	unsigned long rx_adapter_id;
 
 	if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
 		return -1;
 
 	/* Get Rx adapter ID from parameter string */
-	rx_adapter_id = atoi(params);
+	rx_adapter_id = strtoul(params, NULL, 10);
 	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
 
 	/* Reset Rx adapter stats */
@@ -3825,9 +3825,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
 			  const char *params,
 			  struct rte_tel_data *d)
 {
-	uint8_t rx_adapter_id;
-	uint16_t rx_queue_id;
-	uint16_t eth_dev_id;
+	unsigned long rx_adapter_id, rx_queue_id, eth_dev_id;
 	int ret = -1;
 	char *token, *l_params, *saveptr = NULL;
 	struct rte_event_eth_rx_adapter_queue_conf queue_conf;
@@ -3857,7 +3855,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
 	/* Get Rx queue ID from parameter string */
 	rx_queue_id = strtoul(token, NULL, 10);
 	if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
-		RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+		RTE_EDEV_LOG_ERR("Invalid rx queue_id %lu", rx_queue_id);
 		ret = -EINVAL;
 		goto error;
 	}
@@ -3898,9 +3896,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
 			   const char *params,
 			   struct rte_tel_data *d)
 {
-	uint8_t rx_adapter_id;
-	uint16_t rx_queue_id;
-	uint16_t eth_dev_id;
+	unsigned long rx_adapter_id, rx_queue_id, eth_dev_id;
 	int ret = -1;
 	char *token, *l_params, *saveptr = NULL;
 	struct rte_event_eth_rx_adapter_queue_stats q_stats;
@@ -3930,7 +3926,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
 	/* Get Rx queue ID from parameter string */
 	rx_queue_id = strtoul(token, NULL, 10);
 	if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
-		RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+		RTE_EDEV_LOG_ERR("Invalid rx queue_id %lu", rx_queue_id);
 		ret = -EINVAL;
 		goto error;
 	}
@@ -3970,9 +3966,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
 			     const char *params,
 			     struct rte_tel_data *d __rte_unused)
 {
-	uint8_t rx_adapter_id;
-	uint16_t rx_queue_id;
-	uint16_t eth_dev_id;
+	unsigned long rx_adapter_id, rx_queue_id, eth_dev_id;
 	int ret = -1;
 	char *token, *l_params, *saveptr = NULL;
 
@@ -4001,7 +3995,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
 	/* Get Rx queue ID from parameter string */
 	rx_queue_id = strtoul(token, NULL, 10);
 	if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
-		RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+		RTE_EDEV_LOG_ERR("Invalid rx queue_id %lu", rx_queue_id);
 		ret = -EINVAL;
 		goto error;
 	}
@@ -4033,8 +4027,7 @@ handle_rxa_instance_get(const char *cmd __rte_unused,
 			struct rte_tel_data *d)
 {
 	uint8_t instance_id;
-	uint16_t rx_queue_id;
-	uint16_t eth_dev_id;
+	unsigned long rx_queue_id, eth_dev_id;
 	int ret = -1;
 	char *token, *l_params, *saveptr = NULL;
 
@@ -4057,7 +4050,7 @@ handle_rxa_instance_get(const char *cmd __rte_unused,
 	/* Get Rx queue ID from parameter string */
 	rx_queue_id = strtoul(token, NULL, 10);
 	if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
-		RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+		RTE_EDEV_LOG_ERR("Invalid rx queue_id %lu", rx_queue_id);
 		ret = -EINVAL;
 		goto error;
 	}
@@ -4074,7 +4067,7 @@ handle_rxa_instance_get(const char *cmd __rte_unused,
 						  rx_queue_id,
 						  &instance_id)) {
 		RTE_EDEV_LOG_ERR("Failed to get RX adapter instance ID "
-				 " for rx_queue_id = %d", rx_queue_id);
+				 " for rx_queue_id = %lu", rx_queue_id);
 		return -1;
 	}
 
-- 
2.53.0

[PATCH 8/8] eventdev/timer: reject out-of-range ID

[Stephen Hemminger]

The eventdev timer adapter code was using atoi() to parse numeric
parameters which does not handle out-of-range or extra garbage
on input. Tighten the code to only accept valid numbers.

Fixes: 791dfec24d00 ("eventdev/timer: add telemetry")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eventdev/rte_event_timer_adapter.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index af98b1d9f6..e3640a3bf8 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -1402,16 +1402,15 @@ handle_ta_info(const char *cmd __rte_unused, const char *params,
 {
 	struct rte_event_timer_adapter_info adapter_info;
 	struct rte_event_timer_adapter *adapter;
-	uint16_t adapter_id;
+	unsigned long adapter_id;
 	int ret;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
 		return -1;
 
-	adapter_id = atoi(params);
-
+	adapter_id = strtoul(params, NULL, 10);
 	if (adapters == NULL || adapter_id >= RTE_EVENT_TIMER_ADAPTER_NUM_MAX) {
-		EVTIM_LOG_ERR("Invalid timer adapter id %u", adapter_id);
+		EVTIM_LOG_ERR("Invalid timer adapter id %lu", adapter_id);
 		return -EINVAL;
 	}
 
@@ -1419,7 +1418,7 @@ handle_ta_info(const char *cmd __rte_unused, const char *params,
 
 	ret = rte_event_timer_adapter_get_info(adapter, &adapter_info);
 	if (ret < 0) {
-		EVTIM_LOG_ERR("Failed to get info for timer adapter id %u", adapter_id);
+		EVTIM_LOG_ERR("Failed to get info for timer adapter id %lu", adapter_id);
 		return ret;
 	}
 
@@ -1448,16 +1447,15 @@ handle_ta_stats(const char *cmd __rte_unused, const char *params,
 {
 	struct rte_event_timer_adapter_stats stats;
 	struct rte_event_timer_adapter *adapter;
-	uint16_t adapter_id;
+	unsigned long adapter_id;
 	int ret;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
 		return -1;
 
-	adapter_id = atoi(params);
-
+	adapter_id = strtoul(params, NULL, 10);
 	if (adapters == NULL || adapter_id >= RTE_EVENT_TIMER_ADAPTER_NUM_MAX) {
-		EVTIM_LOG_ERR("Invalid timer adapter id %u", adapter_id);
+		EVTIM_LOG_ERR("Invalid timer adapter id %lu", adapter_id);
 		return -EINVAL;
 	}
 
@@ -1465,7 +1463,7 @@ handle_ta_stats(const char *cmd __rte_unused, const char *params,
 
 	ret = rte_event_timer_adapter_stats_get(adapter, &stats);
 	if (ret < 0) {
-		EVTIM_LOG_ERR("Failed to get stats for timer adapter id %u", adapter_id);
+		EVTIM_LOG_ERR("Failed to get stats for timer adapter id %lu", adapter_id);
 		return ret;
 	}
 
-- 
2.53.0

Re: [PATCH 0/8] telemetry: thread-safe and bounded parameter parsing

[Stephen Hemminger]

On Fri,  5 Jun 2026 13:50:57 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> While looking into extending telemetry for other uses, I noticed a
> pattern of unsafe string handling in the command handlers. They run one
> thread per client connection but parse parameters with non-reentrant
> strtok(), and convert ids with atoi()/unchecked strtoul() that silently
> truncate or alias out-of-range values; in eth_rx the strtok()
> continuation chain can also dereference freed memory.
> 
> This series covers the library code (telemetry, ethdev, dmadev, security,
> eventdev, eth_rx, timer). A follow-up is needed for the same strtok()
> use in drivers.
> 
> They are marked for stable: the races and the use-after-free are real and
> the changes are low-risk to backport. But severity is low since telemetry is
> not a remote interface, but these are the kind of issues likely to
> be found by AI security scanning tools.
> 
> In future, atoi() and strtok() look worth adding to the forbidden
> tokens list in devtools/checkpatches.sh.
> 
> Stephen Hemminger (8):
>   telemetry: fix thread-unsafe command parsing
>   ethdev: make telemetry parameter parsing thread-safe
>   dmadev: validate telemetry parameters
>   security: harden telemetry parameter parsing
>   eventdev: remove strtok from telemetry handlers
>   eventdev/eth_rx: fix thread-unsafe telemetry parsing
>   eventdev/eth_rx: reject out-of-range telemetry adapter ID
>   eventdev/timer: reject out-of-range ID
> 
>  lib/dmadev/rte_dmadev.c                 |  44 +++++---
>  lib/ethdev/rte_ethdev_telemetry.c       |  12 ++-
>  lib/eventdev/rte_event_eth_rx_adapter.c |  97 ++++++++---------
>  lib/eventdev/rte_event_timer_adapter.c  |  22 ++--
>  lib/eventdev/rte_eventdev.c             | 136 +++++++++++-------------
>  lib/security/rte_security.c             |  41 ++++---
>  lib/telemetry/telemetry.c               |   5 +-
>  7 files changed, 186 insertions(+), 171 deletions(-)
> 

FYI - the CI AI review is all false positives on this.
Fed the review back into more capable AI that actually looks at the
code like a human and...

Thanks, but I believe all of these are false positives:

- rxa_validate_id() "%lu with uint8_t callers": the (unsigned long) cast is inside the macro, so the argument is unsigned long regardless of caller type. %lu is correct.

- parse_dev_id() / RTE_EVENT_MAX_DEVS: no such function or constant in this change; the bound is unchanged at RTE_EVENT_ETH_RX_ADAPTER_MAX_INSTANCE.

- cnxk qid upper-bound: the existing "qid >= node->n_rq" check already rejects any value above the (small) queue count before it is passed on, so qid > UINT16_MAX cannot reach the ctx function.

- capa_id bounds: security_capability_by_index() walks to the RTE_SECURITY_ACTION_TYPE_NONE sentinel and returns NULL for an out-of-range index, so there is no out-of-bounds access.

The max <= INT_MAX assertion in telemetry_parse_uint() is reasonable as documentation, but both callers pass values within range so it is not a correctness issue.