diff options
author | NRK <nrk@disroot.org> | 2023-01-28 18:38:52 +0600 |
---|---|---|
committer | William Hubbs <w.d.hubbs@gmail.com> | 2023-04-24 19:20:19 -0500 |
commit | bbd3acfc678a891b9ec2b021ad9f745c34e554f7 (patch) | |
tree | 531e67035db17ac6dedd4fc3cbe192c74c0bfef3 /src | |
parent | 910e3e2a0ebc13e98554831423e21112026a82be (diff) |
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 <dominique.martinet@atmark-techno.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/openrc/rc.c | 40 |
1 files changed, 28 insertions, 12 deletions
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"); |