* [PATCH] clocksource: hyper-v: unexport __init-annotated hv_init_clocksource()
@ 2022-06-06 5:02 Masahiro Yamada
2022-06-06 6:59 ` Vitaly Kuznetsov
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Masahiro Yamada @ 2022-06-06 5:02 UTC (permalink / raw)
To: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, linux-hyperv
Cc: Masahiro Yamada, Stephen Rothwell, Daniel Lezcano,
Michael Kelley, Thomas Gleixner, Vitaly Kuznetsov, linux-kernel
EXPORT_SYMBOL and __init is a bad combination because the .init.text
section is freed up after the initialization. Hence, modules cannot
use symbols annotated __init. The access to a freed symbol may end up
with kernel panic.
modpost used to detect it, but it has been broken for a decade.
Recently, I fixed modpost so it started to warn it again, then this
showed up in linux-next builds.
There are two ways to fix it:
- Remove __init
- Remove EXPORT_SYMBOL
I chose the latter for this case because the only in-tree call-site,
arch/x86/kernel/cpu/mshyperv.c is never compiled as modular.
(CONFIG_HYPERVISOR_GUEST is boolean)
Fixes: dd2cb348613b ("clocksource/drivers: Continue making Hyper-V clocksource ISA agnostic")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
drivers/clocksource/hyperv_timer.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index ff188ab68496..bb47610bbd1c 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -565,4 +565,3 @@ void __init hv_init_clocksource(void)
hv_sched_clock_offset = hv_read_reference_counter();
hv_setup_sched_clock(read_hv_sched_clock_msr);
}
-EXPORT_SYMBOL_GPL(hv_init_clocksource);
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] clocksource: hyper-v: unexport __init-annotated hv_init_clocksource()
2022-06-06 5:02 [PATCH] clocksource: hyper-v: unexport __init-annotated hv_init_clocksource() Masahiro Yamada
@ 2022-06-06 6:59 ` Vitaly Kuznetsov
2022-06-07 20:35 ` Michael Kelley (LINUX)
2022-06-08 12:27 ` Wei Liu
2 siblings, 0 replies; 4+ messages in thread
From: Vitaly Kuznetsov @ 2022-06-06 6:59 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Stephen Rothwell, Daniel Lezcano, Michael Kelley,
Thomas Gleixner, linux-kernel, K . Y . Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, Dexuan Cui, linux-hyperv
Masahiro Yamada <masahiroy@kernel.org> writes:
> EXPORT_SYMBOL and __init is a bad combination because the .init.text
> section is freed up after the initialization. Hence, modules cannot
> use symbols annotated __init. The access to a freed symbol may end up
> with kernel panic.
>
> modpost used to detect it, but it has been broken for a decade.
>
> Recently, I fixed modpost so it started to warn it again, then this
> showed up in linux-next builds.
>
> There are two ways to fix it:
>
> - Remove __init
> - Remove EXPORT_SYMBOL
>
> I chose the latter for this case because the only in-tree call-site,
> arch/x86/kernel/cpu/mshyperv.c is never compiled as modular.
> (CONFIG_HYPERVISOR_GUEST is boolean)
>
> Fixes: dd2cb348613b ("clocksource/drivers: Continue making Hyper-V clocksource ISA agnostic")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> drivers/clocksource/hyperv_timer.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index ff188ab68496..bb47610bbd1c 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -565,4 +565,3 @@ void __init hv_init_clocksource(void)
> hv_sched_clock_offset = hv_read_reference_counter();
> hv_setup_sched_clock(read_hv_sched_clock_msr);
> }
> -EXPORT_SYMBOL_GPL(hv_init_clocksource);
hv_init_clocksource() is not called from modules indeed,
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] clocksource: hyper-v: unexport __init-annotated hv_init_clocksource()
2022-06-06 5:02 [PATCH] clocksource: hyper-v: unexport __init-annotated hv_init_clocksource() Masahiro Yamada
2022-06-06 6:59 ` Vitaly Kuznetsov
@ 2022-06-07 20:35 ` Michael Kelley (LINUX)
2022-06-08 12:27 ` Wei Liu
2 siblings, 0 replies; 4+ messages in thread
From: Michael Kelley (LINUX) @ 2022-06-07 20:35 UTC (permalink / raw)
To: Masahiro Yamada, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, linux-hyperv
Cc: Stephen Rothwell, Daniel Lezcano, Thomas Gleixner, vkuznets,
linux-kernel
From: Masahiro Yamada <masahiroy@kernel.org> Sent: Sunday, June 5, 2022 10:03 PM
>
> EXPORT_SYMBOL and __init is a bad combination because the .init.text
> section is freed up after the initialization. Hence, modules cannot
> use symbols annotated __init. The access to a freed symbol may end up
> with kernel panic.
>
> modpost used to detect it, but it has been broken for a decade.
>
> Recently, I fixed modpost so it started to warn it again, then this
> showed up in linux-next builds.
>
> There are two ways to fix it:
>
> - Remove __init
> - Remove EXPORT_SYMBOL
>
> I chose the latter for this case because the only in-tree call-site,
> arch/x86/kernel/cpu/mshyperv.c is never compiled as modular.
> (CONFIG_HYPERVISOR_GUEST is boolean)
>
> Fixes: dd2cb348613b ("clocksource/drivers: Continue making Hyper-V clocksource ISA
> agnostic")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> drivers/clocksource/hyperv_timer.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index ff188ab68496..bb47610bbd1c 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -565,4 +565,3 @@ void __init hv_init_clocksource(void)
> hv_sched_clock_offset = hv_read_reference_counter();
> hv_setup_sched_clock(read_hv_sched_clock_msr);
> }
> -EXPORT_SYMBOL_GPL(hv_init_clocksource);
> --
> 2.32.0
I agree this is the right solution.
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] clocksource: hyper-v: unexport __init-annotated hv_init_clocksource()
2022-06-06 5:02 [PATCH] clocksource: hyper-v: unexport __init-annotated hv_init_clocksource() Masahiro Yamada
2022-06-06 6:59 ` Vitaly Kuznetsov
2022-06-07 20:35 ` Michael Kelley (LINUX)
@ 2022-06-08 12:27 ` Wei Liu
2 siblings, 0 replies; 4+ messages in thread
From: Wei Liu @ 2022-06-08 12:27 UTC (permalink / raw)
To: Masahiro Yamada
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, linux-hyperv, Stephen Rothwell, Daniel Lezcano,
Michael Kelley, Thomas Gleixner, Vitaly Kuznetsov, linux-kernel
On Mon, Jun 06, 2022 at 02:02:38PM +0900, Masahiro Yamada wrote:
> EXPORT_SYMBOL and __init is a bad combination because the .init.text
> section is freed up after the initialization. Hence, modules cannot
> use symbols annotated __init. The access to a freed symbol may end up
> with kernel panic.
>
> modpost used to detect it, but it has been broken for a decade.
>
> Recently, I fixed modpost so it started to warn it again, then this
> showed up in linux-next builds.
>
> There are two ways to fix it:
>
> - Remove __init
> - Remove EXPORT_SYMBOL
>
> I chose the latter for this case because the only in-tree call-site,
> arch/x86/kernel/cpu/mshyperv.c is never compiled as modular.
> (CONFIG_HYPERVISOR_GUEST is boolean)
>
> Fixes: dd2cb348613b ("clocksource/drivers: Continue making Hyper-V clocksource ISA agnostic")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Applied to hyperv-fixes. Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-08 12:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-06 5:02 [PATCH] clocksource: hyper-v: unexport __init-annotated hv_init_clocksource() Masahiro Yamada
2022-06-06 6:59 ` Vitaly Kuznetsov
2022-06-07 20:35 ` Michael Kelley (LINUX)
2022-06-08 12:27 ` Wei Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.