2022-06-06 06:25:21

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 0/3] net: unexport some symbols that are annotated __init


This patch set fixes odd combinations
of EXPORT_SYMBOL and __init.

Checking this in modpost is a good thing and I really wanted to do it,
but Linus Torvalds imposes a very strict rule, "No new warning".

I'd like the maintainer to kindly pick this up and send a fixes pull request.

Unless I eliminate all the sites of warnings beforehand,
Linus refuses to re-enable the modpost check.   [1]

[1]: https://lore.kernel.org/linux-kbuild/CAK7LNATmd0bigp7HQ4fTzHw5ugZMkSb3UXG7L4fxpGbqkRKESA@mail.gmail.com/T/#m5e50cc2da17491ba210c72b5efdbc0ce76e0217f



Masahiro Yamada (3):
net: mdio: unexport __init-annotated mdio_bus_init()
net: xfrm: unexport __init-annotated xfrm4_protocol_init()
net: ipv6: unexport __init-annotated seg6_hmac_init()

drivers/net/phy/mdio_bus.c | 1 -
net/ipv4/xfrm4_protocol.c | 1 -
net/ipv6/seg6_hmac.c | 1 -
3 files changed, 3 deletions(-)

--
2.32.0


2022-06-06 06:25:22

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 2/3] net: xfrm: unexport __init-annotated xfrm4_protocol_init()

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,
net/ipv4/xfrm4_policy.c is never compiled as modular.
(CONFIG_XFRM is boolean)

Fixes: 2f32b51b609f ("xfrm: Introduce xfrm_input_afinfo to access the the callbacks properly")
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---

net/ipv4/xfrm4_protocol.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/net/ipv4/xfrm4_protocol.c b/net/ipv4/xfrm4_protocol.c
index 2fe5860c21d6..b146ce88c5d0 100644
--- a/net/ipv4/xfrm4_protocol.c
+++ b/net/ipv4/xfrm4_protocol.c
@@ -304,4 +304,3 @@ void __init xfrm4_protocol_init(void)
{
xfrm_input_register_afinfo(&xfrm4_input_afinfo);
}
-EXPORT_SYMBOL(xfrm4_protocol_init);
--
2.32.0

2022-06-06 06:27:41

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 3/3] net: ipv6: unexport __init-annotated seg6_hmac_init()

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 caller (net/ipv6/seg6.c)
and the callee (net/ipv6/seg6_hmac.c) belong to the same module.
It seems an internal function call in ipv6.ko.

Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support")
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---

net/ipv6/seg6_hmac.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index 29bc4e7c3046..6de01185cc68 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -399,7 +399,6 @@ int __init seg6_hmac_init(void)
{
return seg6_hmac_init_algo();
}
-EXPORT_SYMBOL(seg6_hmac_init);

int __net_init seg6_hmac_net_init(struct net *net)
{
--
2.32.0

2022-06-07 18:27:39

by Paolo Abeni

[permalink] [raw]
Subject: Re: [PATCH 2/3] net: xfrm: unexport __init-annotated xfrm4_protocol_init()

On Mon, 2022-06-06 at 13:53 +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,
> net/ipv4/xfrm4_policy.c is never compiled as modular.
> (CONFIG_XFRM is boolean)
>
> Fixes: 2f32b51b609f ("xfrm: Introduce xfrm_input_afinfo to access the the callbacks properly")
> Reported-by: Stephen Rothwell <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>

@Steffen: are you ok if we take this one in the -net tree directly?
Otherwise a repost would probably be the better option, with this patch
stand-alone targeting the ipsec tree and the other 2 targeting -net.

Thanks!

Paolo

2022-06-08 17:28:06

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH 0/3] net: unexport some symbols that are annotated __init

Hello:

This series was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:

On Mon, 6 Jun 2022 13:53:52 +0900 you wrote:
> This patch set fixes odd combinations
> of EXPORT_SYMBOL and __init.
>
> Checking this in modpost is a good thing and I really wanted to do it,
> but Linus Torvalds imposes a very strict rule, "No new warning".
>
> I'd like the maintainer to kindly pick this up and send a fixes pull request.
>
> [...]

Here is the summary with links:
- [1/3] net: mdio: unexport __init-annotated mdio_bus_init()
https://git.kernel.org/netdev/net/c/35b42dce6197
- [2/3] net: xfrm: unexport __init-annotated xfrm4_protocol_init()
https://git.kernel.org/netdev/net/c/4a388f08d878
- [3/3] net: ipv6: unexport __init-annotated seg6_hmac_init()
https://git.kernel.org/netdev/net/c/5801f064e351

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html