On Sun, 2026-05-17 at 17:40 +0800, Wen Yang wrote:
> ha_cancel_timer_sync() is the right choice for task exit; the
> ha_mon_initializing guard correctly handles the init-window race.
>
> One issue: after ha_monitor_disable_hook(), an in-flight
> ha_handle_sched_process_exit() handler may still be executing. It
> reads task_mon_slot via da_get_monitor() (&p->rv[task_mon_slot]);
> da_monitor_sync_hook() = synchronize_rcu() cannot drain it because
> tracepoint handlers run outside any RCU read-side section. If
> rv_put_task_monitor_slot() writes RV_PER_TASK_MONITOR_INIT to
> task_mon_slot first, the handler dereferences an OOB index.
>
> This is the same race Patch 5 closes for PER_OBJ with
> tracepoint_synchronize_unregister(); the PER_TASK da_monitor_destroy()
> needs the same call (and so does every other PER_TASK monitor, not only
> the new exit handler).
>
> Could you add tracepoint_synchronize_unregister() to the PER_TASK
> da_monitor_destroy() ? Alternatively, we can carry the fix on top of
> your series.
Yeah you're right, that's the neatest way to solve it.
Indeed, any in-flight handler would do da_get_monitor() not only the newly added
exit hook, so we're going to need tracepoint_synchronize_unregister() before
touching task_mon_slot in any per-task monitor (not only HA, also DA and even
LTL).
Feel free to send your patch and I'll apply it to this series, including also
ltl_monitor_destroy().
Thanks,
Gabriele
>
> --
> Best wishes,
> Wen
>
>
> On 5/12/26 22:02, Gabriele Monaco wrote:
> > 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 <[email protected]>
> > ---
> > include/rv/ha_monitor.h | 44 +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 44 insertions(+)
> >
> > diff --git a/include/rv/ha_monitor.h b/include/rv/ha_monitor.h
> > index 11ae85bad492..1bdf866e9c63 100644
> > --- a/include/rv/ha_monitor.h
> > +++ b/include/rv/ha_monitor.h
> > @@ -28,6 +28,7 @@ static inline void ha_monitor_init_env(struct da_monitor
> > *da_mon);
> > static inline void ha_monitor_reset_env(struct da_monitor *da_mon);
> > static inline void ha_setup_timer(struct ha_monitor *ha_mon);
> > static inline bool ha_cancel_timer(struct ha_monitor *ha_mon);
> > +static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon);
> > static bool ha_monitor_handle_constraint(struct da_monitor *da_mon,
> > enum states curr_state,
> > enum events event,
> > @@ -38,6 +39,26 @@ static bool ha_monitor_handle_constraint(struct
> > da_monitor *da_mon,
> > #define da_monitor_reset_hook ha_monitor_reset_env
> > #define da_monitor_sync_hook() synchronize_rcu()
> >
> > +#if !defined(HA_SKIP_AUTO_CLEANUP) && RV_MON_TYPE == RV_MON_PER_TASK
> > +/*
> > + * Automatic cleanup handlers for per-task HA monitors, only skip if you
> > know
> > + * what you are doing (e.g. you want to implement cleanup manually in
> > another
> > + * handler doing more things).
> > + */
> > +static void ha_handle_sched_process_exit(void *data, struct task_struct *p,
> > + bool group_dead);
> > +
> > +#define
> > ha_monitor_enable_hook() \
> > + rv_attach_trace_probe(__stringify(MONITOR_NAME),
> > sched_process_exit, \
> > + ha_handle_sched_process_exit)
> > +#define
> > ha_monitor_disable_hook() \
> > + rv_detach_trace_probe(__stringify(MONITOR_NAME),
> > sched_process_exit, \
> > + ha_handle_sched_process_exit)
> > +#else
> > +#define ha_monitor_enable_hook()
> > +#define ha_monitor_disable_hook()
> > +#endif
> > +
> > #include <rv/da_monitor.h>
> > #include <linux/seq_buf.h>
> >
> > @@ -124,12 +145,14 @@ static int ha_monitor_init(void)
> >
> > ha_mon_initializing = true;
> > ret = da_monitor_init();
> > + ha_monitor_enable_hook();
> > ha_mon_initializing = false;
> > return ret;
> > }
> >
> > static void ha_monitor_destroy(void)
> > {
> > + ha_monitor_disable_hook();
> > da_monitor_destroy();
> > }
> >
> > @@ -230,6 +253,18 @@ static inline void ha_trace_error_env(struct ha_monitor
> > *ha_mon,
> > {
> > CONCATENATE(trace_error_env_, MONITOR_NAME)(id, curr_state, event,
> > env);
> > }
> > +
> > +#if !defined(HA_SKIP_AUTO_CLEANUP) && RV_MON_TYPE == RV_MON_PER_TASK
> > +static void ha_handle_sched_process_exit(void *data, struct task_struct *p,
> > + bool group_dead)
> > +{
> > + struct da_monitor *da_mon = da_get_monitor(p);
> > +
> > + if (likely(!ha_monitor_uninitialized(da_mon)))
> > + ha_cancel_timer_sync(to_ha_monitor(da_mon));
> > +}
> > +#endif
> > +
> > #endif /* RV_MON_TYPE */
> >
> > /*
> > @@ -455,6 +490,10 @@ static inline bool ha_cancel_timer(struct ha_monitor
> > *ha_mon)
> > {
> > return timer_delete(&ha_mon->timer);
> > }
> > +static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon)
> > +{
> > + timer_delete_sync(&ha_mon->timer);
> > +}
> > #elif HA_TIMER_TYPE == HA_TIMER_HRTIMER
> > /*
> > * Helper functions to handle the monitor timer.
> > @@ -506,6 +545,10 @@ static inline bool ha_cancel_timer(struct ha_monitor
> > *ha_mon)
> > {
> > return hrtimer_try_to_cancel(&ha_mon->hrtimer) == 1;
> > }
> > +static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon)
> > +{
> > + hrtimer_cancel(&ha_mon->hrtimer);
> > +}
> > #else /* HA_TIMER_NONE */
> > /*
> > * Start function is intentionally not defined, monitors using timers must
> > @@ -516,6 +559,7 @@ static inline bool ha_cancel_timer(struct ha_monitor
> > *ha_mon)
> > {
> > return false;
> > }
> > +static inline void ha_cancel_timer_sync(struct ha_monitor *ha_mon) { }
> > #endif
> >
> > #endif
