| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <YuFZ2scVb658mhoq@geo.homenetwork>
Date: Wed, 27 Jul 2022 23:29:30 +0800
From: Tao Zhou <tao.zhou@...ux.dev>
To: Daniel Bristot de Oliveira <bristot@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Wim Van Sebroeck <wim@...ux-watchdog.org>,
Guenter Roeck <linux@...ck-us.net>,
Jonathan Corbet <corbet@....net>,
Ingo Molnar <mingo@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Marco Elver <elver@...gle.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
"Paul E. McKenney" <paulmck@...nel.org>,
Shuah Khan <skhan@...uxfoundation.org>,
Gabriele Paoloni <gpaoloni@...hat.com>,
Juri Lelli <juri.lelli@...hat.com>,
Clark Williams <williams@...hat.com>,
Randy Dunlap <rdunlap@...radead.org>,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-trace-devel@...r.kernel.org, Tao Zhou <tao.zhou@...ux.dev>
Subject: Re: [PATCH V7 04/16] rv/include: Add deterministic automata monitor
definition via C macros
On Mon, Jul 25, 2022 at 10:11:16PM +0200, Daniel Bristot de Oliveira wrote:
> +/*
> + * Event handler for per_task monitors.
> + */
> +#define DECLARE_DA_MON_MODEL_HANDLER_PER_TASK(name, type) \
> + \
> +static inline type da_event_##name(struct da_monitor *da_mon, struct task_struct *tsk, \
Not sure here the type `type` why not the bool. The return value is ture/false.
I checked the caller of this function, the return value is stored on `int`.
> + enum events_##name event) \
> +{ \
> + type curr_state = da_monitor_curr_state_##name(da_mon); \
> + type next_state = model_get_next_state_##name(curr_state, event); \
> + \
> + if (next_state != INVALID_STATE) { \
> + da_monitor_set_state_##name(da_mon, next_state); \
> + \
> + trace_event_##name(tsk->pid, \
> + model_get_state_name_##name(curr_state), \
> + model_get_event_name_##name(event), \
> + model_get_state_name_##name(next_state), \
> + model_is_final_state_##name(next_state)); \
> + \
> + return true; \
> + } \
> + \
> + if (rv_reacting_on_##name()) \
> + cond_react_##name(format_react_msg_##name(curr_state, event)); \
> + \
> + trace_error_##name(tsk->pid, \
> + model_get_state_name_##name(curr_state), \
> + model_get_event_name_##name(event)); \
> + \
> + return false; \
> +}
[snip]
> +/*
> + * Handle event for implicit monitor: da_get_monitor_##name() will figure out
> + * the monitor.
> + */
> +#define DECLARE_DA_MON_MONITOR_HANDLER_IMPLICIT(name, type) \
> + \
> +static inline void __da_handle_event_##name(struct da_monitor *da_mon, \
> + enum events_##name event) \
> +{ \
> + int retval; \
> + \
> + retval = da_monitor_handling_event_##name(da_mon); \
> + if (!retval) \
> + return; \
I checked the callers of __da_handle_event_##name():
da_handle_event_##name() for all cases need the above check.
da_handle_start_event_##name() for all cases may not need this check.
(this function checked the enable first and the da_monitoring later and if
it is not monitoring it will start monitoring and return, the later event
handler will not be called. Otherwise enable is enabled, da_monitoring is
monitoring)
da_handle_start_run_event_##name() for implicit case may not need this check.
(almost the same with the above, the difference is if da-monitor is not
monitoring, it will start monitoring and not return and do the event handler,
here enable is enabled and da_monitoring is monitoring, if I am not wrong)
So after another(v7) looking at this patch, I realized that this check can
be omited in two cases(all three cases). Just in fuction da_handle_event_##name()
we need to do da_monitor_handling_event_##name().
So I'd write like this:
static inline void __da_handle_event_##name(struct da_monitor *da_mon, \
enum events_##name event) \
{ \
int retval; \
\
retval = da_event_##name(da_mon, event); \
if (!retval) \
da_monitor_reset_##name(da_mon); \
} \
static inline void da_handle_event_##name(enum events_##name event) \
{ \
struct da_monitor *da_mon = da_get_monitor_##name(); \
int retval; \
\
retval = da_monitor_handling_event_##name(da_mon); \
if (!retval) \
return; \
\
__da_handle_event_##name(da_mon, event); \
} \
> + \
> + retval = da_event_##name(da_mon, event); \
> + if (!retval) \
> + da_monitor_reset_##name(da_mon); \
> +} \
> + \
> +/* \
> + * da_handle_event_##name - handle an event \
> + */ \
> +static inline void da_handle_event_##name(enum events_##name event) \
> +{ \
> + struct da_monitor *da_mon = da_get_monitor_##name(); \
> + __da_handle_event_##name(da_mon, event); \
> +} \
> + \
> +/* \
> + * da_handle_start_event_##name - start monitoring or handle event \
> + * \
> + * This function is used notify the monitor that the system is returning \
/used/used to/ :-) My wording is not well, sorry for not convenience, Thanks,
> + * to the initial state, so the monitor can start monitoring in the next event. \
> + * Thus: \
> + * \
> + * If the monitor already started, handle the event. \
> + * If the monitor did not start yet, start the monitor but skip the event. \
> + */ \
> +static inline bool da_handle_start_event_##name(enum events_##name event) \
> +{ \
> + struct da_monitor *da_mon; \
> + \
> + if (!da_monitor_enabled_##name()) \
> + return 0; \
> + \
> + da_mon = da_get_monitor_##name(); \
> + \
> + if (unlikely(!da_monitoring_##name(da_mon))) { \
> + da_monitor_start_##name(da_mon); \
> + return 0; \
> + } \
> + \
> + __da_handle_event_##name(da_mon, event); \
> + \
> + return 1; \
> +} \
> + \
> +/* \
> + * da_handle_start_run_event_##name - start monitoring and handle event \
> + * \
> + * This function is used notify the monitor that the system is in the \
> + * initial state, so the monitor can start monitoring and handling event. \
> + */ \
> +static inline bool da_handle_start_run_event_##name(enum events_##name event) \
> +{ \
> + struct da_monitor *da_mon; \
> + \
> + if (!da_monitor_enabled_##name()) \
> + return 0; \
> + \
> + da_mon = da_get_monitor_##name(); \
> + \
> + if (unlikely(!da_monitoring_##name(da_mon))) \
> + da_monitor_start_##name(da_mon); \
> + \
> + __da_handle_event_##name(da_mon, event); \
> + \
> + return 1; \
> +}
> +
> +/*
> + * Handle event for per task.
> + */
> +#define DECLARE_DA_MON_MONITOR_HANDLER_PER_TASK(name, type) \
> + \
> +static inline void \
> +__da_handle_event_##name(struct da_monitor *da_mon, struct task_struct *tsk, \
> + enum events_##name event) \
> +{ \
> + int retval; \
> + \
> + retval = da_monitor_handling_event_##name(da_mon); \
> + if (!retval) \
> + return; \
> + \
> + retval = da_event_##name(da_mon, tsk, event); \
> + if (!retval) \
> + da_monitor_reset_##name(da_mon); \
> +} \
> + \
> +/* \
> + * da_handle_event_##name - handle an event \
> + */ \
> +static inline void \
> +da_handle_event_##name(struct task_struct *tsk, enum events_##name event) \
> +{ \
> + struct da_monitor *da_mon = da_get_monitor_##name(tsk); \
> + __da_handle_event_##name(da_mon, tsk, event); \
> +} \
> + \
> +/* \
> + * da_handle_start_event_##name - start monitoring or handle event \
> + * \
> + * This function is used notify the monitor that the system is returning \
> + * to the initial state, so the monitor can start monitoring in the next event. \
> + * Thus: \
> + * \
> + * If the monitor already started, handle the event. \
> + * If the monitor did not start yet, start the monitor but skip the event. \
> + */ \
> +static inline bool \
> +da_handle_start_event_##name(struct task_struct *tsk, enum events_##name event) \
> +{ \
> + struct da_monitor *da_mon; \
> + \
> + if (!da_monitor_enabled_##name()) \
> + return 0; \
> + \
> + da_mon = da_get_monitor_##name(tsk); \
> + \
> + if (unlikely(!da_monitoring_##name(da_mon))) { \
> + da_monitor_start_##name(da_mon); \
> + return 0; \
> + } \
> + \
> + __da_handle_event_##name(da_mon, tsk, event); \
> + \
> + return 1; \
> +}
Powered by blists - more mailing lists