| Message ID | 20230623154135.1930261-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/xtensa: Assert that interrupt level is within bounds | expand |
On Fri, Jun 23, 2023 at 8:41 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > In handle_interrupt() we use level as an index into the interrupt_vector[] > array. This is safe because we have checked it against env->config->nlevel, > but Coverity can't see that (and it is only true because each CPU config > sets its XCHAL_NUM_INTLEVELS to something less than MAX_NLEVELS), so it > complains about a possible array overrun (CID 1507131) > > Add an assert() which will make Coverity happy and catch the unlikely > case of a mis-set XCHAL_NUM_INTLEVELS in future. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > NB: only tested with 'make check-avocado'. You could argue that we > should mark the coverity issue as false-positive instead if you like. > --- > target/xtensa/exc_helper.c | 3 +++ > 1 file changed, 3 insertions(+) Acked-by: Max Filippov <jcmvbkbc@gmail.com>
On Sat, 24 Jun 2023 at 01:20, Max Filippov <jcmvbkbc@gmail.com> wrote: > > On Fri, Jun 23, 2023 at 8:41 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > > > In handle_interrupt() we use level as an index into the interrupt_vector[] > > array. This is safe because we have checked it against env->config->nlevel, > > but Coverity can't see that (and it is only true because each CPU config > > sets its XCHAL_NUM_INTLEVELS to something less than MAX_NLEVELS), so it > > complains about a possible array overrun (CID 1507131) > > > > Add an assert() which will make Coverity happy and catch the unlikely > > case of a mis-set XCHAL_NUM_INTLEVELS in future. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > NB: only tested with 'make check-avocado'. You could argue that we > > should mark the coverity issue as false-positive instead if you like. > > --- > > target/xtensa/exc_helper.c | 3 +++ > > 1 file changed, 3 insertions(+) > > Acked-by: Max Filippov <jcmvbkbc@gmail.com> Thanks. I'll take it via target-arm since I'm doing a pullreq anyway, unless you'd prefer otherwise. -- PMM
On Tue, Jul 4, 2023 at 6:27 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Sat, 24 Jun 2023 at 01:20, Max Filippov <jcmvbkbc@gmail.com> wrote: > > > > On Fri, Jun 23, 2023 at 8:41 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > > > > > In handle_interrupt() we use level as an index into the interrupt_vector[] > > > array. This is safe because we have checked it against env->config->nlevel, > > > but Coverity can't see that (and it is only true because each CPU config > > > sets its XCHAL_NUM_INTLEVELS to something less than MAX_NLEVELS), so it > > > complains about a possible array overrun (CID 1507131) > > > > > > Add an assert() which will make Coverity happy and catch the unlikely > > > case of a mis-set XCHAL_NUM_INTLEVELS in future. > > > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > > --- > > > NB: only tested with 'make check-avocado'. You could argue that we > > > should mark the coverity issue as false-positive instead if you like. > > > --- > > > target/xtensa/exc_helper.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > Acked-by: Max Filippov <jcmvbkbc@gmail.com> > > Thanks. I'll take it via target-arm since I'm doing a pullreq > anyway, unless you'd prefer otherwise. It's good, please go ahead.
diff --git a/target/xtensa/exc_helper.c b/target/xtensa/exc_helper.c index d4823a65cda..43f6a862de2 100644 --- a/target/xtensa/exc_helper.c +++ b/target/xtensa/exc_helper.c @@ -169,6 +169,9 @@ static void handle_interrupt(CPUXtensaState *env) CPUState *cs = env_cpu(env); if (level > 1) { + /* env->config->nlevel check should have ensured this */ + assert(level < sizeof(env->config->interrupt_vector)); + env->sregs[EPC1 + level - 1] = env->pc; env->sregs[EPS2 + level - 2] = env->sregs[PS]; env->sregs[PS] =
In handle_interrupt() we use level as an index into the interrupt_vector[] array. This is safe because we have checked it against env->config->nlevel, but Coverity can't see that (and it is only true because each CPU config sets its XCHAL_NUM_INTLEVELS to something less than MAX_NLEVELS), so it complains about a possible array overrun (CID 1507131) Add an assert() which will make Coverity happy and catch the unlikely case of a mis-set XCHAL_NUM_INTLEVELS in future. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- NB: only tested with 'make check-avocado'. You could argue that we should mark the coverity issue as false-positive instead if you like. --- target/xtensa/exc_helper.c | 3 +++ 1 file changed, 3 insertions(+)