| 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: <1bf27354248772fa7b8d15fdb305353f6e212966.camel@redhat.com>
Date: Thu, 13 Mar 2025 12:39:36 +0100
From: Gabriele Monaco <gmonaco@...hat.com>
To: Nam Cao <namcao@...utronix.de>
Cc: Steven Rostedt <rostedt@...dmis.org>, john.ogness@...utronix.de,
linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org, Petr
Mladek <pmladek@...e.com>, Sergey Senozhatsky <senozhatsky@...omium.org>
Subject: Re: [PATCH 02/10] rv: Let the reactors take care of buffers
On Thu, 2025-03-13 at 09:16 +0100, Nam Cao wrote:
> Hi Gabriele,
>
> On Wed, Mar 12, 2025 at 08:58:56AM +0100, Gabriele Monaco wrote:
> > On Tue, 2025-03-11 at 18:05 +0100, Nam Cao wrote:
> > > diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> > > index 510c88bfabd4..c55d45544a16 100644
> > > --- a/include/rv/da_monitor.h
> > > +++ b/include/rv/da_monitor.h
> > > @@ -16,58 +16,11 @@
> > > #include <linux/bug.h>
> > > #include <linux/sched.h>
> > >
> > > -#ifdef CONFIG_RV_REACTORS
> > > -
> > > -#define DECLARE_RV_REACTING_HELPERS(name,
> > > type) \
> > > -static char
> > > REACT_MSG_##name[1024];
> > > \
> > > -
> > >
> > > \
> > > -static inline char *format_react_msg_##name(type curr_state,
> > > type
> > > event) \
> > > -
> > > {
> > > \
> > > - snprintf(REACT_MSG_##name,
> > > 1024, \
> > > - "rv: monitor %s does not allow event %s on
> > > state
> > > %s\n", \
> > > -
> > > #name,
> > > \
> > > -
> > > model_get_event_name_##name(event),
> > > \
> > > -
> > > model_get_state_name_##name(curr_state));
> > > \
> > > - return
> > > REACT_MSG_##name;
> > > \
> > > -
> > > }
> > > \
> > > -
> > >
> > > \
> > > -static void cond_react_##name(char
> > > *msg) \
> > > -
> > > {
> > > \
> > > - if
> > > (rv_##name.react)
> > > \
> > > -
> > > rv_##name.react(msg);
> > > \
> > > -
> > > }
> > > \
> > > -
> > >
> > > \
> > > -static bool
> > > rv_reacting_on_##name(void)
> > > \
> > > -
> > > {
> > > \
> > > - return
> > > rv_reacting_on();
> > > \
> > > -}
> > > -
> > > -#else /* CONFIG_RV_REACTOR */
> > > -
> > > -#define DECLARE_RV_REACTING_HELPERS(name,
> > > type) \
> > > -static inline char *format_react_msg_##name(type curr_state,
> > > type
> > > event) \
> > > -
> > > {
> > > \
> > > - return
> > > NULL;
> > > \
> > > -
> > > }
> > > \
> > > -
> > >
> > > \
> > > -static void cond_react_##name(char
> > > *msg) \
> > > -
> > > {
> > > \
> > > -
> > > return;
> > > \
> > > -
> > > }
> > > \
> > > -
> > >
> > > \
> > > -static bool
> > > rv_reacting_on_##name(void)
> > > \
> > > -
> > > {
> > > \
> > > - return
> > > 0;
> > > \
> > > -}
> > > -#endif
> > > -
> >
> > I don't think you need to remove those helper functions, why not
> > just
> > having format_react_msg_ prepare the arguments for react?
>
> I'm not sure what you mean. Making format_react_msg_* a macro that is
> preprocessed into arguments? Then make cond_react_*() a variadic
> function,
> so that we can "pass" format_react_msg_* to it?
>
> Going that way would also need a vreact() variant, as cond_react_*()
> cannot
> pass on the variadic arguments to react().
>
> Instead, is it cleaner to do the below?
Hi Nam,
you're right, I got a bit confused, all I meant was to find a way not
to repeat the arguments for implicit and per-task monitors.
What you propose seems perfect to me.
Also for the sake of simplifying things, a bit like you started, we
could have the reacting_on() check inside cond_react and drop the per-
monitor function. I believe the initial idea was to have a reacting_on
toggle for each monitor but since it isn't the case, we don't really
need it.
Thanks,
Gabriele
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index 510c88bfabd4..e185ebf894a4 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -19,22 +19,14 @@
> #ifdef CONFIG_RV_REACTORS
>
> #define DECLARE_RV_REACTING_HELPERS(name,
> type) \
> -static char
> REACT_MSG_##name[1024]; \
> -
> \
> -static inline char *format_react_msg_##name(type curr_state, type
> event) \
> -
> { \
> - snprintf(REACT_MSG_##name,
> 1024, \
> - "rv: monitor %s does not allow event %s on state
> %s\n", \
> -
> #name, \
> -
> model_get_event_name_##name(event), \
> -
> model_get_state_name_##name(curr_state)); \
> - return
> REACT_MSG_##name; \
> -
> } \
> -
> \
> -static void cond_react_##name(char
> *msg) \
> +static void cond_react_##name(type curr_state, type
> event) \
> {
> \
> - if
> (rv_##name.react) \
> -
> rv_##name.react(msg); \
> + if
> (!rv_##name.react) \
> + return;
> \
> + rv_##name.react("rv: monitor %s does not allow event %s on
> state %s\n", \
> + #name,
> \
> + model_get_event_name_##name(event),
> \
> + model_get_state_name_##name(curr_state));
> \
> }
> \
>
> \
> static bool
> rv_reacting_on_##name(void) \
> @@ -45,12 +37,7 @@ static bool
> rv_reacting_on_##name(void) \
> #else /* CONFIG_RV_REACTOR */
>
> #define DECLARE_RV_REACTING_HELPERS(name,
> type) \
> -static inline char *format_react_msg_##name(type curr_state, type
> event) \
> -
> { \
> - return
> NULL; \
> -
> } \
> -
> \
> -static void cond_react_##name(char
> *msg) \
> +static void cond_react_##name(type curr_state, type
> event) \
> {
> \
> return;
> \
> }
> \
> @@ -171,7 +158,7 @@ da_event_##name(struct da_monitor *da_mon, enum
> events_##name event) \
> }
> \
>
> \
> if
> (rv_reacting_on_##name()) \
> -
> cond_react_##name(format_react_msg_##name(curr_state, event)); \
> + cond_react_##name(curr_state,
> event); \
>
> \
> trace_error_##name(model_get_state_name_##name(curr_state),
> \
>
> model_get_event_name_##name(event)); \
> @@ -203,7 +190,7 @@ static inline bool da_event_##name(struct
> da_monitor *da_mon, struct task_struct
> }
> \
>
> \
> if
> (rv_reacting_on_##name()) \
> -
> cond_react_##name(format_react_msg_##name(curr_state, event)); \
> + cond_react_##name(curr_state,
> event); \
>
> \
> trace_error_##name(tsk-
> >pid, \
>
> model_get_state_name_##name(curr_state), \
>
> Best regards,
> Nam
>
Powered by blists - more mailing lists