diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index e97ea13d6..bbe66324c 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -997,7 +997,7 @@ int main(int argc, char *argv[]) shutdown_subdaemons(ld); /* Remove plugins. */ - ld->plugins = tal_free(ld->plugins); + plugins_free(ld->plugins); /* Clean up the JSON-RPC. This needs to happen in a DB transaction since * it might actually be touching the DB in some destructors, e.g., diff --git a/lightningd/plugin.c b/lightningd/plugin.c index ddd37b214..97bc6c184 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -56,6 +56,19 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, return p; } +void plugins_free(struct plugins *plugins) +{ + struct plugin *p; + /* Plugins are usually the unit of allocation, and they are internally + * consistent, so let's free each plugin first. */ + while (!list_empty(&plugins->plugins)) { + p = list_pop(&plugins->plugins, struct plugin, list); + tal_free(p); + } + + tal_free(plugins); +} + static void destroy_plugin(struct plugin *p) { struct plugin_rpccall *call; diff --git a/lightningd/plugin.h b/lightningd/plugin.h index a95eeb5c1..1a47232dd 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -137,6 +137,26 @@ void plugins_add_default_dir(struct plugins *plugins); */ void plugins_init(struct plugins *plugins, const char *dev_plugin_debug); +/** + * Free all resources that are held by plugins in the correct order. + * + * This function ensures that the resources dangling off of the plugins struct + * are freed in the correct order. This is necessary since the struct manages + * two orthogonal sets of resources: + * + * - Plugins + * - Hook calls and notifications + * + * The tal hierarchy is organized in a plugin centric way, i.e., the plugins + * may exit in an arbitrary order and they'll unregister pointers in the other + * resources. However this will fail if `tal_free` decides to free one of the + * non-plugin resources (technically a sibling in the allocation tree) before + * the plugins we will get a use-after-free. This function fixes this by + * freeing in the correct order without adding additional child-relationships + * in the allocation structure and without adding destructors. + */ +void plugins_free(struct plugins *plugins); + /** * Register a plugin for initialization and execution. * diff --git a/lightningd/test/run-find_my_abspath.c b/lightningd/test/run-find_my_abspath.c index efe6fd415..78382bb54 100644 --- a/lightningd/test/run-find_my_abspath.c +++ b/lightningd/test/run-find_my_abspath.c @@ -190,6 +190,9 @@ void per_peer_state_set_fds_arr(struct per_peer_state *pps UNNEEDED, const int * /* Generated stub for plugins_config */ void plugins_config(struct plugins *plugins UNNEEDED) { fprintf(stderr, "plugins_config called!\n"); abort(); } +/* Generated stub for plugins_free */ +void plugins_free(struct plugins *plugins UNNEEDED) +{ fprintf(stderr, "plugins_free called!\n"); abort(); } /* Generated stub for plugins_init */ void plugins_init(struct plugins *plugins UNNEEDED, const char *dev_plugin_debug UNNEEDED) { fprintf(stderr, "plugins_init called!\n"); abort(); }