From 97d99228466b742133bf12e00197be9b11fcc561 Mon Sep 17 00:00:00 2001
From: Felix Morgner <felix.morgner@ost.ch>
Date: Tue, 29 Sep 2026 11:57:55 +0200
Subject: kapi/interrupts: improve irq_lock protections

The previous implementation suffered from four problems:
    1. try_lock in the basic irq_lock always succeeded.
    2. lock in the basic irq_lock always succeeded.
    3. try_lock in the wrapper irq_lock did not use the base try_lock.
    4. a foreign CPU could wrongly unlock a remote irq_lock.

These issues are mitigated in this patch.
---
 kapi/kapi/interrupts/irq_lock.hpp | 58 +++++++++++++++++++++++++++++++++++----
 1 file changed, 53 insertions(+), 5 deletions(-)

(limited to 'kapi')

diff --git a/kapi/kapi/interrupts/irq_lock.hpp b/kapi/kapi/interrupts/irq_lock.hpp
index 1b6f6987..57c0ef7f 100644
--- a/kapi/kapi/interrupts/irq_lock.hpp
+++ b/kapi/kapi/interrupts/irq_lock.hpp
@@ -3,7 +3,9 @@
 
 // IWYU pragma: private, include <kapi/interrupts.hpp>
 
+#include <kapi/cpu.hpp>
 #include <kapi/interrupts/state.hpp>
+#include <kapi/system.hpp>
 
 #include <kstd/mutex.hpp>
 
@@ -20,35 +22,76 @@ namespace kapi::interrupts
   template<>
   struct irq_lock<void>
   {
-    constexpr irq_lock() = default;
+    constexpr irq_lock()
+        : m_owner{kapi::cpu::current_id()}
+    {}
+
     constexpr irq_lock(irq_lock const &) = delete;
     constexpr irq_lock(irq_lock &&) = delete;
 
+    ~irq_lock()
+    {
+      assert_owner();
+      if (m_locked)
+      {
+        kapi::system::panic("[OS:INT] IRQ lock destroyed while it is still locked!");
+      }
+    };
+
     constexpr auto operator=(irq_lock const &) = delete;
     constexpr auto operator=(irq_lock &&) = delete;
 
     //! Save the current IRQ state and disable further IRQs.
     auto lock() -> void
     {
-      m_old_irq_state = enabled();
-      enabled(false);
+      if (!try_lock())
+      {
+        kapi::system::panic("[OS:INT] IRQ lock reacquired while it is already held!");
+      }
     }
 
     //! Restore the previously saved IRQ state.
     auto unlock() -> void
     {
+      assert_owner();
+
+      if (!m_locked)
+      {
+        kapi::system::panic("[OS:INT] IRQ lock unlocked while it is not being held!");
+      }
+
       enabled(m_old_irq_state);
+      m_locked = false;
     }
 
     //! Save the current IRQ state and disable further IRQs.
     auto try_lock() -> bool
     {
-      lock();
+      assert_owner();
+      auto old_state = enabled();
+      enabled(false);
+      if (m_locked)
+      {
+        enabled(old_state);
+        return false;
+      }
+      m_locked = true;
+      m_old_irq_state = old_state;
       return true;
     }
 
   private:
+    auto assert_owner() -> void
+    {
+      if (m_owner != kapi::cpu::current_id())
+      {
+        kapi::system::panic("[OS:INT] Tried to modify IRQ lock on a CPU that does not own it!");
+      }
+    }
+
+    bool m_locked{};
     bool m_old_irq_state{};
+    kapi::cpu::id m_owner{};
   };
 
   //! A lock to temporarily disable IRQs while also acquiring a further lock.
@@ -83,11 +126,16 @@ namespace kapi::interrupts
     [[nodiscard]] auto try_lock() -> bool
       requires(kstd::lockable<BasicLockable>)
     {
-      irq_lock<>::lock();
+      if (!irq_lock<>::try_lock())
+      {
+        return false;
+      }
+
       if (m_lockable.try_lock())
       {
         return true;
       }
+
       irq_lock<>::unlock();
       return false;
     }
-- 
cgit v1.2.3