kern: fix client-controlled kernel panic in kipc

It turns out it's possible to trigger a bounds check in kipc handling
code by submitting an invalid IRQ number. This fixes that, causing the
bounds check to be optimized out.

In several syscall paths (and one kipc) we manipulate interrupts without
accepting an interrupt number from the client. In these cases, a bad
interrupt number would indicate a problem in our dispatch table, so I've
made them explicit panics. (They were panics before, just implicit ones
in the array access.)

This causes a small increase in kernel text size (40-64 bytes), but kipc
should not be able to crash the kernel!

sys/kern/src/arch/arm_m.rs

@@ -83,7 +83,7 @@ use crate::time::Timestamp;
 use crate::umem::USlice;
 #[cfg(any(armv7m, armv8m))]
 use abi::FaultSource;
-use abi::{FaultInfo, InterruptNum};
+use abi::{FaultInfo, InterruptNum, UsageError};
 #[cfg(armv8m)]
 use armv8_m_mpu::{disable_mpu, enable_mpu};
 use unwrap_lite::UnwrapLite;
@@ -1210,7 +1210,10 @@ pub unsafe extern "C" fn DefaultHandler() {
                 .unwrap_or_else(|| panic!("unhandled IRQ {irq_num}"));
 
             let switch = with_task_table(|tasks| {
-                disable_irq(irq_num, false);
+                // This can only fail if the IRQ number is out of range, which
+                // in this case would mean the hardware is conspiring against
+                // us. So ignore it to ensure we don't generate a bogus check.
+                disable_irq(irq_num, false).ok();
 
                 // Now, post the notification and return the
                 // scheduling hint.
@@ -1227,40 +1230,54 @@ pub unsafe extern "C" fn DefaultHandler() {
     crate::profiling::event_isr_exit();
 }
 
-pub fn disable_irq(n: u32, also_clear_pending: bool) {
+pub fn disable_irq(n: u32, also_clear_pending: bool) -> Result<(), UsageError> {
     // Disable the interrupt by poking the Interrupt Clear Enable Register.
     let nvic = unsafe { &*cortex_m::peripheral::NVIC::PTR };
     let reg_num = (n / 32) as usize;
     let bit_mask = 1 << (n % 32);
     unsafe {
-        nvic.icer[reg_num].write(bit_mask);
+        nvic.icer
+            .get(reg_num)
+            .ok_or(UsageError::NoIrq)?
+            .write(bit_mask);
     }
     if also_clear_pending {
         unsafe {
-            nvic.icpr[reg_num].write(bit_mask);
+            nvic.icpr
+                .get(reg_num)
+                .ok_or(UsageError::NoIrq)?
+                .write(bit_mask);
         }
     }
+    Ok(())
 }
 
-pub fn enable_irq(n: u32, also_clear_pending: bool) {
+pub fn enable_irq(n: u32, also_clear_pending: bool) -> Result<(), UsageError> {
     // Enable the interrupt by poking the Interrupt Set Enable Register.
     let nvic = unsafe { &*cortex_m::peripheral::NVIC::PTR };
     let reg_num = (n / 32) as usize;
     let bit_mask = 1 << (n % 32);
     if also_clear_pending {
         // Do this _before_ enabling.
         unsafe {
-            nvic.icpr[reg_num].write(bit_mask);
+            nvic.icpr
+                .get(reg_num)
+                .ok_or(UsageError::NoIrq)?
+                .write(bit_mask);
         }
     }
     unsafe {
-        nvic.iser[reg_num].write(bit_mask);
+        nvic.iser
+            .get(reg_num)
+            .ok_or(UsageError::NoIrq)?
+            .write(bit_mask);
     }
+    Ok(())
 }
 
 /// Looks up an interrupt in the NVIC and returns a cross-platform
 /// representation of that interrupt's status.
-pub fn irq_status(n: u32) -> abi::IrqStatus {
+pub fn irq_status(n: u32) -> Result<abi::IrqStatus, UsageError> {
     let mut status = abi::IrqStatus::empty();
 
     let nvic = unsafe { &*cortex_m::peripheral::NVIC::PTR };
@@ -1269,25 +1286,30 @@ pub fn irq_status(n: u32) -> abi::IrqStatus {
 
     // See if the interrupt is enabled by checking the bit in the Interrupt Set
     // Enable Register.
-    let enabled = nvic.iser[reg_num].read() & bit_mask == bit_mask;
+    let iser_reg = nvic.iser.get(reg_num).ok_or(UsageError::NoIrq)?;
+    let enabled = iser_reg.read() & bit_mask == bit_mask;
     status.set(abi::IrqStatus::ENABLED, enabled);
 
     // See if the interrupt is pending by checking the bit in the Interrupt
     // Set Pending Register (ISPR).
     let pending = nvic.ispr[reg_num].read() & bit_mask == bit_mask;
     status.set(abi::IrqStatus::PENDING, pending);
 
-    status
+    Ok(status)
 }
 
-pub fn pend_software_irq(InterruptNum(n): InterruptNum) {
+pub fn pend_software_irq(
+    InterruptNum(n): InterruptNum,
+) -> Result<(), UsageError> {
     let nvic = unsafe { &*cortex_m::peripheral::NVIC::PTR };
     let reg_num = (n / 32) as usize;
     let bit_mask = 1 << (n % 32);
 
     // Pend the IRQ by poking the corresponding bit in the Interrupt Set Pending
     // Register (ISPR).
-    unsafe { nvic.ispr[reg_num].write(bit_mask) };
+    let ispr_reg = nvic.ispr.get(reg_num).ok_or(UsageError::NoIrq)?;
+    unsafe { ispr_reg.write(bit_mask) };
+    Ok(())
 }
 
 #[repr(u8)]

sys/kern/src/kipc.rs

@@ -5,12 +5,13 @@
 //! Implementation of IPC operations on the virtual kernel task.
 
 use abi::{FaultInfo, Kipcnum, SchedState, TaskState, UsageError};
+use core::mem::size_of;
+use unwrap_lite::UnwrapLite;
 
 use crate::arch;
 use crate::err::UserError;
 use crate::task::{current_id, ArchState, NextTask, Task};
 use crate::umem::USlice;
-use core::mem::size_of;
 
 /// Message dispatcher.
 pub fn handle_kernel_message(
@@ -462,7 +463,9 @@ fn software_irq(
         )))?;
 
     for &irq in irqs.iter() {
-        crate::arch::pend_software_irq(irq);
+        // Any error here would be a problem in our dispatch table, not the
+        // caller, so we panic because we want to hear about it.
+        crate::arch::pend_software_irq(irq).unwrap_lite();
     }
 
     tasks[caller].save_mut().set_send_response_and_length(0, 0);

sys/kern/src/syscalls.rs

@@ -778,7 +778,9 @@ fn irq_control(
             UsageError::NoIrq,
         )))?;
     for i in irqs.iter() {
-        operation(i.0, also_clear_pending);
+        // Because the IRQ numbers are coming from our own table, any error here
+        // would indicate a kernel bug, _not_ bad syscall arguments.
+        operation(i.0, also_clear_pending).unwrap_lite();
     }
     Ok(NextTask::Same)
 }
@@ -918,9 +920,10 @@ fn irq_status(
         )))?;
 
     // Combine the platform-level status of all the IRQs in the notification set.
-    let mut status = irqs.iter().fold(IrqStatus::empty(), |status, irq| {
-        status | crate::arch::irq_status(irq.0)
-    });
+    let mut status =
+        irqs.iter().try_fold(IrqStatus::empty(), |status, irq| {
+            crate::arch::irq_status(irq.0).map(|n| status | n)
+        })?;
 
     // If any bits in the notification mask are set in the caller's notification
     // set, then a notification has been posted to the task and not yet consumed.