arch: common: sw_isr: make sure that the table index is within range

Assert that the `local_irq` of each levels should only ranges from `0` to `CONFIG_MAX_IRQ_PER_AGGREGATOR`, so that it doesn't overflow the other aggregators. Also, assert that the output of `z_get_sw_isr_table_idx` shouldn't overflow the ISR table. Update the `sw_isr_table` tests to test the range of `CONFIG_MAX_IRQ_PER_AGGREGATOR` instead of the entire range of level bits. Signed-off-by: Yong Cong Sin <ycsin@meta.com>
zephyrproject-rtos · Dec 8, 2023 · f3da086 · f3da086
1 parent a28da92
commit f3da086
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 7 deletions.
diff --git a/arch/common/multilevel_irq.c b/arch/common/multilevel_irq.c
@@ -6,7 +6,9 @@
  */
 
 #include <zephyr/device.h>
+#include <zephyr/irq.h>
 #include <zephyr/sw_isr_table.h>
+#include <zephyr/sys/__assert.h>
 #include <zephyr/sys/util.h>
 
 /*
@@ -133,28 +135,31 @@ unsigned int z_get_sw_isr_irq_from_device(const struct device *dev)
 
 unsigned int z_get_sw_isr_table_idx(unsigned int irq)
 {
-	unsigned int table_idx;
-	unsigned int level, parent_irq, parent_offset;
+	unsigned int table_idx, level, parent_irq, local_irq, parent_offset;
 	const struct _irq_parent_entry *entry = NULL;
 
 	level = irq_get_level(irq);
 
 	if (level == 2U) {
+		local_irq = irq_from_level_2(irq);
+		__ASSERT_NO_MSG(local_irq < CONFIG_MAX_IRQ_PER_AGGREGATOR);
 		parent_irq = irq_parent_level_2(irq);
 		entry = get_parent_entry(parent_irq,
 					 _lvl2_irq_list,
 					 CONFIG_NUM_2ND_LEVEL_AGGREGATORS);
 		parent_offset = entry != NULL ? entry->offset : 0U;
-		table_idx = parent_offset + irq_from_level_2(irq);
+		table_idx = parent_offset + local_irq;
 	}
 #ifdef CONFIG_3RD_LEVEL_INTERRUPTS
 	else if (level == 3U) {
+		local_irq = irq_from_level_3(irq);
+		__ASSERT_NO_MSG(local_irq < CONFIG_MAX_IRQ_PER_AGGREGATOR);
 		parent_irq = irq_parent_level_3(irq);
 		entry = get_parent_entry(parent_irq,
 					 _lvl3_irq_list,
 					 CONFIG_NUM_3RD_LEVEL_AGGREGATORS);
 		parent_offset = entry != NULL ? entry->offset : 0U;
-		table_idx = parent_offset + irq_from_level_3(irq);
+		table_idx = parent_offset + local_irq;
 	}
 #endif /* CONFIG_3RD_LEVEL_INTERRUPTS */
 	else {
@@ -163,5 +168,7 @@ unsigned int z_get_sw_isr_table_idx(unsigned int irq)
 
 	table_idx -= CONFIG_GEN_IRQ_START_VECTOR;
 
+	__ASSERT_NO_MSG(table_idx < IRQ_TABLE_SIZE);
+
 	return table_idx;
 }
diff --git a/arch/common/sw_isr_common.c b/arch/common/sw_isr_common.c
@@ -15,5 +15,9 @@
 
 unsigned int __weak z_get_sw_isr_table_idx(unsigned int irq)
 {
-	return irq - CONFIG_GEN_IRQ_START_VECTOR;
+	unsigned int table_idx = irq - CONFIG_GEN_IRQ_START_VECTOR;
+
+	__ASSERT_NO_MSG(table_idx < IRQ_TABLE_SIZE);
+
+	return table_idx;
 }
diff --git a/tests/kernel/interrupt/src/sw_isr_table.c b/tests/kernel/interrupt/src/sw_isr_table.c
@@ -32,7 +32,7 @@ ZTEST(interrupt_feature, test_sw_isr_irq_parent_table_idx)
 		parent_isr_offset = _lvl2_irq_list[i].offset;
 
 		for (unsigned int local_irq = 0;
-		     local_irq < BIT_MASK(CONFIG_2ND_LEVEL_INTERRUPT_BITS); local_irq++) {
+		     local_irq < CONFIG_MAX_IRQ_PER_AGGREGATOR; local_irq++) {
 			test_irq = irq_to_level_2(local_irq) | parent_irq;
 			test_isr_offset = z_get_sw_isr_table_idx(test_irq);
 			zassert_equal(parent_isr_offset + local_irq, test_isr_offset,
@@ -65,7 +65,7 @@ ZTEST(interrupt_feature, test_sw_isr_irq_parent_table_dev)
 		parent_irq = _lvl2_irq_list[i].irq;
 
 		for (unsigned int local_irq = 0;
-		     local_irq < BIT_MASK(CONFIG_2ND_LEVEL_INTERRUPT_BITS); local_irq++) {
+		     local_irq < CONFIG_MAX_IRQ_PER_AGGREGATOR; local_irq++) {
 			test_irq = irq_to_level_2(local_irq) | parent_irq;
 			test_dev = z_get_sw_isr_device_from_irq(test_irq);
 			zassert_equal_ptr(parent_dev, test_dev, "expected dev: %p, got: %p",