Fiasco.OC: fix bugs in cap_map insertion/removal

The following fixes partly solve the problems triggered by the noux stress
test introduced by nfeske in issue #208.
* The check whether a capability exists in the Cap_map, and its insertion,
  has to be done atomically
* While removing a capability it is looked up in the Cap_map via its id,
  check whether the found capability pointer is the same like the looked up,
  otherwise the wrong capability gets freed
* When a local capability is un- resp. marshalled, only the local pointer
  gets transfered, not the redundant capability id
* Introduce several assertions and warnings to facilitate debugging

base-foc/include/base/cap_map.h

@@ -237,6 +237,20 @@ namespace Genode
 			 */
 			Cap_index* insert(int id, addr_t kcap);
 
+			/**
+			 * Create and insert a new Cap_index with a specific capability id
+			 * and map from given kcap to newly allocated one
+			 *
+			 * Allocation of the Cap_index is done via the global
+			 * Cap_index_allocator, which might throw exceptions that aren't
+			 * caught by this method
+			 *
+			 * \param  id the global capability id
+			 * \return    pointer to the new Cap_index object, or zero
+			 *            when allocation failed
+			 */
+			Cap_index* insert_map(int id, addr_t kcap);
+
 			/**
 			 * Remove a Cap_index object
 			 *

base-foc/include/base/ipc.h

@@ -16,23 +16,37 @@
 #define _INCLUDE__BASE__IPC_H_
 
 #include <base/ipc_generic.h>
+#include <util/assert.h>
 
-namespace Fiasco {
-#include <l4/sys/consts.h>
-#include <l4/sys/task.h>
-}
 
 inline void Genode::Ipc_ostream::_marshal_capability(Genode::Native_capability const &cap)
 {
-	bool local = cap.local();
-	long id    = local ? (long)cap.local() : cap.local_name();
+	using namespace Fiasco;
 
-	_write_to_buf(local);
-	_write_to_buf(id);
+	/* first transfer local capability value */
+	_write_to_buf(cap.local());
 
-	/* only transfer kernel-capability if it's no local capability and valid */
-	if (!local && id)
+	/* if it's a local capability we're done */
+	if (cap.local())
+		return;
+
+	if (cap.valid()) {
+		if (!l4_msgtag_label(l4_task_cap_valid(L4_BASE_TASK_CAP, cap.dst()))) {
+			_write_to_buf(0);
+			return;
+		}
+	}
+
+	/* transfer capability id */
+	_write_to_buf(cap.local_name());
+
+	/* only transfer kernel-capability if it's a valid one */
+	if (cap.valid())
 		_snd_msg->snd_append_cap_sel(cap.dst());
+
+	ASSERT(!cap.valid() ||
+	       l4_msgtag_label(l4_task_cap_valid(L4_BASE_TASK_CAP, cap.dst())),
+	       "Send invalid cap");
 }
 
 
@@ -40,54 +54,28 @@ inline void Genode::Ipc_istream::_unmarshal_capability(Genode::Native_capability
 {
 	using namespace Fiasco;
 
-	bool local = false;
-	long id    = 0;
+	long value = 0;
 
-	/* extract capability id from message buffer, and whether it's a local cap */
-	_read_from_buf(local);
-	_read_from_buf(id);
+	/* get local capability pointer from message buffer */
+	_read_from_buf(value);
 
 	/* if it's a local capability, the pointer is marshalled in the id */
-	if (local) {
-		cap = Capability<Native_capability>::local_cap((Native_capability*)id);
+	if (value) {
+		cap = Capability<Native_capability>::local_cap((Native_capability*)value);
 		return;
 	}
 
+	/* extract capability id from message buffer */
+	_read_from_buf(value);
+
 	/* if id is zero an invalid capability was transfered */
-	if (!id) {
+	if (!value) {
 		cap = Native_capability();
 		return;
 	}
 
-	/* we received a valid, non-local capability, maybe we already own it? */
-	Cap_index *i = cap_map()->find(id);
-	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 = 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;
-		}
-	}
-	/* 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);
+	/* try to insert received capability in the map and return it */
+	cap = Native_capability(cap_map()->insert_map(value, _rcv_msg->rcv_cap_sel()));
 }
 
 #endif /* _INCLUDE__BASE__IPC_H_ */

base-foc/include/util/assert.h

@@ -0,0 +1,37 @@
+/*
+ * \brief  Assertion macros for Fiasco.OC
+ * \author Stefan Kalkowski
+ * \date   2012-05-25
+ */
+
+/*
+ * Copyright (C) 2006-2012 Genode Labs GmbH
+ *
+ * This file is part of the Genode OS framework, which is distributed
+ * under the terms of the GNU General Public License version 2.
+ */
+
+#ifndef _INCLUDE__UTIL__ASSERT_H_
+#define _INCLUDE__UTIL__ASSERT_H_
+
+#include <base/printf.h>
+
+namespace Fiasco {
+#include <l4/sys/kdebug.h>
+}
+
+#if 1
+#define ASSERT(e, s) \
+	do { if (!(e)) {								   \
+		Fiasco::outstring(ESC_ERR s ESC_END "\n");	   \
+		Fiasco::outstring(__FILE__ ":");			   \
+		Fiasco::outhex32((int)__LINE__);			   \
+		Fiasco::outstring("\n");					   \
+		enter_kdebug("ASSERT");				   \
+		}											   \
+	} while(0)
+#else
+#define ASSERT(e, s)  do { } while (0)
+#endif
+
+#endif /* _INCLUDE__UTIL__ASSERT_H_ */

base-foc/src/base/env/cap_map.cc

@@ -16,6 +16,13 @@
 #include <base/cap_map.h>
 #include <base/native_types.h>
 
+#include <util/assert.h>
+
+namespace Fiasco {
+#include <l4/sys/consts.h>
+#include <l4/sys/task.h>
+}
+
 
 /***********************
  **  Cap_index class  **
@@ -62,6 +69,9 @@ Genode::Cap_index* Genode::Capability_map::insert(int id)
 
 	Lock_guard<Spin_lock> guard(_lock);
 
+	ASSERT(!_tree.first() || !_tree.first()->find_by_id(id),
+	       "Double insertion in cap_map()!");
+
 	Cap_index *i = cap_idx_alloc()->alloc(1);
 	if (i) {
 		i->id(id);
@@ -77,6 +87,9 @@ Genode::Cap_index* Genode::Capability_map::insert(int id, addr_t kcap)
 
 	Lock_guard<Spin_lock> guard(_lock);
 
+	ASSERT(!_tree.first() || !_tree.first()->find_by_id(id),
+	       "Double insertion in cap_map()!");
+
 	Cap_index *i = cap_idx_alloc()->alloc(kcap, 1);
 	if (i) {
 		i->id(id);
@@ -86,6 +99,50 @@ Genode::Cap_index* Genode::Capability_map::insert(int id, addr_t kcap)
 }
 
 
+Genode::Cap_index* Genode::Capability_map::insert_map(int id, addr_t kcap)
+{
+	using namespace Genode;
+	using namespace Fiasco;
+
+	Lock_guard<Spin_lock> guard(_lock);
+
+	Cap_index* i = 0;
+
+	/* check whether capability id exists */
+	if (_tree.first())
+		i = _tree.first()->find_by_id(id);
+
+	/* if we own the capability already check whether it's the same */
+	if (i) {
+		l4_msgtag_t tag = l4_task_cap_equal(L4_BASE_TASK_CAP, i->kcap(), kcap);
+		if (!l4_msgtag_label(tag)) {
+			/*
+			 * they aren't equal, possibly an already revoked cap,
+			 * otherwise it's a fake capability and we return an invalid one
+			 */
+			tag = l4_task_cap_valid(L4_BASE_TASK_CAP, i->kcap());
+			if (l4_msgtag_label(tag))
+				return 0;
+		} else
+			/* they are equal so just return the one in the map */
+			return i;
+	} else {
+		/* the capability doesn't exists in the map so allocate a new one */
+		i = cap_idx_alloc()->alloc(1);
+		if (!i)
+			return 0;
+		i->id(id);
+		_tree.insert(i);
+	}
+
+	/* map the given cap to our registry entry */
+	l4_task_map(L4_BASE_TASK_CAP, L4_BASE_TASK_CAP,
+				l4_obj_fpage(kcap, 0, L4_FPAGE_RWX),
+				i->kcap() | L4_ITEM_MAP | L4_MAP_ITEM_GRANT);
+	return i;
+}
+
+
 void Genode::Capability_map::remove(Genode::Cap_index* i)
 {
 	using namespace Genode;
@@ -93,12 +150,10 @@ void Genode::Capability_map::remove(Genode::Cap_index* i)
 	Lock_guard<Spin_lock> guard(_lock);
 
 	if (i) {
-		if (_tree.first())
-			i = _tree.first()->find_by_id(i->id());
-		if (i) {
+		Cap_index* e = _tree.first() ? _tree.first()->find_by_id(i->id()) : 0;
+		if (e == i)
 			_tree.remove(i);
-			cap_idx_alloc()->free(i, 1);
-		}
+		cap_idx_alloc()->free(i, 1);
 	}
 }
 

base-foc/src/base/pager/pager.cc

@@ -59,8 +59,10 @@ void Pager_activation_base::entry()
 		Pager_object *obj = _ep->obj_by_id(pager.badge());
 
 		/* the pager_object might be destroyed, while we got the message */
-		if (!obj)
+		if (!obj) {
+			PWRN("No pager object found!");
 			continue;
+		}
 
 		switch (pager.msg_type()) {
 

base-foc/src/base/thread/thread_start.cc

@@ -62,13 +62,7 @@ void Thread_base::start()
 		l4_utcb_tcr_u(state.utcb)->user[UTCB_TCR_THREAD_OBJ] = (addr_t)this;
 
 		/* there might be leaks in the application */
-		Cap_index *idx = cap_map()->find(state.id);
-		if (idx) {
-			idx->inc();
-			PWRN("leaking capability idx=%p id=%x ref_cnt=%d",
-			     idx, idx->id(), idx->dec());
-			cap_map()->remove(idx);
-		}
+		cap_map()->remove(cap_map()->find(state.id));
 
 		/* we need to manually increase the reference counter here */
 		cap_map()->insert(state.id, state.kcap)->inc();

base-foc/src/core/cap_session_component.cc

@@ -30,6 +30,8 @@ namespace Fiasco {
 #include <l4/sys/types.h>
 }
 
+#include <util/assert.h>
+
 using namespace Genode;
 
 /***************************
@@ -73,6 +75,9 @@ void Cap_mapping::unmap()
 {
 	using namespace Fiasco;
 
+	if (!local)
+		return;
+
 	l4_msgtag_t tag = l4_task_unmap(L4_BASE_TASK_CAP,
 	                                l4_obj_fpage(local->kcap(), 0, L4_FPAGE_RWX),
 	                                L4_FP_ALL_SPACES | L4_FP_DELETE_OBJ);
@@ -125,11 +130,21 @@ Native_capability Cap_session_component::alloc(Cap_session_component *session,
 
 		Core_cap_index* ref = static_cast<Core_cap_index*>(ep.idx());
 
+		ASSERT(ref && ref->pt(), "No valid platform_thread");
+		ASSERT(ref->pt()->thread().local, "No valid platform_thread cap set");
+
 		/*
 		 * 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 = static_cast<Core_cap_index*>(cap_map()->insert(id));
+
+		if (!idx) {
+			PWRN("Out of capabilities!");
+			platform_specific()->cap_id_alloc()->free(id);
+			return cap;
+		}
+
 		l4_msgtag_t tag = l4_factory_create_gate(L4_BASE_FACTORY_CAP,
 		                                         idx->kcap(),
 		                                         ref->pt()->thread().local->kcap(), id);