From: Gabriele Monaco <gmonaco@redhat.com>
To: Nam Cao <namcao@linutronix.de>,
Steven Rostedt <rostedt@goodmis.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
linux-trace-kernel@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org
Subject: Re: [PATCH] rv: Ensure containers are registered first
Date: Fri, 18 Jul 2025 11:35:27 +0200 [thread overview]
Message-ID: <57b1e83ede5ef8f5b2d661b3326374d82da0bd1a.camel@redhat.com> (raw)
In-Reply-To: <20250718091850.2057864-1-namcao@linutronix.de>
On Fri, 2025-07-18 at 11:18 +0200, Nam Cao wrote:
> If rv_register_monitor() is called with a non-NULL parent pointer
> (i.e. by
> monitors inside a container), it is expected that the parent (a.k.a
> container) is already registered.
>
> The containers seem to always be registered first. I suspect because
> of the order in Makefile. But nothing guarantees this.
Yes, the linking order does guarantee this.
But I guess it doesn't hurt enforcing it.
Anyway do you have a good reason for using device_initcall/__exitcall
instead of module_init/module_exit?
Keeping those might save a bit of ifdeffery if (when) we decide to
fully support RV monitors as kernel modules.
Thanks,
Gabriele
>
> If this registering order is changed, "strange" things happen. For
> example,
> if the container is registered last, rv_is_container_monitor()
> incorrectly
> says this is NOT a container. .enable() is then called, which is NULL
> for
> container, thus we have a NULL pointer deref crash.
>
> Guarantee that containers are registered first.
>
> Fixes: cb85c660fcd4 ("rv: Add option for nested monitors and include
> sched")
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> Cc: stable@vger.kernel.org
> ---
> include/linux/rv.h | 5 +++++
> kernel/trace/rv/monitors/pagefault/pagefault.c | 4 ++--
> kernel/trace/rv/monitors/rtapp/rtapp.c | 4 ++--
> kernel/trace/rv/monitors/sched/sched.c | 4 ++--
> kernel/trace/rv/monitors/sco/sco.c | 4 ++--
> kernel/trace/rv/monitors/scpd/scpd.c | 4 ++--
> kernel/trace/rv/monitors/sleep/sleep.c | 4 ++--
> kernel/trace/rv/monitors/sncid/sncid.c | 4 ++--
> kernel/trace/rv/monitors/snep/snep.c | 4 ++--
> kernel/trace/rv/monitors/snroc/snroc.c | 4 ++--
> kernel/trace/rv/monitors/tss/tss.c | 4 ++--
> kernel/trace/rv/monitors/wip/wip.c | 4 ++--
> kernel/trace/rv/monitors/wwnr/wwnr.c | 4 ++--
> tools/verification/rvgen/rvgen/templates/container/main.c | 4 ++--
> tools/verification/rvgen/rvgen/templates/dot2k/main.c | 4 ++--
> tools/verification/rvgen/rvgen/templates/ltl2k/main.c | 4 ++--
> 16 files changed, 35 insertions(+), 30 deletions(-)
>
> diff --git a/include/linux/rv.h b/include/linux/rv.h
> index 97baf58d88b2..094c9f62389c 100644
> --- a/include/linux/rv.h
> +++ b/include/linux/rv.h
> @@ -119,5 +119,10 @@ static inline bool rv_reacting_on(void)
> }
> #endif /* CONFIG_RV_REACTORS */
>
> +#define rv_container_init device_initcall
> +#define rv_container_exit __exitcall
> +#define rv_monitor_init late_initcall
> +#define rv_monitor_exit __exitcall
> +
> #endif /* CONFIG_RV */
> #endif /* _LINUX_RV_H */
> diff --git a/kernel/trace/rv/monitors/pagefault/pagefault.c
> b/kernel/trace/rv/monitors/pagefault/pagefault.c
> index 9fe6123b2200..2b226d27ddff 100644
> --- a/kernel/trace/rv/monitors/pagefault/pagefault.c
> +++ b/kernel/trace/rv/monitors/pagefault/pagefault.c
> @@ -80,8 +80,8 @@ static void __exit unregister_pagefault(void)
> rv_unregister_monitor(&rv_pagefault);
> }
>
> -module_init(register_pagefault);
> -module_exit(unregister_pagefault);
> +rv_monitor_init(register_pagefault);
> +rv_monitor_exit(unregister_pagefault);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Nam Cao <namcao@linutronix.de>");
> diff --git a/kernel/trace/rv/monitors/rtapp/rtapp.c
> b/kernel/trace/rv/monitors/rtapp/rtapp.c
> index fd75fc927d65..b078327e71bf 100644
> --- a/kernel/trace/rv/monitors/rtapp/rtapp.c
> +++ b/kernel/trace/rv/monitors/rtapp/rtapp.c
> @@ -25,8 +25,8 @@ static void __exit unregister_rtapp(void)
> rv_unregister_monitor(&rv_rtapp);
> }
>
> -module_init(register_rtapp);
> -module_exit(unregister_rtapp);
> +rv_container_init(register_rtapp);
> +rv_container_exit(unregister_rtapp);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Nam Cao <namcao@linutronix.de>");
> diff --git a/kernel/trace/rv/monitors/sched/sched.c
> b/kernel/trace/rv/monitors/sched/sched.c
> index 905e03c3c934..e89e193bd8e0 100644
> --- a/kernel/trace/rv/monitors/sched/sched.c
> +++ b/kernel/trace/rv/monitors/sched/sched.c
> @@ -30,8 +30,8 @@ static void __exit unregister_sched(void)
> rv_unregister_monitor(&rv_sched);
> }
>
> -module_init(register_sched);
> -module_exit(unregister_sched);
> +rv_container_init(register_sched);
> +rv_container_exit(unregister_sched);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
> diff --git a/kernel/trace/rv/monitors/sco/sco.c
> b/kernel/trace/rv/monitors/sco/sco.c
> index 4cff59220bfc..b96e09e64a2f 100644
> --- a/kernel/trace/rv/monitors/sco/sco.c
> +++ b/kernel/trace/rv/monitors/sco/sco.c
> @@ -80,8 +80,8 @@ static void __exit unregister_sco(void)
> rv_unregister_monitor(&rv_sco);
> }
>
> -module_init(register_sco);
> -module_exit(unregister_sco);
> +rv_monitor_init(register_sco);
> +rv_monitor_exit(unregister_sco);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
> diff --git a/kernel/trace/rv/monitors/scpd/scpd.c
> b/kernel/trace/rv/monitors/scpd/scpd.c
> index cbdd6a5f8d7f..a4c8e78fa768 100644
> --- a/kernel/trace/rv/monitors/scpd/scpd.c
> +++ b/kernel/trace/rv/monitors/scpd/scpd.c
> @@ -88,8 +88,8 @@ static void __exit unregister_scpd(void)
> rv_unregister_monitor(&rv_scpd);
> }
>
> -module_init(register_scpd);
> -module_exit(unregister_scpd);
> +rv_monitor_init(register_scpd);
> +rv_monitor_exit(unregister_scpd);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
> diff --git a/kernel/trace/rv/monitors/sleep/sleep.c
> b/kernel/trace/rv/monitors/sleep/sleep.c
> index eea447b06907..6980f8de725d 100644
> --- a/kernel/trace/rv/monitors/sleep/sleep.c
> +++ b/kernel/trace/rv/monitors/sleep/sleep.c
> @@ -229,8 +229,8 @@ static void __exit unregister_sleep(void)
> rv_unregister_monitor(&rv_sleep);
> }
>
> -module_init(register_sleep);
> -module_exit(unregister_sleep);
> +rv_monitor_init(register_sleep);
> +rv_monitor_exit(unregister_sleep);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Nam Cao <namcao@linutronix.de>");
> diff --git a/kernel/trace/rv/monitors/sncid/sncid.c
> b/kernel/trace/rv/monitors/sncid/sncid.c
> index f5037cd6214c..97a126c6083a 100644
> --- a/kernel/trace/rv/monitors/sncid/sncid.c
> +++ b/kernel/trace/rv/monitors/sncid/sncid.c
> @@ -88,8 +88,8 @@ static void __exit unregister_sncid(void)
> rv_unregister_monitor(&rv_sncid);
> }
>
> -module_init(register_sncid);
> -module_exit(unregister_sncid);
> +rv_monitor_init(register_sncid);
> +rv_monitor_exit(unregister_sncid);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
> diff --git a/kernel/trace/rv/monitors/snep/snep.c
> b/kernel/trace/rv/monitors/snep/snep.c
> index 0076ba6d7ea4..376a856ebffa 100644
> --- a/kernel/trace/rv/monitors/snep/snep.c
> +++ b/kernel/trace/rv/monitors/snep/snep.c
> @@ -88,8 +88,8 @@ static void __exit unregister_snep(void)
> rv_unregister_monitor(&rv_snep);
> }
>
> -module_init(register_snep);
> -module_exit(unregister_snep);
> +rv_monitor_init(register_snep);
> +rv_monitor_exit(unregister_snep);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
> diff --git a/kernel/trace/rv/monitors/snroc/snroc.c
> b/kernel/trace/rv/monitors/snroc/snroc.c
> index bb1f60d55296..e4439605b4b6 100644
> --- a/kernel/trace/rv/monitors/snroc/snroc.c
> +++ b/kernel/trace/rv/monitors/snroc/snroc.c
> @@ -77,8 +77,8 @@ static void __exit unregister_snroc(void)
> rv_unregister_monitor(&rv_snroc);
> }
>
> -module_init(register_snroc);
> -module_exit(unregister_snroc);
> +rv_monitor_init(register_snroc);
> +rv_monitor_exit(unregister_snroc);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
> diff --git a/kernel/trace/rv/monitors/tss/tss.c
> b/kernel/trace/rv/monitors/tss/tss.c
> index 542787e6524f..8f960c9fe0ff 100644
> --- a/kernel/trace/rv/monitors/tss/tss.c
> +++ b/kernel/trace/rv/monitors/tss/tss.c
> @@ -83,8 +83,8 @@ static void __exit unregister_tss(void)
> rv_unregister_monitor(&rv_tss);
> }
>
> -module_init(register_tss);
> -module_exit(unregister_tss);
> +rv_monitor_init(register_tss);
> +rv_monitor_exit(unregister_tss);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Gabriele Monaco <gmonaco@redhat.com>");
> diff --git a/kernel/trace/rv/monitors/wip/wip.c
> b/kernel/trace/rv/monitors/wip/wip.c
> index ed758fec8608..5c39c6074bd3 100644
> --- a/kernel/trace/rv/monitors/wip/wip.c
> +++ b/kernel/trace/rv/monitors/wip/wip.c
> @@ -80,8 +80,8 @@ static void __exit unregister_wip(void)
> rv_unregister_monitor(&rv_wip);
> }
>
> -module_init(register_wip);
> -module_exit(unregister_wip);
> +rv_monitor_init(register_wip);
> +rv_monitor_exit(unregister_wip);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Daniel Bristot de Oliveira <bristot@kernel.org>");
> diff --git a/kernel/trace/rv/monitors/wwnr/wwnr.c
> b/kernel/trace/rv/monitors/wwnr/wwnr.c
> index 172f31c4b0f3..ec671546f571 100644
> --- a/kernel/trace/rv/monitors/wwnr/wwnr.c
> +++ b/kernel/trace/rv/monitors/wwnr/wwnr.c
> @@ -79,8 +79,8 @@ static void __exit unregister_wwnr(void)
> rv_unregister_monitor(&rv_wwnr);
> }
>
> -module_init(register_wwnr);
> -module_exit(unregister_wwnr);
> +rv_monitor_init(register_wwnr);
> +rv_monitor_exit(unregister_wwnr);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Daniel Bristot de Oliveira <bristot@kernel.org>");
> diff --git
> a/tools/verification/rvgen/rvgen/templates/container/main.c
> b/tools/verification/rvgen/rvgen/templates/container/main.c
> index 89fc17cf8958..5820c9705d0f 100644
> --- a/tools/verification/rvgen/rvgen/templates/container/main.c
> +++ b/tools/verification/rvgen/rvgen/templates/container/main.c
> @@ -30,8 +30,8 @@ static void __exit unregister_%%MODEL_NAME%%(void)
> rv_unregister_monitor(&rv_%%MODEL_NAME%%);
> }
>
> -module_init(register_%%MODEL_NAME%%);
> -module_exit(unregister_%%MODEL_NAME%%);
> +rv_container_init(register_%%MODEL_NAME%%);
> +rv_container_exit(unregister_%%MODEL_NAME%%);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("dot2k: auto-generated");
> diff --git a/tools/verification/rvgen/rvgen/templates/dot2k/main.c
> b/tools/verification/rvgen/rvgen/templates/dot2k/main.c
> index 83044a20c89a..d6bd248aba9c 100644
> --- a/tools/verification/rvgen/rvgen/templates/dot2k/main.c
> +++ b/tools/verification/rvgen/rvgen/templates/dot2k/main.c
> @@ -83,8 +83,8 @@ static void __exit unregister_%%MODEL_NAME%%(void)
> rv_unregister_monitor(&rv_%%MODEL_NAME%%);
> }
>
> -module_init(register_%%MODEL_NAME%%);
> -module_exit(unregister_%%MODEL_NAME%%);
> +rv_monitor_init(register_%%MODEL_NAME%%);
> +rv_monitor_exit(unregister_%%MODEL_NAME%%);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("dot2k: auto-generated");
> diff --git a/tools/verification/rvgen/rvgen/templates/ltl2k/main.c
> b/tools/verification/rvgen/rvgen/templates/ltl2k/main.c
> index f85d076fbf78..2069a7a0f1ae 100644
> --- a/tools/verification/rvgen/rvgen/templates/ltl2k/main.c
> +++ b/tools/verification/rvgen/rvgen/templates/ltl2k/main.c
> @@ -94,8 +94,8 @@ static void __exit unregister_%%MODEL_NAME%%(void)
> rv_unregister_monitor(&rv_%%MODEL_NAME%%);
> }
>
> -module_init(register_%%MODEL_NAME%%);
> -module_exit(unregister_%%MODEL_NAME%%);
> +rv_monitor_init(register_%%MODEL_NAME%%);
> +rv_monitor_exit(unregister_%%MODEL_NAME%%);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR(/* TODO */);
next prev parent reply other threads:[~2025-07-18 9:35 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-18 9:18 [PATCH] rv: Ensure containers are registered first Nam Cao
2025-07-18 9:35 ` Gabriele Monaco [this message]
2025-07-18 10:27 ` Greg KH
2025-07-18 10:40 ` Nam Cao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57b1e83ede5ef8f5b2d661b3326374d82da0bd1a.camel@redhat.com \
--to=gmonaco@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=namcao@linutronix.de \
--cc=rostedt@goodmis.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox