Re: [PATCH v3 07/13] rv: Add automatic cleanup handlers for per-task HA monitors
Gabriele Monaco writes:
> Hybrid automata monitors may start timers, depending on the model, these
> may remain active on an exiting task and cause false positives or even
> access freed memory.
>
> Add an enable/disable hook in the HA code, currently only populated by
> the per-task handler for registration and deregistration.
> This hooks to the sched_process_exit event and ensures the timer is
> stopped for every exiting task. The handler is enabled automatically but
> may be disabled, for instance if the monitor uses the event for another
> purpose (but should still manually ensure timers are stopped).
>
> Fixes: f5587d1b6ec9 ("rv: Add Hybrid Automata monitor type")
> Signed-off-by: Gabriele Monaco
Reviewed-by: Nam Cao
Re: [PATCH v3 07/13] rv: Add automatic cleanup handlers for per-task HA monitors
On Mon, 2026-06-01 at 09:39 +0200, Nam Cao wrote:
> Gabriele Monaco writes:
> > @@ -123,12 +144,15 @@ static int ha_monitor_init(void)
> >
> > ha_mon_initializing = true;
> > ret = da_monitor_init();
> > + if (ret == 0)
> > + ha_monitor_enable_hook();
> > ha_mon_initializing = false;
> > return ret;
> > }
>
> What if between da_monitor_init() and ha_monitor_enable_hook(), a
> task exits while a timer is still active, and then the timer callback
> is invoked?
We are initialising, timers shouldn't be active, that sits right before
setting up other hooks, and the exit hooks this way is just the first
of them.
By the way, in this case, we likely have a valid reset scenario on an
invalid (uninitialised) timer. This is also what checking the
monitoring flag guards against.
In short, in that handler we really should reset, but need to know
whether we ever initialised in the first place.
> Extremely rare, but I think it can be fixed easily by reordering the
> two functions.
>
> > static void ha_monitor_destroy(void)
> > {
> > + ha_monitor_disable_hook();
> > da_monitor_destroy();
> > }
>
> Same here, there is small window between the two function calls.
Likewise here, we removed all hooks, then da_monitor_destroy() is going
to sync with them and clean everything up. Swapping them will expose
more races because we dropped the slot by then.
Am I missing something?
Thanks,
Gabriele
Re: [PATCH v3 07/13] rv: Add automatic cleanup handlers for per-task HA monitors
Gabriele Monaco writes:
> @@ -123,12 +144,15 @@ static int ha_monitor_init(void)
>
> ha_mon_initializing = true;
> ret = da_monitor_init();
> + if (ret == 0)
> + ha_monitor_enable_hook();
> ha_mon_initializing = false;
> return ret;
> }
What if between da_monitor_init() and ha_monitor_enable_hook(), a task
exits while a timer is still active, and then the timer callback is invoked?
Extremely rare, but I think it can be fixed easily by reordering the two
functions.
> static void ha_monitor_destroy(void)
> {
> + ha_monitor_disable_hook();
> da_monitor_destroy();
> }
Same here, there is small window between the two function calls.
Nam
