From 4630c4b9ff29371d43110913dbcd2aa554b7a627 Mon Sep 17 00:00:00 2001
From: Martin Stein <martin.stein@genode-labs.com>
Date: Sat, 2 Jun 2018 00:18:02 +0200
Subject: [PATCH] nic_router: fix domain invalidation during init

Previously we were doing the initialization once over all domains,
remembered which of them became invalid and destroyed those afterwards.
This isn't sufficient. As soon as one domain becomes invalid we have to
dissolve/destroy this one, deinitialize all other domains again (as they
could contain references to the invalid domain) and retry to initialize
them from the beginning. We proceed with this until we have one run
without a domain becoming invalid. Then we can be sure that the last
initialization run did not create references to any invalid domain.

Issue #2840
---
 .../os/src/server/nic_router/configuration.cc | 46 +++++++++++++------
 repos/os/src/server/nic_router/domain.cc      | 27 ++++++-----
 repos/os/src/server/nic_router/domain.h       |  2 +
 3 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/repos/os/src/server/nic_router/configuration.cc b/repos/os/src/server/nic_router/configuration.cc
index 33d3efb6e0..f998ec7afa 100644
--- a/repos/os/src/server/nic_router/configuration.cc
+++ b/repos/os/src/server/nic_router/configuration.cc
@@ -65,7 +65,7 @@ Configuration::Configuration(Env               &env,
 	_tcp_max_segm_lifetime(read_sec_attr(node, "tcp_max_segm_lifetime_sec", DEFAULT_TCP_MAX_SEGM_LIFETIME_SEC)),
 	_node(node)
 {
-	/* read domains */
+	/* do parts of domain initialization that do not lookup other domains */
 	node.for_each_sub_node("domain", [&] (Xml_node const node) {
 		try {
 			Domain &domain = *new (_alloc) Domain(*this, node, _alloc);
@@ -77,20 +77,40 @@ Configuration::Configuration(Env               &env,
 		}
 		catch (Domain::Invalid) { log("[?] invalid domain"); }
 	});
-	/* do those parts of domain init that require the domain tree to be complete */
-	List<Domain> invalid_domains;
-	_domains.for_each([&] (Domain &domain) {
+	/* do parts of domain initialization that may lookup other domains */
+	while (true) {
+
+		struct Retry_without_domain : Genode::Exception
+		{
+			Domain &domain;
+
+			Retry_without_domain(Domain &domain) : domain(domain) { }
+		};
 		try {
-			domain.init(_domains);
-			if (_verbose) {
-				log("[", domain, "] initiated domain"); }
+			_domains.for_each([&] (Domain &domain) {
+				try { domain.init(_domains); }
+				catch (Domain::Invalid) { throw Retry_without_domain(domain); }
+				if (_verbose) {
+					log("[", domain, "] initiated domain"); }
+			});
 		}
-		catch (Domain::Invalid) { invalid_domains.insert(&domain); }
-	});
-	invalid_domains.for_each([&] (Domain &domain) {
-		_domains.remove(domain);
-		destroy(_alloc, &domain);
-	});
+		catch (Retry_without_domain exception) {
+
+			/* deinitialize all domains again */
+			_domains.for_each([&] (Domain &domain) {
+				domain.deinit();
+				if (_verbose) {
+					log("[", domain, "] deinitiated domain"); }
+			});
+			/* destroy domain that became invalid during initialization */
+			_domains.remove(exception.domain);
+			destroy(_alloc, &exception.domain);
+
+			/* retry to initialize the remaining domains */
+			continue;
+		}
+		break;
+	}
 	try {
 		/* check whether we shall create a report generator */
 		Xml_node const report_node = node.sub_node("report");
diff --git a/repos/os/src/server/nic_router/domain.cc b/repos/os/src/server/nic_router/domain.cc
index 918a0ad550..2a6ba96a1c 100644
--- a/repos/os/src/server/nic_router/domain.cc
+++ b/repos/os/src/server/nic_router/domain.cc
@@ -204,18 +204,7 @@ Link_side_tree &Domain::links(L3_protocol const protocol)
 
 Domain::~Domain()
 {
-	/* destroy rules */
-	_ip_rules.destroy_each(_alloc);
-	_nat_rules.destroy_each(_alloc);
-	_icmp_rules.destroy_each(_alloc);
-	_udp_rules.destroy_each(_alloc);
-	_tcp_rules.destroy_each(_alloc);
-	_udp_forward_rules.destroy_each(_alloc);
-	_tcp_forward_rules.destroy_each(_alloc);
-
-	/* destroy DHCP server and IP config */
-	try { destroy(_alloc, &_dhcp_server()); }
-	catch (Pointer<Dhcp_server>::Invalid) { }
+	deinit();
 	_ip_config.destruct();
 }
 
@@ -297,6 +286,20 @@ void Domain::init(Domain_tree &domains)
 }
 
 
+void Domain::deinit()
+{
+	_ip_rules.destroy_each(_alloc);
+	_nat_rules.destroy_each(_alloc);
+	_icmp_rules.destroy_each(_alloc);
+	_udp_rules.destroy_each(_alloc);
+	_tcp_rules.destroy_each(_alloc);
+	_udp_forward_rules.destroy_each(_alloc);
+	_tcp_forward_rules.destroy_each(_alloc);
+	try { destroy(_alloc, &_dhcp_server()); }
+	catch (Pointer<Dhcp_server>::Invalid) { }
+}
+
+
 Ipv4_address const &Domain::next_hop(Ipv4_address const &ip) const
 {
 	if (ip_config().interface.prefix_matches(ip)) { return ip; }
diff --git a/repos/os/src/server/nic_router/domain.h b/repos/os/src/server/nic_router/domain.h
index c0a1b98982..4c092208d0 100644
--- a/repos/os/src/server/nic_router/domain.h
+++ b/repos/os/src/server/nic_router/domain.h
@@ -125,6 +125,8 @@ class Net::Domain : public Domain_base,
 
 		void init(Domain_tree &domains);
 
+		void deinit();
+
 		Ipv4_address const &next_hop(Ipv4_address const &ip) const;
 
 		void ip_config(Ipv4_config const &ip_config);