From 8d1cf08b1a998073ad47b7200cf2ef291e9c0d7f Mon Sep 17 00:00:00 2001
From: Norman Feske <norman.feske@genode-labs.com>
Date: Fri, 19 Aug 2016 17:55:39 +0200
Subject: [PATCH] core: fix regression in dataspace ownership test

This patch revives our ds_ownership test from 2012, which just revealed
a regression in core where the dataspace-free operation of the RAM
service would unconditionally destroy dataspace objects from foreign
sessions. The patch fixes the bug and adds an updated version of the
test to the autopilot.

Fixes #2065
---
 repos/base/run/ds_ownership.run              |  7 +--
 repos/base/src/core/ram_session_component.cc | 11 ++--
 repos/base/src/test/ds_ownership/main.cc     | 56 +++++++++-----------
 tool/autopilot.list                          |  1 +
 4 files changed, 35 insertions(+), 40 deletions(-)

diff --git a/repos/base/run/ds_ownership.run b/repos/base/run/ds_ownership.run
index 038c4a7c8..f682db889 100644
--- a/repos/base/run/ds_ownership.run
+++ b/repos/base/run/ds_ownership.run
@@ -33,10 +33,7 @@ install_config {
 
 build_boot_image "core init test-ds_ownership"
 
-run_genode_until {.*Test ended.*\.} 10
+append qemu_args "-nographic -m 64"
 
-grep_output {\[init -\> test-ds_ownership\] Test ended}
+run_genode_until {.*test succeeded.*\n} 10
 
-compare_output_to {
-	[init -> test-ds_ownership] Test ended successfully.
-}
diff --git a/repos/base/src/core/ram_session_component.cc b/repos/base/src/core/ram_session_component.cc
index 70a5e27c7..087be5fc8 100644
--- a/repos/base/src/core/ram_session_component.cc
+++ b/repos/base/src/core/ram_session_component.cc
@@ -34,13 +34,13 @@ addr_t Ram_session_component::phys_addr(Ram_dataspace_capability ds)
 
 void Ram_session_component::_free_ds(Dataspace_capability ds_cap)
 {
-	Dataspace_component *ds;
+	Dataspace_component *ds = nullptr;
 	_ds_ep->apply(ds_cap, [&] (Dataspace_component *c)
 	{
-		ds = c;
+		if (!c) return;
+		if (!c->owner(this)) return;
 
-		if (!ds) return;
-		if (!ds->owner(this)) return;
+		ds = c;
 
 		size_t ds_size = ds->size();
 
@@ -62,7 +62,8 @@ void Ram_session_component::_free_ds(Dataspace_capability ds_cap)
 	});
 
 	/* call dataspace destructors and free memory */
-	destroy(&_ds_slab, ds);
+	if (ds)
+		destroy(&_ds_slab, ds);
 }
 
 
diff --git a/repos/base/src/test/ds_ownership/main.cc b/repos/base/src/test/ds_ownership/main.cc
index c1e98f6ec..28ce8d03d 100644
--- a/repos/base/src/test/ds_ownership/main.cc
+++ b/repos/base/src/test/ds_ownership/main.cc
@@ -1,57 +1,53 @@
 /*
  * \brief  Testing the distinction between user and owner of a RAM dataspace
  * \author Martin Stein
+ * \author Norman Feske
  * \date   2012-04-19
- *
  */
 
 /*
- * Copyright (C) 2008-2013 Genode Labs GmbH
+ * Copyright (C) 2008-2016 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.
  */
 
 /* Genode includes */
-#include <base/env.h>
 #include <ram_session/connection.h>
-#include <base/printf.h>
-
-using namespace Genode;
+#include <base/log.h>
+#include <base/component.h>
 
 
-int main(int argc, char **argv)
+void Component::construct(Genode::Env &env)
 {
-	/* Create some RAM sessions */
-	printf("Dataspace ownership test\n");
-	static Ram_connection ram_1;
-	static Ram_connection ram_2;
+	using namespace Genode;
 
-	/* Allocate dataspace at one of the RAM sessions */
-	ram_1.ref_account(env()->ram_session_cap());
-	env()->ram_session()->transfer_quota(ram_1.cap(), 8*1024);
+	log("--- dataspace ownership test ---");
+
+	static Ram_connection ram_1 { env };
+	static Ram_connection ram_2 { env };
+
+	log("allocate dataspace from one RAM session");
+	ram_1.ref_account(env.ram_session_cap());
+	env.ram().transfer_quota(ram_1.cap(), 8*1024);
 	Ram_dataspace_capability ds = ram_1.alloc(sizeof(unsigned));
 
-	/* Try to free dataspace at another RAM session */
+	log("attempt to free dataspace from foreign RAM session");
 	ram_2.free(ds);
 
-	/* Check if dataspace was falsely freed */
-	try { env()->rm_session()->attach(ds); }
-	catch (...) {
-		printf("Test ended faulty.\n");
-		return -2;
-	}
+	log("try to attach dataspace to see if it still exists");
+	env.rm().attach(ds);
 
-	/* Try to free dataspace at its originating RAM session */
+	log("attach operation succeeded");
+
+	log("free dataspace from legitimate RAM session");
+	size_t const quota_before_free = ram_1.avail();
 	ram_1.free(ds);
+	size_t const quota_after_free = ram_1.avail();
 
-	/* Check if dataspace was freed as expected */
-	try { env()->rm_session()->attach(ds); }
-	catch (...) {
-		printf("Test ended successfully.\n");
-		return 0;
-	}
-	printf("Test ended faulty.\n");
-	return -4;
+	if (quota_after_free > quota_before_free)
+		log("test succeeded");
+	else
+		error("test failed");
 }
 
diff --git a/tool/autopilot.list b/tool/autopilot.list
index b1f07be5d..c34733c49 100644
--- a/tool/autopilot.list
+++ b/tool/autopilot.list
@@ -66,3 +66,4 @@ clipboard
 rust
 xml_node
 fpu
+ds_ownership