NOVA: Improve stability of process destruction

The UTCB of the thread cleaning up thread objects has been unmapped.
However the UTCB of the destroyed thread must be unmapped.

Objects must explicitly be made unreachable before cleaning up. The
server and pager objects must be unreachable before they can be freed.
Both object types are threads. Revoking the thread(EC) cap on NOVA
doesn't mean that the thread stops executing. All portals pointing to a
thread are still reachable by clients even if the last EC cap is gone in
user land. So it must be taken care that no portals are pointing anymore
to a thread when the associated objects are getting destroyed. This
commit handles this.

Additionally, even if the last portal is gone - there can be still an
ongoing request handled by such server/pager object/threads. For each
such object an additional portal is created. This object is called
'cleanup portal' and is only local to the object. After all portals are
revoked the cleanup portal is called. When the call returns we know that
nobody is anymore handled by the object since all remotely available
portals are gone.

Fixes #20

base-nova/include/base/pager.h

@@ -44,8 +44,9 @@ namespace Genode {
 			 */
 			Signal_context_capability _exception_sigh;
 
-			int _exc_pt_sel;       /* base of event portal window */
+			unsigned _exc_pt_sel;  /* base of event portal window */
-			int _pt_sel;           /* portal selector for object identity */
+			unsigned _pt_sel;      /* portal selector for object identity */
+			unsigned _pt_cleanup;  /* portal selector for object cleanup/destruction */
 
 			addr_t _initial_esp;
 			addr_t _initial_eip;
@@ -75,7 +76,7 @@ namespace Genode {
 			/**
 			 * Return base of initial portal window
 			 */
-			int exc_pt_sel() { return _exc_pt_sel; }
+			unsigned exc_pt_sel() { return _exc_pt_sel; }
 
 			/**
 			 * Set initial stack pointer used by the startup handler
@@ -90,7 +91,7 @@ namespace Genode {
 			/**
 			 * Return portal capability selector used for object identity
 			 */
-			int pt_sel() { return _pt_sel; }
+			unsigned pt_sel() { return _pt_sel; }
 
 			/**
 			 * Continue execution of pager object
@@ -119,7 +120,7 @@ namespace Genode {
 	class Pager_activation_base { };
 
 
-	template <int STACK_SIZE>
+	template <unsigned STACK_SIZE>
 	class Pager_activation : public Pager_activation_base
 	{ };
 

base-nova/src/base/pager/pager.cc

@@ -48,9 +48,11 @@ void Pager_object::_page_fault_handler()
 	pf_lock()->unlock();
 
 	if (ret) {
-		PWRN("page-fault resolution for for address 0x%lx failed, going to sleep forever",
+		PWRN("page-fault resolution for address 0x%lx, ip=0x%lx failed",
-		     ipc_pager.fault_addr());
+		     ipc_pager.fault_addr(), ipc_pager.fault_ip());
-		sleep_forever();
+		/* revoke paging capability */
+		Nova::revoke(Nova::Obj_crd(obj->exc_pt_sel() + PT_SEL_PAGE_FAULT, 0), true);
+		ipc_pager.wait_for_fault();
 	}
 
 	ipc_pager.reply_and_wait_for_fault();
@@ -78,7 +80,7 @@ void Pager_object::_invoke_handler()
 	Pager_object *obj = static_cast<Pager_object *>(Thread_base::myself());
 
 	/* send single portal as reply */
-	int event = utcb->msg[0];
+	addr_t event = utcb->msg[0];
 	utcb->mtd = 0;
 
 	if (event == PT_SEL_STARTUP || event == PT_SEL_PAGE_FAULT)
@@ -103,10 +105,10 @@ Pager_object::Pager_object(unsigned long badge)
 	mword_t  thread_utcb = (mword_t)  &_context->utcb;
 
 	/* create local EC */
-	int res = create_ec(_tid.ec_sel, pd_sel,
+	uint8_t res = create_ec(_tid.ec_sel, pd_sel,
-	                    CPU_NO, thread_utcb,
+	                        CPU_NO, thread_utcb,
-	                    (mword_t)thread_sp, /* <- delivered to the startup handler */
+	                        (mword_t)thread_sp, /* <- delivered to the startup handler */
-	                    EXC_BASE, GLOBAL);
+	                        EXC_BASE, GLOBAL);
 	if (res)
 		PDBG("create_ec returned %d", res);
 
@@ -143,15 +145,44 @@ Pager_object::Pager_object(unsigned long badge)
 	res = create_pt(_pt_sel, pd_sel, _tid.ec_sel, Mtd(0), (mword_t)_invoke_handler);
 	if (res)
 		PERR("could not create pager object identity, create_pt returned %d\n", res);
-}
 
+	_pt_cleanup = cap_selector_allocator()->alloc();
+	res = create_pt(_pt_cleanup, pd_sel, _tid.ec_sel, Mtd(0), (mword_t)_invoke_handler);
+	if (res)
+		PERR("could not create pager cleanup portal, create_pt returned %d\n", res);
+}
 
 Pager_object::~Pager_object()
 {
-	revoke(Obj_crd(_tid.ec_sel, 0));
+	/* Revoke thread capability */
+	revoke(Obj_crd(_tid.ec_sel, 0), true);
+	/* Revoke thread portals serving exceptions */
+	revoke(Obj_crd(_exc_pt_sel,NUM_INITIAL_PT_LOG2), true);
+	/* Revoke portal used as identity object */
+	revoke(Obj_crd(_pt_sel, 0), true);
+
+	/* Make sure nobody is in the handler anymore by doing an IPC to a
+         * local cap pointing to same 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();
+	utcb->mtd = 0;
+	if (uint8_t res = call(_pt_cleanup))
+		PERR("failure - cleanup call failed res=%d", res);
+
+	/* Revoke portal used for the cleanup call */
+	revoke(Obj_crd(_pt_cleanup, 0), true);
+
+	cap_selector_allocator()->free(_tid.ec_sel, 0);
+	cap_selector_allocator()->free(_exc_pt_sel, NUM_INITIAL_PT_LOG2);
+	cap_selector_allocator()->free(_pt_sel, 0);
+	cap_selector_allocator()->free(_pt_cleanup, 0);
+
 	/* revoke utcb */
 	Rights rwx(true, true, true);
-	revoke(Nova::Mem_crd((addr_t)Thread_base::myself()->utcb() >> 12, 0, rwx));
+	mword_t utcb_context = reinterpret_cast<mword_t>(&_context->utcb);
+	revoke(Nova::Mem_crd(utcb_context >> 12, 0, rwx));
 }
 
 
@@ -170,6 +201,13 @@ Pager_capability Pager_entrypoint::manage(Pager_object *obj)
 
 void Pager_entrypoint::dissolve(Pager_object *obj)
 {
+	/* cleanup cap session */
+	_cap_session->free(obj->Object_pool<Pager_object>::Entry::cap());
+	if (obj->pt_sel() != 0UL + obj->Object_pool<Pager_object>::Entry::cap().dst()) {
+		cap_selector_allocator()->free(obj->Object_pool<Pager_object>::Entry::cap().dst(), 0);
+		revoke(Obj_crd(obj->Object_pool<Pager_object>::Entry::cap().dst(), 0), true);
+	}
+
 	pf_lock()->lock();
 	remove(obj);
 	pf_lock()->unlock();

base-nova/src/base/server/server.cc

@@ -23,6 +23,15 @@
 
 using namespace Genode;
 
+/**
+ * Function gets called during destruction of an entrypoint.
+ * It is just here to make sure nobody else is in the entrypoint
+ * anymore (all communication portals must be revoked beforehand).
+ */
+static void _cleanup_entry()
+{
+	Nova::reply(Thread_base::myself()->stack_top());
+}
 
 /***********************
  ** Server entrypoint **
@@ -32,19 +41,26 @@ Untyped_capability Rpc_entrypoint::_manage(Rpc_object_base *obj)
 {
 	using namespace Nova;
 
-	int ec_sel = tid().ec_sel;
+	unsigned ec_sel = tid().ec_sel;
-	int pt_sel = cap_selector_allocator()->alloc();
+	addr_t pt_sel = cap_selector_allocator()->alloc(1);
-	int pd_sel = cap_selector_allocator()->pd_sel();
+	unsigned pd_sel = cap_selector_allocator()->pd_sel();
 
 	/* create portal */
-	int res = create_pt(pt_sel, pd_sel, ec_sel, Mtd(0),
+	uint8_t res = create_pt(pt_sel, pd_sel, ec_sel, Mtd(0),
-	                    (mword_t)_activation_entry);
+	                        (mword_t)_activation_entry);
-
 	if (res) {
 		PERR("could not create server-object portal, create_pt returned %d\n",
 		     res);
 		return Untyped_capability();
 	}
+	/* create cleanup portal */
+	res = create_pt(pt_sel + 1, pd_sel, ec_sel, Mtd(0),
+	                (mword_t)_cleanup_entry);
+	if (res) {
+		PERR("could not create server-object-cleanup portal, create_pt returned %d\n",
+		     res);
+		return Untyped_capability();
+	}
 
 	/* create capability to portal as destination address */
 	Untyped_capability ep_cap = Native_capability(pt_sel, 0);
@@ -52,6 +68,12 @@ Untyped_capability Rpc_entrypoint::_manage(Rpc_object_base *obj)
 	/* supplement capability with object ID obtained from CAP session */
 	Untyped_capability new_obj_cap = _cap_session->alloc(ep_cap);
 
+	if (new_obj_cap.dst() != ep_cap.dst()) {
+		Nova::revoke(Nova::Obj_crd(new_obj_cap.dst(), 0), true);
+		cap_selector_allocator()->free(new_obj_cap.dst(), 0);
+		new_obj_cap = Native_capability(ep_cap.dst(), new_obj_cap.local_name());
+	}
+
 	/* add server object to object pool */
 	obj->cap(new_obj_cap);
 	insert(obj);
@@ -63,6 +85,9 @@ Untyped_capability Rpc_entrypoint::_manage(Rpc_object_base *obj)
 
 void Rpc_entrypoint::_dissolve(Rpc_object_base *obj)
 {
+	/* Avoid any incoming IPC early */
+	Nova::revoke(Nova::Obj_crd(obj->cap().dst(), 0), true);
+
 	/* make sure nobody is able to find this object */
 	remove(obj);
 
@@ -81,17 +106,23 @@ void Rpc_entrypoint::_dissolve(Rpc_object_base *obj)
 	obj->lock();
 
 	/* now the object may be safely destructed */
-}
 
+	_cap_session->free(obj->cap());
+	/* revoke cleanup portal */
+	Nova::revoke(Nova::Obj_crd(obj->cap().dst() + 1, 0), true);
+	/* free 2 cap selectors */
+	cap_selector_allocator()->free(obj->cap().dst(), 1);
+
+}
 
 void Rpc_entrypoint::_activation_entry()
 {
-	/* retrieve portal id from eax */
 #ifdef __x86_64__
-       addr_t id_pt; asm volatile ("" : "=D" (id_pt));
+	addr_t id_pt; asm volatile ("" : "=D" (id_pt));
 #else
-       addr_t id_pt; asm volatile ("" : "=a" (id_pt));
+	addr_t id_pt; asm volatile ("" : "=a" (id_pt));
 #endif
+	/* retrieve portal id from eax */
 	Rpc_entrypoint *ep = static_cast<Rpc_entrypoint *>(Thread_base::myself());
 
 	Ipc_server srv(&ep->_snd_buf, &ep->_rcv_buf);
@@ -112,7 +143,7 @@ void Rpc_entrypoint::_activation_entry()
 
 		ep->_curr_obj = ep->obj_by_id(srv.badge());
 		if (!ep->_curr_obj) {
-			PERR("could not look up server object, return from call");
+			PERR("could not look up server object, return from call badge=%lx id_pt=%lx", srv.badge(), id_pt);
 			ep->_curr_obj_lock.unlock();
 			srv << IPC_REPLY;
 		}
@@ -140,7 +171,16 @@ void Rpc_entrypoint::entry()
 }
 
 
-void Rpc_entrypoint::_leave_server_object(Rpc_object_base *obj) { }
+void Rpc_entrypoint::_leave_server_object(Rpc_object_base *obj)
+{
+	Nova::Utcb *utcb = reinterpret_cast<Nova::Utcb *>(Thread_base::myself()->utcb());
+	/* don't call ourself */
+	if (utcb != reinterpret_cast<Nova::Utcb *>(&_context->utcb)) {
+		utcb->mtd = 0;
+		if (Nova::call(obj->cap().dst() + 1))
+			PERR("could not clean up entry point");
+	}
+}
 
 
 void Rpc_entrypoint::_block_until_cap_valid() { }

base-nova/src/base/thread/thread_nova.cc

@@ -107,7 +107,8 @@ void Thread_base::_deinit_platform_thread()
 
 	/* revoke utcb */
 	Nova::Rights rwx(true, true, true);
-	Nova::revoke(Nova::Mem_crd((addr_t)Thread_base::myself()->utcb() >> 12, 0, rwx));
+	addr_t utcb = reinterpret_cast<addr_t>(&_context->utcb);
+	Nova::revoke(Nova::Mem_crd(utcb >> 12, 0, rwx));
 }