|SUCCESS| net/af_packet: fix qpairs argument upper bound check

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/164869

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-03

DPDK Patch Review

Commit: [hash not available from patch]
Author: Denis Sergeev <denserg.edu at gmail.com>
Subject: net/af_packet: fix qpairs argument upper bound check

This patch addresses an integer overflow vulnerability in the af_packet PMD
where negative qpairs values wrap to UINT_MAX and bypass validation checks.

> diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
> index 0ee94e71ea..ebf015dd9a 100644
> --- a/drivers/net/af_packet/rte_eth_af_packet.c
> +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> @@ -1169,7 +1169,7 @@ rte_eth_from_packet(struct rte_vdev_device *dev,
>  		pair = &kvlist->pairs[k_idx];
>  		if (strstr(pair->key, ETH_AF_PACKET_NUM_Q_ARG) != NULL) {
>  			qpairs = atoi(pair->value);
> -			if (qpairs < 1) {
> +			if (qpairs < 1 || qpairs > RTE_MAX_QUEUES_PER_PORT) {

Does this check actually prevent the overflow from atoi()? When atoi()
parses "-1", it returns -1 as a signed integer. This gets implicitly
converted to unsigned int where it wraps to UINT_MAX (4294967295). The
comparison "qpairs < 1" evaluates as "4294967295 < 1" which is false, so
the overflow bypasses detection. However, UINT_MAX is still greater than
RTE_MAX_QUEUES_PER_PORT, so does the new check now catch this case?

Does RTE_MAX_QUEUES_PER_PORT have a defined value that is safely less than
UINT_MAX? If RTE_MAX_QUEUES_PER_PORT is defined as UINT16_MAX (65535),
then does this fix work correctly for the overflow case?

Should the code use strtol() with proper error checking instead of atoi()
to detect negative values before the unsigned conversion happens? The
atoi()->unsigned int pattern appears elsewhere in this function for
blocksize, blockcnt, and framecnt. Do those parameters have the same
vulnerability?

Consider this alternative approach in rte_eth_from_packet():

    long qpairs_long = strtol(pair->value, &endptr, 10);
    if (qpairs_long < 1 || qpairs_long > RTE_MAX_QUEUES_PER_PORT) {
        /* error */
    }
    qpairs = (unsigned int)qpairs_long;

>  				PMD_LOG(ERR,
>  					"%s: invalid qpairs value",
>  				        name);

The error message could be more descriptive. Should it indicate the valid
range for qpairs to help users understand what values are acceptable?