summaryrefslogtreecommitdiffstats
path: root/src/vppinfra/interrupt.h
diff options
context:
space:
mode:
authorPaul Atkins <patkins@graphiant.com>2022-04-06 14:51:21 +0100
committerDamjan Marion <dmarion@me.com>2022-04-08 15:35:39 +0000
commitdfc43164078b481e39dc0a87e8e358cc6a56d14e (patch)
treea4367aef179208043d964d77d17b41dd469c5a5a /src/vppinfra/interrupt.h
parent853530b528d57506236c6e2b5ffc02fc976cdc68 (diff)
vppinfra: clib_interrupt_get_next reading unallocated memory
The clib interrupt structure has a couple of fields at the start of the cacheline, and then in the next cacheline it has a bitmap, which is then followed by an atomic bitmap. The size of the bitmaps is based on the number of interrupts, and when the memory is allocated the number of interrupts needed is used to size the overall block of memory. The interrupts typically map to pool entries, so if we want to store 512 entries then we store them in indices 0..511. This would then take 8 6 4bit words, so each bitmap would be this size when the struct is allocated. It is possible to walk over the end of the allocated data with certain sizes, one of which is 512. The reason this happens with 512 is that the check to see when to exit the loop is returning when offset is greater than the value needed to fit all the values. In this case 512 >> 6 = 8. If there had only been 511 entries then the size would have been 511 >> 6 = 7, and so it would have fitted in the space. Therefore modify the check to also check that we are not looking into the memory beyond what we have allocated in the case where the number of interrupt is one of the boundary values like 512. Also add a similar check first time round the loop as it is possible we could have ate same problem there too. Add a new test file to verify the new code works. The old version of the code made this test fail when run with the address sanitizer. Without the sanitiser it tended to pass because the following memory was typically set to 0 even though it was uninitialised. Type: fix Signed-off-by: Paul Atkins <patkins@graphiant.com> Change-Id: I2ec4afae43d296a5c30299bd7694c072ca76b9a4
Diffstat (limited to 'src/vppinfra/interrupt.h')
-rw-r--r--src/vppinfra/interrupt.h4
1 files changed, 3 insertions, 1 deletions
diff --git a/src/vppinfra/interrupt.h b/src/vppinfra/interrupt.h
index 393574b076b..5c39b2183f8 100644
--- a/src/vppinfra/interrupt.h
+++ b/src/vppinfra/interrupt.h
@@ -110,6 +110,8 @@ clib_interrupt_get_next (void *in, int last)
ASSERT (last >= -1 && last < h->n_int);
off = (last + 1) >> log2_uword_bits;
+ if (off > h->n_int >> log2_uword_bits || off >= h->n_uword_alloc)
+ return -1;
last -= off << log2_uword_bits;
bmp[off] |= __atomic_exchange_n (abm + off, 0, __ATOMIC_SEQ_CST);
@@ -121,7 +123,7 @@ next:
off++;
- if (off > h->n_int >> log2_uword_bits)
+ if (off > h->n_int >> log2_uword_bits || off >= h->n_uword_alloc)
return -1;
bmp[off] |= __atomic_exchange_n (abm + off, 0, __ATOMIC_SEQ_CST);