Nthw: array overrun

https://bugs.dpdk.org/show_bug.cgi?id=1580

            Bug ID: 1580
           Summary: Nthw: array overrun
           Product: DPDK
           Version: 24.11
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: Normal
         Component: ethdev
          Assignee: [email protected]
          Reporter: [email protected]

Target Milestone: ---

In the article:
https://pvs-studio.com/en/blog/posts/cpp/1183/
Fix is not obvious, either offset the bus_type_id or change checks.

Bug 35: strange check and possible array overrun static const char *const sa_nthw_fpga_bus_type_str[] = {

  "ERR",  /* NTHW_FPGA_BUS_TYPE_UNKNOWN, */
  "BAR",  /* NTHW_FPGA_BUS_TYPE_BAR, */
  "PCI",  /* NTHW_FPGA_BUS_TYPE_PCI, */
  "CCIP",  /* NTHW_FPGA_BUS_TYPE_CCIP, */
  "RAB0",  /* NTHW_FPGA_BUS_TYPE_RAB0, */
  "RAB1",  /* NTHW_FPGA_BUS_TYPE_RAB1, */
  "RAB2",  /* NTHW_FPGA_BUS_TYPE_RAB2, */

"NMB", /* NTHW_FPGA_BUS_TYPE_NMB, */ "NDM", /* NTHW_FPGA_BUS_TYPE_NDM, */
};
static const char *get_bus_name(int n_bus_type_id) {
if (n_bus_type_id >= 1 &&

      n_bus_type_id <= (int)ARRAY_SIZE(sa_nthw_fpga_bus_type_str))
    return sa_nthw_fpga_bus_type_str[n_bus_type_id];
  else
    return "ERR";
}

PVS-Studio warnings:

V557 Array overrun is possible. The value of 'n_bus_type_id' index could reach
9. nthw_fpga_model.c 32

The n_bus_type_id index is checked before extracting a row from an array. There are two questions to this check:

Why is an index starting with 1 considered valid?
Why is the right boundary checked using the <= operator? If the index is equal to the number of elements in the array, an Off-by-one Error will occur. I would venture to guess that the ID values in the n_bus_type_id variable start with 1. This way, the mistake is that one forgot to subtract 1 before extracting an element from the array. In this case, the correct code will look like this:

static const char *get_bus_name(int n_bus_type_id) {
if (n_bus_type_id >= 1 &&

      n_bus_type_id <= (int)ARRAY_SIZE(sa_nthw_fpga_bus_type_str))
    return sa_nthw_fpga_bus_type_str[n_bus_type_id - 1];
  else
    return "ERR";
}

I'm not sure, though. It's strange that no one noticed that the function returns the wrong lines. Perhaps the indexes are numbered from 0 after all. Then the check should be rewritten:

static const char *get_bus_name(int n_bus_type_id) {
if (n_bus_type_id >= 0 &&

      n_bus_type_id < (int)ARRAY_SIZE(sa_nthw_fpga_bus_type_str))
    return sa_nthw_fpga_bus_type_str[n_bus_type_id];
  else
    return "ERR";
}

Please forgive my uncertainty. It's the first time when I see this code. The code is obviously incorrect, but unfortunately, I am limited in time to study each found error in more depth. There are dozens of them, and one of me.

You are receiving this mail because:
You are the assignee for the bug.

https://bugs.dpdk.org/show_bug.cgi?id=1580

Stephen Hemminger ([email protected]) changed:

What |Removed |Added
----------------------------------------------------------------------------

         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #2 from Stephen Hemminger ([email protected]) --- The scan was done by the owner of the PVS studio blog (not me) and appears to have been rung against github main branch early in 24.11.

You are receiving this mail because:
You are the assignee for the bug.