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

[Stephen Hemminger <stephen@networkplumber.org>]

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.