From bbd3acfc678a891b9ec2b021ad9f745c34e554f7 Mon Sep 17 00:00:00 2001 From: NRK Date: Sat, 28 Jan 2023 18:38:52 +0600 Subject: rc: avoid calling free inside SIGCHLD handler `free` is not async-signal-safe and calling it inside a signal handler can have bad effects, as reported in the musl ML: https://www.openwall.com/lists/musl/2023/01/23/1 the solution: - keep track of weather remove_pid() is being called from inside a signal handler or not. - if it's inside a signal handler then DO NOT call free - instead put that pointer into a "to be freed later" list. - if it's not inside a signal handler then take the "to be freed later" list and free anything in it. Bug: https://github.com/OpenRC/openrc/issues/589 Reported-by: Dominique MARTINET --- src/openrc/rc.c | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/openrc/rc.c b/src/openrc/rc.c index 5d2d7049..d162b514 100644 --- a/src/openrc/rc.c +++ b/src/openrc/rc.c @@ -87,6 +87,7 @@ static RC_HOOK hook_out; struct termios *termios_orig = NULL; RC_PIDLIST service_pids; +RC_PIDLIST free_these_pids; static void clean_failed(void) @@ -145,6 +146,12 @@ cleanup(void) p1 = p2; } + for (p1 = LIST_FIRST(&free_these_pids); p1; /* no-op */) { + p2 = LIST_NEXT(p1, entries); + free(p1); + p1 = p2; + } + rc_stringlist_free(main_hotplugged_services); rc_stringlist_free(main_stop_services); rc_stringlist_free(main_start_services); @@ -350,16 +357,24 @@ add_pid(pid_t pid) } static void -remove_pid(pid_t pid) +remove_pid(pid_t pid, bool inside_signal) { - RC_PID *p; - - LIST_FOREACH(p, &service_pids, entries) - if (p->pid == pid) { - LIST_REMOVE(p, entries); - free(p); - return; - } + RC_PID *p, *tmp; + + LIST_FOREACH(p, &service_pids, entries) { + if (p->pid == pid) { + LIST_REMOVE(p, entries); + LIST_INSERT_HEAD(&free_these_pids, p, entries); + break; + } + } + /* only call free if we're not inside a signal handler */ + if (!inside_signal) { + LIST_FOREACH_SAFE(p, &free_these_pids, entries, tmp) { + LIST_REMOVE(p, entries); + free(p); + } + } } static void @@ -397,7 +412,7 @@ handle_signal(int sig) /* Remove that pid from our list */ if (pid > 0) - remove_pid(pid); + remove_pid(pid, true); break; case SIGWINCH: @@ -606,7 +621,7 @@ stop: add_pid(pid); if (!parallel) { rc_waitpid(pid); - remove_pid(pid); + remove_pid(pid, false); } } } @@ -672,7 +687,7 @@ do_start_services(const RC_STRINGLIST *start_services, bool parallel) add_pid(pid); if (!parallel) { rc_waitpid(pid); - remove_pid(pid); + remove_pid(pid, false); } } } @@ -745,6 +760,7 @@ int main(int argc, char **argv) applet = basename_c(argv[0]); LIST_INIT(&service_pids); + LIST_INIT(&free_these_pids); atexit(cleanup); if (!applet) eerrorx("arguments required"); -- cgit v1.2.3