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 <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---
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
Masahiro Yamada <[email protected]> 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 <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> 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 <[email protected]>
--
Vitaly
From: Masahiro Yamada <[email protected]> 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 <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> 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 <[email protected]>
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 <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
Applied to hyperv-fixes. Thanks.