diff options
| author | Felix Morgner <felix.morgner@ost.ch> | 2026-09-29 11:57:55 +0200 |
|---|---|---|
| committer | Felix Morgner <felix.morgner@ost.ch> | 2026-09-29 11:57:55 +0200 |
| commit | 97d99228466b742133bf12e00197be9b11fcc561 (patch) | |
| tree | d4c164949576c1c1f01c063ded300efa5f74cff6 /kapi | |
| parent | 2315e95d48337aa612b2bb20b5f8d3d0e061716c (diff) | |
| download | kernel-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.
Diffstat (limited to 'kapi')
| -rw-r--r-- | kapi/kapi/interrupts/irq_lock.hpp | 58 |
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; } |
