From 52c64d24ce9cf255b9c348e5db118a35c9407d75 Mon Sep 17 00:00:00 2001 From: Tim Nordell Date: Wed, 15 Oct 2025 15:43:59 -0500 Subject: [PATCH 1/2] service instance: Fix overwriting of watchdog linked list members The instance_config_move(...) function copies the settings from a newly constructed instance from a "ubus call service set {...}" call into a prior instance. The `in->watchdog.timeout` is of the type `struct uloop_timeout` which contains a `struct list` inside of it. This list is a linked list structure, so when the uloop_timeout is already in the linked list, this overwrites this particular location with a NULL pointer for the next/prev entries. This causes procd to crash. This crash occurs every time "ubus call service set {...}" is called (e.g. via /etc/init.d/SERVICE start) when a daemon is using the "watchdog" feature has already been started. Fixes: 28be011 ("instance: make sure values are not inherited from previous runs") Signed-off-by: Tim Nordell --- service/instance.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/instance.c b/service/instance.c index b611c0f..fcffd3d 100644 --- a/service/instance.c +++ b/service/instance.c @@ -1519,7 +1519,7 @@ instance_config_move(struct service_instance *in, struct service_instance *in_sr in->term_timeout = in_src->term_timeout; in->watchdog.mode = in_src->watchdog.mode; in->watchdog.freq = in_src->watchdog.freq; - in->watchdog.timeout = in_src->watchdog.timeout; + // Note: in->watchdog.timeout is in a linked list; do not copy in->name = in_src->name; in->nice = in_src->nice; in->trace = in_src->trace; From afa4391d9cbb0706bbae9c8345436fa6d99c3013 Mon Sep 17 00:00:00 2001 From: Tim Nordell Date: Wed, 15 Oct 2025 15:52:04 -0500 Subject: [PATCH 2/2] service instance: Improve handling of watchdog config changes When the ubus call "ubus call service watchdog" is used, it's expected that the service might change its watchdog values during runtime when it pets the watchdog. The instance restarting behavior was setup though to restart the instance if these changed. This causes any service which adjusts its watchdog to be restarted via "/etc/init.d/SERVICENAME start" which is probably not what was intended. Introduce a new variable, self_managed, if the service adjusted these values at runtime so that a "start" doesn't cause the entire service to restart. Signed-off-by: Tim Nordell --- service/instance.c | 12 +++++++----- service/instance.h | 1 + service/service.c | 5 +++++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/service/instance.c b/service/instance.c index fcffd3d..2b8bba0 100644 --- a/service/instance.c +++ b/service/instance.c @@ -1040,10 +1040,10 @@ instance_config_changed(struct service_instance *in, struct service_instance *in if (in->jail.flags != in_new->jail.flags) return true; - if (in->watchdog.mode != in_new->watchdog.mode) + if (!in->watchdog.self_managed && in->watchdog.mode != in_new->watchdog.mode) return true; - if (in->watchdog.freq != in_new->watchdog.freq) + if (!in->watchdog.self_managed && in->watchdog.freq != in_new->watchdog.freq) return true; return false; @@ -1517,9 +1517,11 @@ instance_config_move(struct service_instance *in, struct service_instance *in_sr in->respawn_timeout = in_src->respawn_timeout; in->reload_signal = in_src->reload_signal; in->term_timeout = in_src->term_timeout; - in->watchdog.mode = in_src->watchdog.mode; - in->watchdog.freq = in_src->watchdog.freq; - // Note: in->watchdog.timeout is in a linked list; do not copy + if (!in->watchdog.self_managed) { + // Note: in->watchdog.timeout is in a linked list; do not copy + in->watchdog.mode = in_src->watchdog.mode; + in->watchdog.freq = in_src->watchdog.freq; + } in->name = in_src->name; in->nice = in_src->nice; in->trace = in_src->trace; diff --git a/service/instance.h b/service/instance.h index 32fae19..d5a10e2 100644 --- a/service/instance.h +++ b/service/instance.h @@ -55,6 +55,7 @@ typedef enum instance_watchdog { } instance_watchdog_mode_t; struct watchdog { + bool self_managed; instance_watchdog_mode_t mode; uint32_t freq; struct uloop_timeout timeout; diff --git a/service/service.c b/service/service.c index 831f075..7f1f408 100644 --- a/service/service.c +++ b/service/service.c @@ -1059,6 +1059,11 @@ service_handle_watchdog(struct ubus_context *ctx, struct ubus_object *obj, ubus_send_reply(ctx, req, b.head); + // If the service adjusts the mode or timeout, mark it as self managed + // so that it doesn't get restarted with a new /etc/init.d/SERVICE start + if (tb[SERVICE_WATCHDOG_MODE] || tb[SERVICE_WATCHDOG_TIMEOUT]) + in->watchdog.self_managed = true; + return UBUS_STATUS_OK; }