aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelix Morgner <felix.morgner@ost.ch>2026-09-29 11:57:55 +0200
committerFelix Morgner <felix.morgner@ost.ch>2026-09-29 11:57:55 +0200
commit97d99228466b742133bf12e00197be9b11fcc561 (patch)
treed4c164949576c1c1f01c063ded300efa5f74cff6
parent2315e95d48337aa612b2bb20b5f8d3d0e061716c (diff)
downloadkernel-97d99228466b742133bf12e00197be9b11fcc561.tar.xz
kernel-97d99228466b742133bf12e00197be9b11fcc561.zip
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.
-rw-r--r--kapi/kapi/interrupts/irq_lock.hpp58
1 files changed, 53 insertions, 5 deletions
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;
}