CC2530: ccpcheck on ZNP/Z-Stack 3.0.2

Mario DW

Part Number: CC2530
Other Parts Discussed in Thread: Z-STACK

Hi,

I ran 'cppcheck' on the Z-Stack 3.0.2 code and there was an interesting notification:

[Components/osal/common/OSAL_Memory.c:454]: (error) Array 'proCur[8]' accessed at index 8, which is out of bounds.
[Components/osal/common/OSAL_Memory.c:455]: (error) Array 'proCur[8]' accessed at index 8, which is out of bounds.
[Components/osal/common/OSAL_Memory.c:550]: (error) Array 'proCur[8]' accessed at index 8, which is out of bounds.
[Components/osal/common/OSAL_Memory.c:455]: (error) Array 'proMax[8]' accessed at index 8, which is out of bounds.

That's the only valid warning, the other kind was a false positive. So that's not too bad, but it's inside a critical function.

If hdr->hdr.len always exceeds proCnt[idx], the 'idx' value reaches 'OSALMEM_PROMAX' and the array bounds are exceeded.

What is the recommended fix ?

over 3 years ago

0 Ryan Brown1 over 3 years ago

TI__Guru**** 202236 points

Hi le_top,

The following is contained in osal_mem_alloc:

#if OSALMEM_PROFILER
#define OSALMEM_PROMAX  8
/* The profiling buckets must differ by at least OSALMEM_MIN_BLKSZ; the
 * last bucket must equal the max alloc size. Set the bucket sizes to
 * whatever sizes necessary to show how your application is using memory.
 */
static uint16 proCnt[OSALMEM_PROMAX] = {
OSALMEM_SMALL_BLKSZ, 48, 112, 176, 192, 224, 256, 65535 };
static uint16 proCur[OSALMEM_PROMAX] = { 0 };
static uint16 proMax[OSALMEM_PROMAX] = { 0 };
static uint16 proTot[OSALMEM_PROMAX] = { 0 };
static uint16 proSmallBlkMiss;
#endif

      for ( idx = 0; idx < OSALMEM_PROMAX; idx++ )
      {
        if ( hdr->hdr.len <= proCnt[idx] )
        {
          break;
        }
      }
      proCur[idx]++;
      if ( proMax[idx] < proCur[idx] )
      {
        proMax[idx] = proCur[idx];
      }
      proTot[idx]++;

I suppose there are cases where idx could reach the limit of the for loop before breaking, in this case it would have a value of 9 8 and thus be an invalid entry for the proCur/proMax/proTot calculations which follow. Perhaps in this situation these calculations should be skipped. This could only ever happen if OSALMEM_PROFILER is defined.

Regards,
Ryan

0 Mario DW over 3 years ago in reply to Ryan Brown1

Genius 3145 points

Hi Ryan

The invalid value for idx would be 8 , but as all information for the algortihm is in one place now, one can see that the last value of proCnt is 65535, which is the uint16 maximum, so the loop breaks at 7.

Still, it would be better to adjust the loop's limit.

In practice, there is no problem in the end ;-).

BR,

Mario

Zigbee & Thread

Zigbee & Thread forum

CC2530: ccpcheck on ZNP/Z-Stack 3.0.2