From 6accb9703f2071af6f961eb8070ba4c873652ce0 Mon Sep 17 00:00:00 2001 From: ulfvonbelow 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