From dd77f63ad3ae22e607bd1510294583d4658ec628 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Sun, 26 Jul 2026 22:14:09 +0200 Subject: [PATCH] service: do not let a script timeout take PID 1 with it A stop: or reload: script written with a timeout killed Finit at config load: service stop:5,/bin/true service.sh -- Boom parse_script() takes the timeout as a pointer and the caller decides whether it wants one. However, both stop: and reload: scripts so far have no timeout, i.e., NULL. Guard the branch that reads a leading number. Signed-off-by: Joachim Wiberg --- src/service.c | 3 ++- test/Makefile.am | 3 ++- test/script-timeout.sh | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100755 test/script-timeout.sh diff --git a/src/service.c b/src/service.c index 127e0099..d31ec3e7 100644 --- a/src/service.c +++ b/src/service.c @@ -1654,7 +1654,8 @@ static void parse_script(svc_t *svc, char *type, char *script, int *tmo, char *b script, errstr); goto err; } - *tmo = (int)(sec * 1000); + if (tmo) + *tmo = (int)(sec * 1000); } else { path = script; if (tmo) diff --git a/test/Makefile.am b/test/Makefile.am index da6eb905..5ed12ef3 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -30,7 +30,7 @@ EXTRA_DIST += setup-sysroot.sh EXTRA_DIST += add-remove-dynamic-service.sh EXTRA_DIST += add-remove-dynamic-service-sub-config.sh EXTRA_DIST += bootstrap-crash.sh -EXTRA_DIST += cond-start-task.sh +EXTRA_DIST += script-timeout.sh EXTRA_DIST += crashing.sh EXTRA_DIST += dep-chain-reload.sh EXTRA_DIST += depserv.sh @@ -75,6 +75,7 @@ TESTS += add-remove-dynamic-service.sh TESTS += add-remove-dynamic-service-sub-config.sh TESTS += bootstrap-crash.sh TESTS += cond-start-task.sh +TESTS += script-timeout.sh TESTS += crashing.sh TESTS += dep-chain-reload.sh TESTS += depserv.sh diff --git a/test/script-timeout.sh b/test/script-timeout.sh new file mode 100755 index 00000000..64b6debc --- /dev/null +++ b/test/script-timeout.sh @@ -0,0 +1,36 @@ +#!/bin/sh +# A timeout on a stop: or reload: script must not take PID 1 with it. +# Those two hooks passed a NULL timeout pointer to parse_script(), +# which wrote through it whenever the script was prefixed with a +# valid number. Only a valid number reached the store, so +# 'stop:abc,/bin/true' was harmless while 'stop:5,/bin/true' was not. +set -eu + +TEST_DIR=$(dirname "$0") + +test_teardown() +{ + say "Running test teardown." + run "rm -f $FINIT_CONF" +} + +# shellcheck source=/dev/null +. "$TEST_DIR/lib/setup.sh" + +# shellcheck disable=SC2154 +assert_alive() +{ + assert "Finit survived $1" "$(kill -0 "$finit_pid" 2>/dev/null && echo yes)" = "yes" +} + +for hook in stop reload post pre; do + say "Timeout on a $hook: script" + run "echo 'service $hook:5,/bin/true service.sh -- Timeout test' > $FINIT_CONF" + run "initctl reload" || true + assert_alive "$hook:5,/bin/true" +done + +say 'The service still runs afterwards' +run "echo 'service stop:5,/bin/true service.sh -- Timeout test' > $FINIT_CONF" +run "initctl reload" +retry 'assert_num_children 1 service.sh'