[PATCH 0/2] net/cnxk: harden telemetry parameter parsing

[PATCH 0/2] net/cnxk: harden telemetry parameter parsing

[Stephen Hemminger]

The cnxk telemetry handlers parse their command parameters with strtok(),
which keeps non-reentrant internal state and races when telemetry callbacks
run on per-connection threads. Both handlers also trust the parsed integers
more than they should: values are narrowed to the destination width or
aliased to a valid index before any range check, so an out-of-range port or
queue id can slip through and read past the backing array.

These two patches replace the strtok() walks with strtoul()/endptr parsing,
range-check each value before it is used, and drop the strdup()/free() that
the old SA-info path needed. The NIX handler additionally copies the full
parameter string rather than capping it at PCI_PRI_STR_SIZE + 1, which had
been truncating the queue id for longer device addresses.

Stephen Hemminger (2):
  net/cnxk: fix telemetry SA info parameter parsing
  common/cnxk: fix thread-unsafe NIX telemetry parsing

 drivers/common/cnxk/cnxk_telemetry_nix.c     | 80 ++++++++------------
 drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c | 50 ++++++------
 2 files changed, 54 insertions(+), 76 deletions(-)

-- 
2.53.0

[PATCH 1/2] net/cnxk: fix telemetry SA info parameter parsing

[Stephen Hemminger]

The /cnxk/ipsec/sa_info handler would silently wrap 32 bit value
to 16 bit port id.
An out-of-range port such as 65536 narrowed to a valid port
for the check and then read past the array.
Reject port ids >= RTE_MAX_ETHPORTS before the lookup.

The /cnxk/ipsec/info handler has similar issue with
strtoul().

Rework parse_params() to walk the string with strtoul()/endptr
rather than strtok(), which is not thread safe and races when the
telemetry callbacks run on per-connection threads. This drops the
strdup()/free(), range checks each value against UINT32_MAX, and
passes an unsigned char to isdigit().

Fixes: d74ed1628f7e ("net/cnxk: add SA info telemetry")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c | 50 ++++++++++----------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c b/drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c
index 86c2453c09..0c1533e3d7 100644
--- a/drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c
+++ b/drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c
@@ -211,33 +211,30 @@ copy_inb_sa_10k(struct rte_tel_data *d, uint32_t i, void *sa)
 static int
 parse_params(const char *params, uint32_t *vals, size_t n_vals)
 {
-	char dlim[2] = ",";
-	char *params_args;
 	size_t count = 0;
-	char *token;
 
-	if (vals == NULL || params == NULL || strlen(params) == 0)
+	if (params == NULL || !isdigit((unsigned char)params[0]))
 		return -1;
 
-	/* strtok expects char * and param is const char *. Hence on using
-	 * params as "const char *" compiler throws warning.
-	 */
-	params_args = strdup(params);
-	if (params_args == NULL)
-		return -1;
+	while (count < n_vals) {
+		char *end;
+		unsigned long v;
 
-	token = strtok(params_args, dlim);
-	while (token && isdigit(*token) && count < n_vals) {
-		vals[count++] = strtoul(token, NULL, 10);
-		token = strtok(NULL, dlim);
-	}
+		errno = 0;
+		v = strtoul(params, &end, 10);
+		if (errno != 0 || v > UINT32_MAX)
+			return -EINVAL;
+		vals[count++] = v;
 
-	free(params_args);
+		if (*end == '\0')
+			break;
 
-	if (count < n_vals)
-		return -1;
+		if (*end != ',' || !isdigit((unsigned char)end[1]))
+			return -EINVAL;
+		params = end + 1;
+	}
 
-	return 0;
+	return count == n_vals ? 0 : -EINVAL;
 }
 
 static int
@@ -252,13 +249,13 @@ ethdev_sec_tel_handle_sa_info(const char *cmd __rte_unused, const char *params,
 	uint32_t i;
 	int ret;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
-		return -EINVAL;
-
 	if (parse_params(params, vals, RTE_DIM(vals)) < 0)
 		return -EINVAL;
 
 	port_id = vals[0];
+	if (port_id >= RTE_MAX_ETHPORTS)
+		return -EINVAL;
+
 	sa_idx = vals[1];
 
 	if (!rte_eth_dev_is_valid_port(port_id)) {
@@ -320,12 +317,13 @@ ethdev_sec_tel_handle_info(const char *cmd __rte_unused, const char *params,
 	struct cnxk_eth_sec_sess *eth_sec, *tvar;
 	struct rte_eth_dev *eth_dev;
 	struct cnxk_eth_dev *dev;
-	uint16_t port_id;
+	unsigned long port_id;
 	char *end_p;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+	if (params == NULL || !isdigit((unsigned char)*params))
 		return -EINVAL;
 
+	errno = 0;
 	port_id = strtoul(params, &end_p, 0);
 	if (errno != 0)
 		return -EINVAL;
@@ -333,8 +331,8 @@ ethdev_sec_tel_handle_info(const char *cmd __rte_unused, const char *params,
 	if (*end_p != '\0')
 		plt_err("Extra parameters passed to telemetry, ignoring it");
 
-	if (!rte_eth_dev_is_valid_port(port_id)) {
-		plt_err("Invalid port id %u", port_id);
+	if (port_id >= RTE_MAX_ETHPORTS || !rte_eth_dev_is_valid_port(port_id)) {
+		plt_err("Invalid port id %lu", port_id);
 		return -EINVAL;
 	}
 
-- 
2.53.0

[PATCH 2/2] common/cnxk: fix thread-unsafe NIX telemetry parsing

[Stephen Hemminger]

cnxk_nix_tel_handle_info_x() backs the /cnxk/nix/{rq,cq,sq}/{info,ctx}
telemetry commands and parsed its "<pcidev>,<queue_id>" parameter with
strtok(), which keeps non-reentrant state and races when telemetry
callbacks run on per-connection threads.

Split the parameter with strchr() and parse the queue id with strtoul().
While here, copy the full parameter (the length was capped at
PCI_PRI_STR_SIZE + 1, truncating the id for longer device addresses) and
reject non-numeric or out-of-range ids instead of letting strtol() alias
them to queue 0.

Fixes: af75aac78978 ("common/cnxk: support telemetry for NIX")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/common/cnxk/cnxk_telemetry_nix.c | 80 +++++++++---------------
 1 file changed, 30 insertions(+), 50 deletions(-)

diff --git a/drivers/common/cnxk/cnxk_telemetry_nix.c b/drivers/common/cnxk/cnxk_telemetry_nix.c
index abeefafe1e..82a146c139 100644
--- a/drivers/common/cnxk/cnxk_telemetry_nix.c
+++ b/drivers/common/cnxk/cnxk_telemetry_nix.c
@@ -1015,76 +1015,56 @@ cnxk_nix_tel_handle_info_x(const char *cmd, const char *params,
 			   struct plt_tel_data *d)
 {
 	struct nix_tel_node *node;
-	char *name, *param;
 	char buf[1024];
+	char *comma, *end;
+	unsigned long qid;
 	int rc = -1;
 
-	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
-		goto exit;
+	if (params == NULL || !isdigit((unsigned char)params[0]))
+		return -1;
 
-	plt_strlcpy(buf, params, PCI_PRI_STR_SIZE + 1);
-	name = strtok(buf, ",");
-	if (name == NULL)
-		goto exit;
+	plt_strlcpy(buf, params, sizeof(buf));	/* was PCI_PRI_STR_SIZE + 1 */
 
-	param = strtok(NULL, "\0");
+	/* params is "<pcidev_name>,<queue_id>" */
+	comma = strchr(buf, ',');
+	if (comma == NULL || !isdigit((unsigned char)comma[1]))
+		return -1;
+	*comma = '\0';
 
-	node = nix_tel_node_get_by_pcidev_name(name);
-	if (!node)
-		goto exit;
+	errno = 0;
+	qid = strtoul(comma + 1, &end, 10);
+	if (errno != 0 || (*end != '\0' && *end != ','))
+		return -1;
+
+	node = nix_tel_node_get_by_pcidev_name(buf);
+	if (node == NULL)
+		return -1;
 
 	plt_tel_data_start_dict(d);
 
 	if (strstr(cmd, "rq")) {
-		char *tok = strtok(param, ",");
-		int rq;
-
-		if (!tok)
-			goto exit;
-
-		rq = strtol(tok, NULL, 10);
-		if ((node->n_rq <= rq) || (rq < 0))
-			goto exit;
-
+		if (qid >= node->n_rq)
+			return -1;
 		if (strstr(cmd, "ctx"))
-			rc = cnxk_tel_nix_rq_ctx(node->nix, rq, d);
+			rc = cnxk_tel_nix_rq_ctx(node->nix, qid, d);
 		else
-			rc = cnxk_tel_nix_rq(node->rqs[rq], d);
-
+			rc = cnxk_tel_nix_rq(node->rqs[qid], d);
 	} else if (strstr(cmd, "cq")) {
-		char *tok = strtok(param, ",");
-		int cq;
-
-		if (!tok)
-			goto exit;
-
-		cq = strtol(tok, NULL, 10);
-		if ((node->n_cq <= cq) || (cq < 0))
-			goto exit;
-
+		if (qid >= node->n_cq)
+			return -1;
 		if (strstr(cmd, "ctx"))
-			rc = cnxk_tel_nix_cq_ctx(node->nix, cq, d);
+			rc = cnxk_tel_nix_cq_ctx(node->nix, qid, d);
 		else
-			rc = cnxk_tel_nix_cq(node->cqs[cq], d);
-
+			rc = cnxk_tel_nix_cq(node->cqs[qid], d);
 	} else if (strstr(cmd, "sq")) {
-		char *tok = strtok(param, ",");
-		int sq;
-
-		if (!tok)
-			goto exit;
-
-		sq = strtol(tok, NULL, 10);
-		if ((node->n_sq <= sq) || (sq < 0))
-			goto exit;
-
+		if (qid >= node->n_sq)
+			return -1;
 		if (strstr(cmd, "ctx"))
-			rc = cnxk_tel_nix_sq_ctx(node->nix, sq, d);
+			rc = cnxk_tel_nix_sq_ctx(node->nix, qid, d);
 		else
-			rc = cnxk_tel_nix_sq(node->sqs[sq], d);
+			rc = cnxk_tel_nix_sq(node->sqs[qid], d);
 	}
 
-exit:
 	return rc;
 }
 
-- 
2.53.0

Re: [PATCH 2/2] common/cnxk: fix thread-unsafe NIX telemetry parsing

[Jerin Jacob]

On Sat, Jun 6, 2026 at 4:15 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> cnxk_nix_tel_handle_info_x() backs the /cnxk/nix/{rq,cq,sq}/{info,ctx}
> telemetry commands and parsed its "<pcidev>,<queue_id>" parameter with
> strtok(), which keeps non-reentrant state and races when telemetry
> callbacks run on per-connection threads.
>
> Split the parameter with strchr() and parse the queue id with strtoul().
> While here, copy the full parameter (the length was capped at
> PCI_PRI_STR_SIZE + 1, truncating the id for longer device addresses) and
> reject non-numeric or out-of-range ids instead of letting strtol() alias
> them to queue 0.
>
> Fixes: af75aac78978 ("common/cnxk: support telemetry for NIX")
> Cc: stable@dpdk.org
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

@Nithin Dabilpuram  Please review and Ack the series.