From 6accb9703f2071af6f961eb8070ba4c873652ce0 Mon Sep 17 00:00:00 2001
From: ulfvonbelow <striness@tilde.club>
Date: Wed, 1 May 2024 22:32:13 -0500
Subject: [PATCH] util: temporary workaround for use-after-free in
 finish_client_drop.

The use-after-free is triggered when calling GNUNET_SERVICE_stop on a service
that has clients, which to my knowledge currently only occurs in test code,
thankfully. It occurs because GNUNET_SERVICE_stop calls
GNUNET_SERVICE_client_drop for each of those clients, which schedules a task
to run finish_client_drop, which attempts to read from the
GNUNET_SERVICE_Handle passed to GNUNET_SERVICE_stop. Immediately after
scheduling these tasks, it frees that same handle. This freeing needs to
happen only after finish_client_drop has been run for all the clients. The
workaround enqueues a task to free the handle, which due to the scheduler's
queueing behavior should cause it to run only after all the finish_client_drop
tasks for that handle have run. Just in case in the future the
finish_client_drop tasks are scheduled in such a way that this isn't the
case (for example, if they were scheduled with a lower priority), the freeing
task is also scheduled with GNUNET_SCHEDULER_PRIORITY_IDLE.

A proper fix would probably involve making the teardown code paths for
heap-allocated (created with GNUNET_SERVICE_start()) and
stack-allocated (created with GNUNET_SERVICE_run_()) more similar, or just
making it so that both use heap allocation. It's a bit strange to have both
GNUNET_SERVICE_stop and GNUNET_SERVICE_shutdown, which both do more or less
the same thing but in different contexts.
---
 src/lib/util/service.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/lib/util/service.c b/src/lib/util/service.c
index 7aeabf687..6354718de 100644
--- a/src/lib/util/service.c
+++ b/src/lib/util/service.c
@@ -1904,6 +1904,11 @@ GNUNET_SERVICE_start (const char *service_name,
 }
 
 
+static void free_thing (void *thing)
+{
+  GNUNET_free (thing);
+}
+
 /**
  * Stops a service that was started with #GNUNET_SERVICE_start().
  *
@@ -1916,10 +1921,17 @@ GNUNET_SERVICE_stop (struct GNUNET_SERVICE_Handle *srv)
 
   GNUNET_SERVICE_suspend (srv);
   while (NULL != (client = srv->clients_head))
-    GNUNET_SERVICE_client_drop (client);
+    GNUNET_SERVICE_client_drop (client); // schedules a task that needs srv
   teardown_service (srv);
   GNUNET_free (srv->handlers);
-  GNUNET_free (srv);
+  // ... frees srv
+  /* GNUNET_free (srv); */
+  /* XXX: this relies on either the queue behavior of the scheduler or the
+     priority level of the finish_client_drop task, but should work for
+     now. */
+  GNUNET_SCHEDULER_add_with_priority (GNUNET_SCHEDULER_PRIORITY_IDLE,
+                                      &free_thing,
+                                      srv);
 }
 
 
-- 
2.41.0