From 4342d0d234258c46a2af37a4b0f22f894de108e9 Mon Sep 17 00:00:00 2001 From: Alexander Boettcher Date: Fri, 3 Aug 2012 10:56:08 +0200 Subject: [PATCH] NOVA: let thread die if SM cap is invalid Patch prevents following bugs: * In sleep_forever the thread return from semaphore down if cap is revoked during destruction of a thread. This causes an endless loop consuming time not available for other threads. * In lock_helper and cap_sel_alloc the thread return from the lock() method even if the semaphore down call failed because of an revoked semaphore. This lead to the situation that a thread subject to de-construction returns from the lock method, but not holding the lock, entering the critical section and modifying state inside the critical section. Another thread in parallel already in the critical section or entering the critical section also modifies the state. This lead to curious bugs ... * thread_nova, thread_start, irq_session Detect early bugs if the SM is gone unexpectedly where it should never happen. --- base-nova/include/base/sleep.h | 6 +++++- base-nova/include/nova/util.h | 13 ++++++++++++- base-nova/include/signal_session/source_client.h | 9 +++++---- base-nova/src/base/env/cap_sel_alloc.cc | 13 +++++++++++-- base-nova/src/base/lock/lock_helper.h | 4 +++- base-nova/src/base/server/server.cc | 3 +-- base-nova/src/base/thread/thread_nova.cc | 7 +++++-- base-nova/src/core/irq_session_component.cc | 4 +++- base-nova/src/core/thread_start.cc | 3 ++- 9 files changed, 47 insertions(+), 15 deletions(-) diff --git a/base-nova/include/base/sleep.h b/base-nova/include/base/sleep.h index f096ca6fae..26b9b1025d 100644 --- a/base-nova/include/base/sleep.h +++ b/base-nova/include/base/sleep.h @@ -20,6 +20,7 @@ /* NOVA includes */ #include +#include extern int main_thread_running_semaphore(); @@ -32,7 +33,10 @@ namespace Genode { Thread_base *myself = Thread_base::myself(); addr_t sem = myself ? myself->tid().exc_pt_sel + SM_SEL_EC : main_thread_running_semaphore(); - while (1) { Nova::sm_ctrl(sem, Nova::SEMAPHORE_DOWNZERO); } + while (1) { + if (Nova::sm_ctrl(sem, SEMAPHORE_DOWNZERO)) + nova_die(); + } } } diff --git a/base-nova/include/nova/util.h b/base-nova/include/nova/util.h index 956f582cd7..9c1df028c9 100644 --- a/base-nova/include/nova/util.h +++ b/base-nova/include/nova/util.h @@ -17,6 +17,17 @@ #include #include +__attribute__((always_inline)) +inline void nova_die(const char * text = 0) +{ + /* + * If thread is de-constructed the sessions are already gone. + * Be careful when enabling printf here. + */ + while (1) + asm volatile ("ud2a" : : "a"(text)); +} + inline void request_event_portal(Genode::Native_capability cap, Genode::addr_t exc_base, Genode::addr_t event) { @@ -31,7 +42,7 @@ inline void request_event_portal(Genode::Native_capability cap, utcb->msg[0] = event; utcb->set_msg_word(1); - uint8_t res = call(pager_cap.local_name()); + uint8_t res = call(cap.local_name()); if (res) PERR("request of event (%lu) capability selector failed", event); diff --git a/base-nova/include/signal_session/source_client.h b/base-nova/include/signal_session/source_client.h index 3788e777a6..86100c7c2d 100644 --- a/base-nova/include/signal_session/source_client.h +++ b/base-nova/include/signal_session/source_client.h @@ -21,11 +21,12 @@ #ifndef _INCLUDE__SIGNAL_SESSION__SOURCE_CLIENT_H_ #define _INCLUDE__SIGNAL_SESSION__SOURCE_CLIENT_H_ -#include #include #include -#include +/* NOVA includes */ +#include +#include namespace Genode { @@ -34,7 +35,7 @@ namespace Genode { private: /** - * Capability with 'pt_sel' referring to a NOVA semaphore + * Capability referring to a NOVA semaphore */ Native_capability _sem; @@ -78,7 +79,7 @@ namespace Genode { */ if (Nova::sm_ctrl(_sem.local_name(), Nova::SEMAPHORE_DOWN)) - nova_die(__FILE__, __LINE__); + nova_die(); /* * Now that the server has unblocked the semaphore, we are sure diff --git a/base-nova/src/base/env/cap_sel_alloc.cc b/base-nova/src/base/env/cap_sel_alloc.cc index 3a9443b628..2265745a96 100644 --- a/base-nova/src/base/env/cap_sel_alloc.cc +++ b/base-nova/src/base/env/cap_sel_alloc.cc @@ -20,6 +20,7 @@ /* NOVA includes */ #include +#include using namespace Genode; @@ -52,9 +53,17 @@ class Alloc_lock */ Alloc_lock() : _sm_cap(Nova::PD_SEL_CAP_LOCK) { } - void lock() { Nova::sm_ctrl(_sm_cap, Nova::SEMAPHORE_DOWN); } + void lock() + { + if (Nova::sm_ctrl(_sm_cap, Nova::SEMAPHORE_DOWN)) + nova_die(); + } - void unlock() { Nova::sm_ctrl(_sm_cap, Nova::SEMAPHORE_UP); } + void unlock() + { + if (Nova::sm_ctrl(_sm_cap, Nova::SEMAPHORE_UP)) + nova_die(); + } }; diff --git a/base-nova/src/base/lock/lock_helper.h b/base-nova/src/base/lock/lock_helper.h index 3709227712..5222d81570 100644 --- a/base-nova/src/base/lock/lock_helper.h +++ b/base-nova/src/base/lock/lock_helper.h @@ -22,6 +22,7 @@ /* NOVA includes */ #include +#include extern int main_thread_running_semaphore(); @@ -100,5 +101,6 @@ static inline void thread_stop_myself() else sem = main_thread_running_semaphore(); - sm_ctrl(sem, SEMAPHORE_DOWNZERO); + if (sm_ctrl(sem, SEMAPHORE_DOWNZERO)) + nova_die(); } diff --git a/base-nova/src/base/server/server.cc b/base-nova/src/base/server/server.cc index e2f0c7cf4d..858f2a4b6a 100644 --- a/base-nova/src/base/server/server.cc +++ b/base-nova/src/base/server/server.cc @@ -21,9 +21,8 @@ /* NOVA includes */ #include -#include #include - +#include using namespace Genode; diff --git a/base-nova/src/base/thread/thread_nova.cc b/base-nova/src/base/thread/thread_nova.cc index e2f2d70814..162a85b1a9 100644 --- a/base-nova/src/base/thread/thread_nova.cc +++ b/base-nova/src/base/thread/thread_nova.cc @@ -25,7 +25,7 @@ /* NOVA includes */ #include -#include +#include #include using namespace Genode; @@ -139,5 +139,8 @@ void Thread_base::start() void Thread_base::cancel_blocking() { - Nova::sm_ctrl(_tid.exc_pt_sel + Nova::SM_SEL_EC, Nova::SEMAPHORE_UP); + using namespace Nova; + + if (sm_ctrl(_tid.exc_pt_sel + SM_SEL_EC, SEMAPHORE_UP)) + nova_die(); } diff --git a/base-nova/src/core/irq_session_component.cc b/base-nova/src/core/irq_session_component.cc index ac2ff44eac..d994b18a0c 100644 --- a/base-nova/src/core/irq_session_component.cc +++ b/base-nova/src/core/irq_session_component.cc @@ -21,6 +21,7 @@ /* NOVA includes */ #include +#include #include using namespace Genode; @@ -28,7 +29,8 @@ using namespace Genode; void Irq_session_component::wait_for_irq() { - Nova::sm_ctrl(_irq_number, Nova::SEMAPHORE_DOWN); + if (Nova::sm_ctrl(_irq_number, Nova::SEMAPHORE_DOWN)) + nova_die(); } diff --git a/base-nova/src/core/thread_start.cc b/base-nova/src/core/thread_start.cc index 16e9e6d605..e0d2ccef0e 100644 --- a/base-nova/src/core/thread_start.cc +++ b/base-nova/src/core/thread_start.cc @@ -92,5 +92,6 @@ void Thread_base::cancel_blocking() { using namespace Nova; - sm_ctrl(_tid.exc_pt_sel + SM_SEL_EC, SEMAPHORE_UP); + if (sm_ctrl(_tid.exc_pt_sel + SM_SEL_EC, SEMAPHORE_UP)) + nova_die(); }