| 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: <57b1e83ede5ef8f5b2d661b3326374d82da0bd1a.camel@redhat.com>
Date: Fri, 18 Jul 2025 11:35:27 +0200
From: Gabriele Monaco <gmonaco@...hat.com>
To: Nam Cao <namcao@...utronix.de>, Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
<mathieu.desnoyers@...icios.com>, linux-trace-kernel@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: stable@...r.kernel.org
Subject: Re: [PATCH] rv: Ensure containers are registered first
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@...utronix.de>
> Cc: stable@...r.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@...utronix.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@...utronix.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@...hat.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@...hat.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@...hat.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@...utronix.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@...hat.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@...hat.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@...hat.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@...hat.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@...nel.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@...nel.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 */);
Powered by blists - more mailing lists