daemon: introduce observer_finisher for delayed free

We introduce an observer finisher which can be registered in
the compartment struct. This finisher is called now in the method
compartment_notify_observers() after all observers have been executed
in the notify observer list. With this it is possible to register a
callback which can cleanly destroy the compartment object triggerd by
an observer.

Previously on container reload the reaload callback destroyed the
compartment object, but in the compartment_notify observers() method
the compartment object is accessed after each callback, even if it is
the last one. This caused the use-after-free error below.

=================================================================
==210==ERROR: AddressSanitizer: heap-use-after-free on address \
 0x5100000011b8 at pc 0x560d9ae6a3a5 bp 0x7fffd1ff8710 sp 0x7fffd1ff8700
READ of size 8 at 0x5100000011b8 thread T0
    #0 0x560d9ae6a3a4 in compartment_notify_observers daemon/compartment.c:1677
    gyroidos#1 0x560d9ae6fd49 in compartment_sigchld_handle_helpers daemon/compartment.c:700
    gyroidos#2 0x560d9ae70319 in compartment_sigchld_cb daemon/compartment.c:783
    gyroidos#3 0x560d9af16329 in event_signal_handler common/event.c:780
    gyroidos#4 0x560d9af16329 in event_loop common/event.c:851
    gyroidos#5 0x560d9ae4e6ac in main daemon/main.c:146
    gyroidos#6 0x7fe2bf99b863 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    gyroidos#7 0x7fe2bf99b90a in __libc_start_main_impl ../csu/libc-start.c:389
    gyroidos#8 0x560d9ae509d4 in _start (/usr/sbin/cmld+0x13e9d4)

Signed-off-by: Michael Weiß <[email protected]>

daemon/cmld.c

@@ -586,6 +586,15 @@ cmld_container_config_sync_cb(container_t *container, container_callback_t *cb,
 	mem_free0(c_uuid);
 }
 
+static void
+cmld_reload_delayed_free(void *data)
+{
+	ASSERT(data);
+
+	container_t *c = data;
+	container_free(c);
+}
+
 container_t *
 cmld_reload_container(const uuid_t *uuid, const char *path)
 {
@@ -619,7 +628,8 @@ cmld_reload_container(const uuid_t *uuid, const char *path)
 		      container_get_name(c_current));
 
 		cmld_containers_list = list_remove(cmld_containers_list, c_current);
-		container_free(c_current);
+		// delayed free to allow all observers to finish up
+		container_finish_observers(c_current, cmld_reload_delayed_free, c_current);
 	}
 
 	DEBUG("Loaded config for container %s", container_get_name(c));

daemon/compartment.c

@@ -105,6 +105,10 @@ struct compartment {
 	char *debug_log_dir; /* log for output of compartment child */
 
 	list_t *observer_list; /* list of function callbacks to be called when the state changes */
+	void (*observer_finish_cb)(
+		void *); /* finisher called after all observers run during state change */
+	void *observer_finish_cb_data; /* data pointer as paramenter for finisher callback */
+
 	event_timer_t *stop_timer;  /* timer to handle compartment stop timeout */
 	event_timer_t *start_timer; /* timer to handle a compartment start timeout */
 
@@ -697,8 +701,8 @@ compartment_sigchld_handle_helpers(compartment_t *compartment, event_signal_t *s
 		compartment_state_t state = compartment->is_rebooting ?
 						    COMPARTMENT_STATE_REBOOTING :
 						    COMPARTMENT_STATE_STOPPED;
-		compartment_set_state(compartment, state);
 		compartment->is_rebooting = false;
+		compartment_set_state(compartment, state);
 	}
 }
 
@@ -803,14 +807,14 @@ compartment_sigchld_early_cb(UNUSED int signum, event_signal_t *sig, void *data)
 		/* remove the sigchld callback for this early child from the event loop */
 		event_remove_signal(sig);
 		event_signal_free(sig);
+		compartment->pid_early = -1;
 		// cleanup if early child returned with an error
 		if ((WIFEXITED(status) && WEXITSTATUS(status)) || WIFSIGNALED(status)) {
 			if (compartment->pid == -1)
 				compartment_cleanup(compartment, false);
 
 			compartment_set_state(compartment, COMPARTMENT_STATE_STOPPED);
 		}
-		compartment->pid_early = -1;
 	}
 
 	// reap any open helper child and set state accordingly
@@ -1682,6 +1686,11 @@ compartment_notify_observers(compartment_t *compartment)
 			l = l->next;
 		}
 	}
+
+	if (compartment->observer_finish_cb) {
+		DEBUG("all observers handled, observer_finish_cb is set, execute it!");
+		compartment->observer_finish_cb(compartment->observer_finish_cb_data);
+	}
 }
 
 void
@@ -1763,6 +1772,15 @@ compartment_unregister_observer(compartment_t *compartment, compartment_callback
 	}
 }
 
+void
+compartment_finish_observers(compartment_t *compartment, void (*cb)(void *), void *data)
+{
+	ASSERT(compartment);
+
+	compartment->observer_finish_cb = cb;
+	compartment->observer_finish_cb_data = data;
+}
+
 const char *
 compartment_get_key(const compartment_t *compartment)
 {

daemon/compartment.h

@@ -399,6 +399,12 @@ compartment_register_observer(compartment_t *compartment,
 void
 compartment_unregister_observer(compartment_t *compartment, compartment_callback_t *cb);
 
+/**
+ * Set observer finishiner callback.
+ */
+void
+compartment_finish_observers(compartment_t *compartment, void (*cb)(void *), void *data);
+
 /**
  * Gets the compartment's key previously set by compartment_set_key or NULL if no key
  * has been set.

daemon/container.c

@@ -313,6 +313,15 @@ container_unregister_observer(container_t *container, container_callback_t *cont
 	mem_free0(container_cb);
 }
 
+void
+container_finish_observers(container_t *container, void (*cb)(void *), void *data)
+{
+	ASSERT(container);
+	ASSERT(cb);
+
+	compartment_finish_observers(container->compartment, cb, data);
+}
+
 void
 container_init_env_prepend(container_t *container, char **init_env, size_t init_env_len)
 {

daemon/container.h

@@ -393,6 +393,9 @@ container_register_observer(container_t *container,
 void
 container_unregister_observer(container_t *container, container_callback_t *cb);
 
+void
+container_finish_observers(container_t *container, void (*cb)(void *), void *data);
+
 void
 container_init_env_prepend(container_t *container, char **init_env, size_t init_env_len);