[DPDK/ethdev Bug 1580] Nthw: array overrun

[bugzilla at dpdk.org]

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: dev at dpdk.org
          Reporter: stephen at networkplumber.org
  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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/dev/attachments/20241115/5dae0010/attachment.htm>