From edd30b56a292d886d73fe99fa33731d13b0c738c Mon Sep 17 00:00:00 2001 From: Alexander Boettcher Date: Tue, 4 Dec 2012 09:49:01 +0100 Subject: [PATCH] nova: fix pager cleanup handling The cleanup call must be performed already during the _dissolve function shortly after the object at the cap_session is freed up. Otherwise there is the chance that an in-flight IPC will find the to be dissolved function again. Bomb test triggered the case, that a already dissolved rpc_object was found by a in-flight IPC. If the rpc_object was already freed up by alloc->destroy the thread using this stale rpc_object pointer cause page-faults in core. Fixes partly #549 --- base-nova/include/base/pager.h | 9 ++++ base-nova/src/base/pager/pager.cc | 81 ++++++++++++++++--------------- 2 files changed, 51 insertions(+), 39 deletions(-) diff --git a/base-nova/include/base/pager.h b/base-nova/include/base/pager.h index 871fa72fe..1ee6894ff 100644 --- a/base-nova/include/base/pager.h +++ b/base-nova/include/base/pager.h @@ -186,6 +186,15 @@ namespace Genode { */ Thread_capability thread_cap() { return _thread_cap; } const void thread_cap(Thread_capability cap) { _thread_cap = cap; } + + /** + * Make sure nobody is in the handler anymore by doing an IPC to a + * local cap pointing to same serving thread (if not running in the + * context of the serving thread). When the call returns + * we know that nobody is handled by this object anymore, because + * all remotely available portals had been revoked beforehand. + */ + void cleanup_call(); }; diff --git a/base-nova/src/base/pager/pager.cc b/base-nova/src/base/pager/pager.cc index 6832fc7e2..9961e5cc4 100644 --- a/base-nova/src/base/pager/pager.cc +++ b/base-nova/src/base/pager/pager.cc @@ -79,8 +79,8 @@ void Pager_object::_exception_handler(addr_t portal_id) Utcb *utcb = _check_handler(myself, obj); addr_t fault_ip = utcb->ip; - if (obj->submit_exception_signal()) - /* Somebody takes care don't die - just recall and block */ + if (obj->submit_exception_signal()) + /* somebody takes care don't die - just recall and block */ obj->client_recall(); else { PWRN("unresolvable exception at ip 0x%lx, exception portal 0x%lx", @@ -126,13 +126,13 @@ void Pager_object::_recall_handler() utcb->flags = obj->_state.thread.eflags | 0x100UL; utcb->mtd = Nova::Mtd(Mtd::EFL).value(); } else - if (!obj->_state.singlestep && singlestep_state) { - utcb->flags = obj->_state.thread.eflags & ~0x100UL; - utcb->mtd = Nova::Mtd(Mtd::EFL).value(); - } else - utcb->mtd = 0; + if (!obj->_state.singlestep && singlestep_state) { + utcb->flags = obj->_state.thread.eflags & ~0x100UL; + utcb->mtd = Nova::Mtd(Mtd::EFL).value(); + } else + utcb->mtd = 0; utcb->set_msg_word(0); - + reply(myself->stack_top()); } @@ -200,6 +200,27 @@ uint8_t Pager_object::client_recall() } +void Pager_object::cleanup_call() +{ + /* + * Revoke all portals of Pager_object from others. + * The portals will be finally revoked during thread destruction. + */ + revoke(Obj_crd(exc_pt_sel(), NUM_INITIAL_PT_LOG2), false); + + Utcb *utcb = (Utcb *)Thread_base::myself()->utcb(); + if (reinterpret_cast(this->utcb()) == utcb) return; + + /* if pager is blocked wake him up */ + wake_up(); + + utcb->set_msg_word(0); + if (uint8_t res = call(_pt_cleanup)) + PERR("%8p - cleanup call to pager (%8p) failed res=%d", + utcb, this->utcb(), res); +} + + Pager_object::Pager_object(unsigned long badge) : Thread_base("pager", PF_HANDLER_STACK_SIZE), _badge(badge) { @@ -290,40 +311,24 @@ Pager_object::Pager_object(unsigned long badge) Pager_object::~Pager_object() { - /* - * Revoke all portals of Pager_object from others. - * The portals will be finally revoked during thread destruction. - */ - revoke(Obj_crd(exc_pt_sel(), NUM_INITIAL_PT_LOG2), false); - /* revoke semaphore cap to signal valid state after recall */ - addr_t sm_cap = _sm_state_notify; + addr_t sm_cap = _sm_state_notify; _sm_state_notify = Native_thread::INVALID_INDEX; - /* wake up client blocked in a thread::pause call */ + + /* if pager is blocked wake him up */ sm_ctrl(sm_cap, SEMAPHORE_UP); revoke(Obj_crd(sm_cap, 0)); - /* if pager is blocked wake him up */ - wake_up(); - - /* - * Make sure nobody is in the handler anymore by doing an IPC to a - * local cap pointing to same serving thread (if not running in the - * context of the serving thread). When the call returns - * we know that nobody is handled by this object anymore, because - * all remotely available portals had been revoked beforehand. - */ - Utcb *utcb = (Utcb *)Thread_base::myself()->utcb(); - if (reinterpret_cast(&_context->utcb) != utcb) { - utcb->set_msg_word(0); - if (uint8_t res = call(_pt_cleanup)) - PERR("failure - cleanup call failed res=%d", res); - } + /* take care nobody is handled anymore by this object */ + cleanup_call(); /* revoke portal used for the cleanup call */ revoke(Obj_crd(_pt_cleanup, 0)); + + Native_capability pager_obj = ::Object_pool::Entry::cap(); cap_selector_allocator()->free(_pt_cleanup, 0); cap_selector_allocator()->free(sm_cap, 0); + cap_selector_allocator()->free(pager_obj.local_name(), 0); } @@ -346,17 +351,15 @@ Pager_capability Pager_entrypoint::manage(Pager_object *obj) void Pager_entrypoint::dissolve(Pager_object *obj) { + Native_capability pager_obj = obj->Object_pool::Entry::cap(); + /* cleanup at cap session */ - _cap_session->free(obj->Object_pool::Entry::cap()); + _cap_session->free(pager_obj); /* cleanup locally */ - Native_capability pager_pt = - obj->Object_pool::Entry::cap(); - - revoke(pager_pt.dst(), true); - - cap_selector_allocator()->free(pager_pt.local_name(), 0); + revoke(pager_obj.dst(), true); remove_locked(obj); + obj->cleanup_call(); }