On Fri, 5 Jun 2026 13:50:57 -0700 Stephen Hemminger <[email protected]> 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.
