From: Kyuwon Shim Date: Thu, 9 Mar 2023 01:24:47 +0000 (+1300) Subject: ulogd2: avoid use after free in unregister on global ulogd_fds linked list X-Git-Tag: ulogd-2.0.9~34 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=61fc904e36c1d3309829eaa8736d27477208aded;p=thirdparty%2Fulogd2.git ulogd2: avoid use after free in unregister on global ulogd_fds linked list Invalid read of size 4 at 0x405F60: ulogd_unregister_fd (select.c:74) by 0x4E4E3DF: ??? (in /usr/lib/ulogd/ulogd_inppkt_NFLOG.so) by 0x405003: stop_pluginstances (ulogd.c:1335) by 0x405003: sigterm_handler_task (ulogd.c:1383) by 0x405153: call_signal_handler_tasks (ulogd.c:424) by 0x405153: signal_channel_callback (ulogd.c:443) by 0x406163: ulogd_select_main (select.c:105) by 0x403CF3: ulogd_main_loop (ulogd.c:1070) by 0x403CF3: main (ulogd.c:1649) Problem is that ulogd_inppkt_NFLOG.c::stop() calls ulogd_unregister_fd() which does llist_del(). This llist_del may touch ->prev pointer. As the list element is in private data, we cannot do this llist_del from stop_pluginstances(). Therefore, the free() process moved location after finishing ulogd_unregister_fd(). Signed-off-by: Kyuwon Shim Signed-off-by: Florian Westphal --- diff --git a/src/ulogd.c b/src/ulogd.c index 8ea9793..6c5ff9a 100644 --- a/src/ulogd.c +++ b/src/ulogd.c @@ -1334,6 +1334,15 @@ static void stop_pluginstances() (*pi->plugin->stop)(pi); pi->private[0] = 0; } + + /* NB: plugin->stop() might access other plugin instances, + * so we cannot free right away. + */ + } + } + + llist_for_each_entry(stack, &ulogd_pi_stacks, stack_list) { + llist_for_each_entry_safe(pi, npi, &stack->list, list) { free(pi); } }