aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorNRK <nrk@disroot.org>2023-01-28 18:38:52 +0600
committerWilliam Hubbs <w.d.hubbs@gmail.com>2023-04-24 19:20:19 -0500
commitbbd3acfc678a891b9ec2b021ad9f745c34e554f7 (patch)
tree531e67035db17ac6dedd4fc3cbe192c74c0bfef3 /src
parent910e3e2a0ebc13e98554831423e21112026a82be (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.c40
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");