From ca004658d99abef856789a66f52dcb1ca5765ede Mon Sep 17 00:00:00 2001 From: Stefan Kalkowski Date: Tue, 17 Apr 2012 15:09:27 +0200 Subject: [PATCH] Fiasco.OC: smart-pointer for kernel capabilities. Implements Native_capability as smart-pointer type referencing Cap_index objects. Whenever capabilities are copied, assigned, constructed, or destructed the reference-counter of the Cap_index is incremented/decremented. When it reaches zero the Cap_index is removed from the process-global cap_map and gets freed. Fix for issue #32. --- base-foc/include/base/cap_map.h | 7 +++-- base-foc/include/base/ipc.h | 36 +++++++++++++--------- base-foc/include/base/native_types.h | 34 +++++++++++++++++++- base-foc/src/base/env/cap_map.cc | 2 +- base-foc/src/base/thread/thread.cc | 6 +++- base-foc/src/base/thread/thread_start.cc | 17 +++------- base-foc/src/core/cap_session_component.cc | 28 +++++++++++------ base-foc/src/core/include/platform_pd.h | 3 +- base-foc/src/core/pd_session_extension.cc | 2 +- base-foc/src/core/platform.cc | 2 +- base-foc/src/core/platform_thread.cc | 10 +++--- base-foc/src/platform/_main_helper.h | 6 ++-- 12 files changed, 100 insertions(+), 53 deletions(-) diff --git a/base-foc/include/base/cap_map.h b/base-foc/include/base/cap_map.h index 61529ac35a..9d73984cc8 100644 --- a/base-foc/include/base/cap_map.h +++ b/base-foc/include/base/cap_map.h @@ -53,16 +53,19 @@ namespace Genode enum { INVALID_ID = -1, UNUSED = 0 }; - uint16_t _id; /* global capability id */ + uint8_t _ref_cnt; /* reference counter */ + uint16_t _id; /* global capability id */ public: - Cap_index() : _id(INVALID_ID) { } + Cap_index() : _ref_cnt(0), _id(INVALID_ID) { } bool valid() const { return _id != INVALID_ID; } bool used() const { return _id != UNUSED; } uint16_t id() const { return _id; } void id(uint16_t id) { _id = id; } + uint8_t inc() { return ++_ref_cnt; } + uint8_t dec() { return --_ref_cnt; } addr_t kcap(); void* operator new (size_t size, Cap_index* idx) { return idx; } diff --git a/base-foc/include/base/ipc.h b/base-foc/include/base/ipc.h index 7fdfed4803..dcb9c5c361 100644 --- a/base-foc/include/base/ipc.h +++ b/base-foc/include/base/ipc.h @@ -53,7 +53,7 @@ inline void Genode::Ipc_istream::_unmarshal_capability(Genode::Native_capability return; } - /* if id is zero an invalid capability was tranfered */ + /* if id is zero an invalid capability was transfered */ if (!id) { cap = Native_capability(); return; @@ -61,26 +61,32 @@ inline void Genode::Ipc_istream::_unmarshal_capability(Genode::Native_capability /* we received a valid, non-local capability, maybe we already own it? */ Cap_index *i = cap_map()->find(id); - bool map = false; + Genode::addr_t rcv_cap = _rcv_msg->rcv_cap_sel(); if (i) { /** * If we've a dead capability in our database, which is already * revoked, its id might be reused. */ - l4_msgtag_t tag = Fiasco::l4_task_cap_valid(L4_BASE_TASK_CAP, i->kcap()); - if (!tag.label()) - map = true; - } else { - /* insert the new capability in the map */ - i = cap_map()->insert(id); - map = true; + l4_msgtag_t tag = l4_task_cap_valid(L4_BASE_TASK_CAP, i->kcap()); + if (!l4_msgtag_label(tag)) { + i->inc(); + PWRN("leaking capability idx=%p id=%x ref_cnt=%d",i, i->id(), i->dec()); + } else { + /* does somebody tries to fake us? */ + tag = l4_task_cap_equal(L4_BASE_TASK_CAP, i->kcap(), rcv_cap); + if (!l4_msgtag_label(tag)) { + PWRN("Got fake capability"); + cap = Native_capability(); + } else + cap = Native_capability(i); + return; + } } - - /* map the received capability from the receive-buffer if necessary */ - if (map) - l4_task_map(L4_BASE_TASK_CAP, L4_BASE_TASK_CAP, - l4_obj_fpage(_rcv_msg->rcv_cap_sel(), 0, L4_FPAGE_RWX), - i->kcap() | L4_ITEM_MAP | L4_MAP_ITEM_GRANT); + /* insert the new capability in the map */ + i = cap_map()->insert(id); + l4_task_map(L4_BASE_TASK_CAP, L4_BASE_TASK_CAP, + l4_obj_fpage(rcv_cap, 0, L4_FPAGE_RWX), + i->kcap() | L4_ITEM_MAP | L4_MAP_ITEM_GRANT); cap = Native_capability(i); } diff --git a/base-foc/include/base/native_types.h b/base-foc/include/base/native_types.h index 16a72e5900..2c1ae5d361 100644 --- a/base-foc/include/base/native_types.h +++ b/base-foc/include/base/native_types.h @@ -78,6 +78,19 @@ namespace Genode { */ Native_capability(void* ptr) : _idx(0), _ptr(ptr) { } + inline void _inc() + { + if (_idx) + _idx->inc(); + } + + inline void _dec() + { + if (_idx && !_idx->dec()) { + cap_map()->remove(_idx); + } + } + public: /** @@ -88,7 +101,16 @@ namespace Genode { /** * Construct capability manually */ - Native_capability(Cap_index* idx) : _idx(idx), _ptr(0) { } + Native_capability(Cap_index* idx) + : _idx(idx), _ptr(0) { _inc(); } + + Native_capability(const Native_capability &o) + : _idx(o._idx), _ptr(o._ptr) { _inc(); } + + ~Native_capability() + { + _dec(); + } /** * Return Cap_index object referenced by this object @@ -101,6 +123,16 @@ namespace Genode { bool operator==(const Native_capability &o) const { return (_ptr) ? _ptr == o._ptr : _idx == o._idx; } + Native_capability& operator=(const Native_capability &o){ + if (this == &o) + return *this; + + _dec(); + _ptr = o._ptr; + _idx = o._idx; + _inc(); + return *this; + } /******************************************* ** Interface provided by all platforms ** diff --git a/base-foc/src/base/env/cap_map.cc b/base-foc/src/base/env/cap_map.cc index 93c6600737..c5e5bfbb0f 100644 --- a/base-foc/src/base/env/cap_map.cc +++ b/base-foc/src/base/env/cap_map.cc @@ -97,7 +97,7 @@ void Genode::Capability_map::remove(Genode::Cap_index* i) i = _tree.first()->find_by_id(i->id()); if (i) { _tree.remove(i); - cap_idx_alloc()->free(i,1); + cap_idx_alloc()->free(i, 1); } } } diff --git a/base-foc/src/base/thread/thread.cc b/base-foc/src/base/thread/thread.cc index 8b8aba7745..710f08782c 100644 --- a/base-foc/src/base/thread/thread.cc +++ b/base-foc/src/base/thread/thread.cc @@ -151,8 +151,12 @@ Thread_base::Context *Thread_base::_alloc_context(size_t stack_size) /* * Now the thread context is backed by memory, so it is safe to access its * members. + * + * We need to initalize the context object's memory with zeroes, + * otherwise the ds_cap isn't invalid. That would cause trouble + * when the assignment operator of Native_capability is used. */ - + memset(context, 0, sizeof(Context)); context->thread_base = this; context->stack_base = ds_addr; context->ds_cap = ds_cap; diff --git a/base-foc/src/base/thread/thread_start.cc b/base-foc/src/base/thread/thread_start.cc index 1dfa0cae5c..2739d8be05 100644 --- a/base-foc/src/base/thread/thread_start.cc +++ b/base-foc/src/base/thread/thread_start.cc @@ -57,23 +57,14 @@ void Thread_base::start() _tid = state.kcap; _context->utcb = state.utcb; - /** - * If we've a dead capability in our database, which is already - * revoked, its id might be reused. - */ - Cap_index *i = cap_map()->find(state.id); - if (i) { - l4_msgtag_t tag = l4_task_cap_valid(L4_BASE_TASK_CAP, i->kcap()); - if (!tag.label()) - cap_map()->remove(i); - } - try { l4_utcb_tcr_u(state.utcb)->user[UTCB_TCR_BADGE] = state.id; l4_utcb_tcr_u(state.utcb)->user[UTCB_TCR_THREAD_OBJ] = (addr_t)this; - cap_map()->insert(state.id, state.kcap); + + /* we need to manually increase the reference counter here */ + cap_map()->insert(state.id, state.kcap)->inc(); } catch(Cap_index_allocator::Region_conflict) { - PERR("could not insert id %lx", state.id); + PERR("could not insert id %x", state.id); } /* register initial IP and SP at core */ diff --git a/base-foc/src/core/cap_session_component.cc b/base-foc/src/core/cap_session_component.cc index 8c188775ad..b7da5f9bc1 100644 --- a/base-foc/src/core/cap_session_component.cc +++ b/base-foc/src/core/cap_session_component.cc @@ -50,7 +50,7 @@ Genode::Cap_index_allocator* Genode::cap_idx_alloc() Core_cap_index* Cap_mapping::_get_cap() { int id = platform_specific()->cap_id_alloc()->alloc(); - return reinterpret_cast(cap_map()->insert(id)); + return static_cast(cap_map()->insert(id)); } @@ -82,17 +82,27 @@ void Cap_mapping::unmap() Cap_mapping::Cap_mapping(bool alloc, Native_thread_id r) -: local(alloc ? _get_cap() : 0), remote(r) { } +: local(alloc ? _get_cap() : 0), remote(r) +{ + if (local) + local->inc(); +} Cap_mapping::Cap_mapping(Core_cap_index* i, Native_thread_id r) -: local(i), remote(r) { } +: local(i), remote(r) +{ + if (local) + local->inc(); +} Cap_mapping::~Cap_mapping() { - unmap(); - cap_map()->remove(local); + if (local) { + unmap(); + cap_map()->remove(local); + } } @@ -113,14 +123,13 @@ Native_capability Cap_session_component::alloc(Cap_session_component *session, try { using namespace Fiasco; - Core_cap_index* ref = reinterpret_cast(ep.idx()); + Core_cap_index* ref = static_cast(ep.idx()); /* * Allocate new id, and ipc-gate and set id as gate-label */ unsigned long id = platform_specific()->cap_id_alloc()->alloc(); - Core_cap_index* idx = - reinterpret_cast(cap_map()->insert(id)); + Core_cap_index* idx = static_cast(cap_map()->insert(id)); l4_msgtag_t tag = l4_factory_create_gate(L4_BASE_FACTORY_CAP, idx->kcap(), ref->pt()->thread().local->kcap(), id); @@ -135,6 +144,7 @@ Native_capability Cap_session_component::alloc(Cap_session_component *session, idx->session(session); idx->pt(ref->pt()); + idx->inc(); cap = Native_capability(idx); } catch (Cap_id_allocator::Out_of_ids) { PERR("Out of IDs"); @@ -156,7 +166,7 @@ void Cap_session_component::free(Native_capability cap) if (!cap.valid()) return; - Core_cap_index* idx = reinterpret_cast(cap.idx()); + Core_cap_index* idx = static_cast(cap.idx()); /* * check whether this cap_session has created the capability to delete. diff --git a/base-foc/src/core/include/platform_pd.h b/base-foc/src/core/include/platform_pd.h index 37ba924e36..29a80638fd 100644 --- a/base-foc/src/core/include/platform_pd.h +++ b/base-foc/src/core/include/platform_pd.h @@ -97,8 +97,7 @@ namespace Genode { ** Fiasco-specific Accessors ** *******************************/ - Native_capability native_task() const { - return Native_capability(_task.local); } + Core_cap_index* native_task() { return _task.local; } }; } diff --git a/base-foc/src/core/pd_session_extension.cc b/base-foc/src/core/pd_session_extension.cc index 07dfdf4548..6800faa3e8 100644 --- a/base-foc/src/core/pd_session_extension.cc +++ b/base-foc/src/core/pd_session_extension.cc @@ -19,4 +19,4 @@ Genode::Native_capability Genode::Pd_session_component::task_cap() { - return _pd.native_task(); } + return Native_capability(_pd.native_task()); } diff --git a/base-foc/src/core/platform.cc b/base-foc/src/core/platform.cc index 69c05fa0e7..b32b1c13a2 100644 --- a/base-foc/src/core/platform.cc +++ b/base-foc/src/core/platform.cc @@ -126,7 +126,7 @@ Platform::Sigma0::Sigma0(Cap_index* i) : Pager_object(0) * We use the Pager_object here in a slightly different manner, * just to tunnel the pager cap to the Platform_thread::start method. */ - cap(reinterpret_cap_cast(Native_capability(i))); + cap(Native_capability(i)); } diff --git a/base-foc/src/core/platform_thread.cc b/base-foc/src/core/platform_thread.cc index 14e9f19a3e..0c19c20df6 100644 --- a/base-foc/src/core/platform_thread.cc +++ b/base-foc/src/core/platform_thread.cc @@ -40,13 +40,13 @@ int Platform_thread::start(void *ip, void *sp) { /* map the pager cap */ if (_platform_pd) - _pager.map(_platform_pd->native_task().dst()); + _pager.map(_platform_pd->native_task()->kcap()); /* reserve utcb area and associate thread with this task */ l4_thread_control_start(); l4_thread_control_pager(_pager.remote); l4_thread_control_exc_handler(_pager.remote); - l4_thread_control_bind(_utcb, _platform_pd->native_task().dst()); + l4_thread_control_bind(_utcb, _platform_pd->native_task()->kcap()); l4_msgtag_t tag = l4_thread_control_commit(_thread.local->kcap()); if (l4_msgtag_has_error(tag)) { PWRN("l4_thread_control_commit for %lx failed!", @@ -132,8 +132,8 @@ void Platform_thread::resume() void Platform_thread::bind(Platform_pd *pd) { _platform_pd = pd; - _gate.map(pd->native_task().dst()); - _irq.map(pd->native_task().dst()); + _gate.map(pd->native_task()->kcap()); + _irq.map(pd->native_task()->kcap()); } @@ -188,7 +188,7 @@ void Platform_thread::_create_thread() PERR("cannot create more thread kernel-objects!"); /* create initial gate for thread */ - _gate.local = reinterpret_cast(Cap_session_component::alloc(0, _thread.local).idx()); + _gate.local = static_cast(Cap_session_component::alloc(0, _thread.local).idx()); } diff --git a/base-foc/src/platform/_main_helper.h b/base-foc/src/platform/_main_helper.h index 0c63dad703..7a75b74df6 100644 --- a/base-foc/src/platform/_main_helper.h +++ b/base-foc/src/platform/_main_helper.h @@ -26,6 +26,8 @@ namespace Fiasco { enum { MAIN_THREAD_CAP_ID = 1 }; static void main_thread_bootstrap() { + using namespace Genode; + /** * Unfortunately ldso calls this function twice. So the second time when * inserting the main thread's gate-capability an exception would be raised. @@ -33,11 +35,11 @@ static void main_thread_bootstrap() { * that's why we first check if the cap is already registered before * inserting it. */ - Genode::Cap_index *idx = Genode::cap_map()->find(MAIN_THREAD_CAP_ID); + Cap_index *idx = cap_map()->find(MAIN_THREAD_CAP_ID); if (!idx) { Fiasco::l4_utcb_tcr()->user[Fiasco::UTCB_TCR_BADGE] = MAIN_THREAD_CAP_ID; Fiasco::l4_utcb_tcr()->user[Fiasco::UTCB_TCR_THREAD_OBJ] = 0; - Genode::cap_map()->insert(MAIN_THREAD_CAP_ID, Fiasco::MAIN_THREAD_CAP); + cap_map()->insert(MAIN_THREAD_CAP_ID, Fiasco::MAIN_THREAD_CAP)->inc(); } }