[Coverity CID :206086] Out-of-bounds access in drivers/timer/cortex_m_systick.c

[zephyrbot]

Static code scan issues found in file:

https://github.com/zephyrproject-rtos/zephyr/tree/42c5b0a7fafa3360b8d835d4b76d7b1bebbe9237/drivers/timer/cortex_m_systick.c#L156

Category: Memory - corruptions
Function: z_clock_driver_init
Component: Drivers
CID: 206086

Details:

150         }
151         z_arm_exc_exit();
152     }
153    
154     int z_clock_driver_init(struct device *device)
155     {
>>>     CID 206086:  Memory - corruptions  (OVERRUN)
>>>     Overrunning callee's array of size 240 by passing argument "SysTick_IRQn" in call to "__NVIC_SetPriority".
156         NVIC_SetPriority(SysTick_IRQn, _IRQ_PRIO_OFFSET);
157         last_load = CYC_PER_TICK - 1;
158         overflow_cyc = 0U;
159         SysTick->LOAD = last_load;
160         SysTick->VAL = 0; /* resets timer to last_load */
161         SysTick->CTRL |= (SysTick_CTRL_ENABLE_Msk |

Please fix or provide comments in coverity using the link:

https://scan9.coverity.com/reports.htm#v32951/p12996.

Note: This issue was created automatically. Priority was set based on classification
of the file affected and the impact field in coverity. Assignees were set using the CODEOWNERS file.

[ioannisg]

This looks like a false-positive to me: SysTick_IRQn is always (or should always be) an element in the IRQn_Type enumeration, defined in vendor CMSIS and NVIC_SetPriority() takes an IRQn_Type as argument.
@pabigot @galak could you look at it so we can, hopefully, close this one?

[pabigot]

I agree it's bogus just based on the call, but I don't understand how Coverity could get here. SysTick_IRQn is always -1, and the branch in NVIC_SetPriority that assigns into a 240-element array requires the int32_t value of IRQn be non-negative. The other branch deals with the SHP array which is always smaller.

[dleach02]

Yes. I've been digging on this for a bit as well. I was at the same place @pabigot is with the -1 and branch in set priority.

It would be useful if we could get the package that was submitted to coverity for this report. The number of false positives like this type should be debugged a bit with coverity so we can figure out why they happen and avoid the time sync that occurs when we triage them.

[ioannisg]

Let's keep this open but remove the bug/priority labels

[nashif]

why is this still open? Can we mark it as false-postive in coverity and close please?

[nashif]

also, if something is not a bug, just close it with reasoning, removing the bug label will just keep it as an issue without a path forward.

[ioannisg]

Listed as False Positive, in Coverity. Closing this down.