2023-12-12 21:47:24

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] power: reset: at91: mark at91_wakeup_status non-__init

From: Arnd Bergmann <[email protected]>

Two copies of the at91_wakeup_status() function are called by the
respective probe() callbacks and are marked __init, but the probe
functions are no longer annotated that way. This works with gcc because
the functions always get inlined, but clang keeps them separate, which
can lead to executing freed memory:

WARNING: modpost: vmlinux: section mismatch in reference: at91_poweroff_probe+0x80 (section: .text) -> at91_wakeup_status (section: .init.text)
WARNING: modpost: vmlinux: section mismatch in reference: at91_shdwc_probe+0xcc (section: .text) -> at91_wakeup_status (section: .init.text)

Drop the incorrect annotation on these.

Fixes: 099806de68b7 ("power: reset: at91-poweroff: Stop using module_platform_driver_probe()")
Fixes: dde74a5de817 ("power: reset: at91-sama5d2_shdwc: Stop using module_platform_driver_probe()")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/power/reset/at91-poweroff.c | 2 +-
drivers/power/reset/at91-sama5d2_shdwc.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/reset/at91-poweroff.c b/drivers/power/reset/at91-poweroff.c
index 126e774e210c..93eece027865 100644
--- a/drivers/power/reset/at91-poweroff.c
+++ b/drivers/power/reset/at91-poweroff.c
@@ -57,7 +57,7 @@ static struct shdwc {
void __iomem *mpddrc_base;
} at91_shdwc;

-static void __init at91_wakeup_status(struct platform_device *pdev)
+static void at91_wakeup_status(struct platform_device *pdev)
{
const char *reason;
u32 reg = readl(at91_shdwc.shdwc_base + AT91_SHDW_SR);
diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c
index af95c7b39cb3..959ce0dbe91d 100644
--- a/drivers/power/reset/at91-sama5d2_shdwc.c
+++ b/drivers/power/reset/at91-sama5d2_shdwc.c
@@ -107,7 +107,7 @@ static const unsigned long long sdwc_dbc_period[] = {
0, 3, 32, 512, 4096, 32768,
};

-static void __init at91_wakeup_status(struct platform_device *pdev)
+static void at91_wakeup_status(struct platform_device *pdev)
{
struct shdwc *shdw = platform_get_drvdata(pdev);
const struct reg_config *rcfg = shdw->rcfg;
--
2.39.2


2023-12-12 21:50:48

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] power: reset: at91: mark at91_wakeup_status non-__init

On Tue, Dec 12, 2023 at 10:46:49PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> Two copies of the at91_wakeup_status() function are called by the
> respective probe() callbacks and are marked __init, but the probe
> functions are no longer annotated that way. This works with gcc because
> the functions always get inlined, but clang keeps them separate, which
> can lead to executing freed memory:
>
> WARNING: modpost: vmlinux: section mismatch in reference: at91_poweroff_probe+0x80 (section: .text) -> at91_wakeup_status (section: .init.text)
> WARNING: modpost: vmlinux: section mismatch in reference: at91_shdwc_probe+0xcc (section: .text) -> at91_wakeup_status (section: .init.text)
>
> Drop the incorrect annotation on these.
>
> Fixes: 099806de68b7 ("power: reset: at91-poweroff: Stop using module_platform_driver_probe()")
> Fixes: dde74a5de817 ("power: reset: at91-sama5d2_shdwc: Stop using module_platform_driver_probe()")
> Signed-off-by: Arnd Bergmann <[email protected]>

Thanks, I sent the same change three weeks ago at this point:

https://lore.kernel.org/[email protected]/

Your commit message is a little better than mine and I don't really care
which one goes in but it would be good if this could get cleared up
soon...

Reviewed-by: Nathan Chancellor <[email protected]>

> ---
> drivers/power/reset/at91-poweroff.c | 2 +-
> drivers/power/reset/at91-sama5d2_shdwc.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/power/reset/at91-poweroff.c b/drivers/power/reset/at91-poweroff.c
> index 126e774e210c..93eece027865 100644
> --- a/drivers/power/reset/at91-poweroff.c
> +++ b/drivers/power/reset/at91-poweroff.c
> @@ -57,7 +57,7 @@ static struct shdwc {
> void __iomem *mpddrc_base;
> } at91_shdwc;
>
> -static void __init at91_wakeup_status(struct platform_device *pdev)
> +static void at91_wakeup_status(struct platform_device *pdev)
> {
> const char *reason;
> u32 reg = readl(at91_shdwc.shdwc_base + AT91_SHDW_SR);
> diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c
> index af95c7b39cb3..959ce0dbe91d 100644
> --- a/drivers/power/reset/at91-sama5d2_shdwc.c
> +++ b/drivers/power/reset/at91-sama5d2_shdwc.c
> @@ -107,7 +107,7 @@ static const unsigned long long sdwc_dbc_period[] = {
> 0, 3, 32, 512, 4096, 32768,
> };
>
> -static void __init at91_wakeup_status(struct platform_device *pdev)
> +static void at91_wakeup_status(struct platform_device *pdev)
> {
> struct shdwc *shdw = platform_get_drvdata(pdev);
> const struct reg_config *rcfg = shdw->rcfg;
> --
> 2.39.2
>
>

2023-12-13 07:26:14

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH] power: reset: at91: mark at91_wakeup_status non-__init

Hello,

[dropped Justin Stitt from Cc, their email address bounced for me in the
past]

On Tue, Dec 12, 2023 at 02:50:02PM -0700, Nathan Chancellor wrote:
> On Tue, Dec 12, 2023 at 10:46:49PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > Two copies of the at91_wakeup_status() function are called by the
> > respective probe() callbacks and are marked __init, but the probe
> > functions are no longer annotated that way. This works with gcc because
> > the functions always get inlined, but clang keeps them separate, which
> > can lead to executing freed memory:
> >
> > WARNING: modpost: vmlinux: section mismatch in reference: at91_poweroff_probe+0x80 (section: .text) -> at91_wakeup_status (section: .init.text)
> > WARNING: modpost: vmlinux: section mismatch in reference: at91_shdwc_probe+0xcc (section: .text) -> at91_wakeup_status (section: .init.text)
> >
> > Drop the incorrect annotation on these.
> >
> > Fixes: 099806de68b7 ("power: reset: at91-poweroff: Stop using module_platform_driver_probe()")
> > Fixes: dde74a5de817 ("power: reset: at91-sama5d2_shdwc: Stop using module_platform_driver_probe()")
> > Signed-off-by: Arnd Bergmann <[email protected]>
>
> Thanks, I sent the same change three weeks ago at this point:
>
> https://lore.kernel.org/[email protected]/
>
> Your commit message is a little better than mine and I don't really care
> which one goes in but it would be good if this could get cleared up
> soon...
>
> Reviewed-by: Nathan Chancellor <[email protected]>

I don't care either. Given the change is identical (Nathan's submission
even has "index 126e774e210c..93eece027865 100644" which exactly matches
Arnd's patch), I'll forward my Reviewed-by: tag to here:

Reviewed-by: Uwe Kleine-K?nig <[email protected]>

Thanks
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |


Attachments:
(No filename) (2.01 kB)
signature.asc (499.00 B)
Download all attachments

2023-12-13 13:03:35

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH] power: reset: at91: mark at91_wakeup_status non-__init

On 13/12/2023 at 08:25, Uwe Kleine-König wrote:
> On Tue, Dec 12, 2023 at 02:50:02PM -0700, Nathan Chancellor wrote:
>> On Tue, Dec 12, 2023 at 10:46:49PM +0100, Arnd Bergmann wrote:
>>> From: Arnd Bergmann<[email protected]>
>>>
>>> Two copies of the at91_wakeup_status() function are called by the
>>> respective probe() callbacks and are marked __init, but the probe
>>> functions are no longer annotated that way. This works with gcc because
>>> the functions always get inlined, but clang keeps them separate, which
>>> can lead to executing freed memory:
>>>
>>> WARNING: modpost: vmlinux: section mismatch in reference: at91_poweroff_probe+0x80 (section: .text) -> at91_wakeup_status (section: .init.text)
>>> WARNING: modpost: vmlinux: section mismatch in reference: at91_shdwc_probe+0xcc (section: .text) -> at91_wakeup_status (section: .init.text)
>>>
>>> Drop the incorrect annotation on these.
>>>
>>> Fixes: 099806de68b7 ("power: reset: at91-poweroff: Stop using module_platform_driver_probe()")
>>> Fixes: dde74a5de817 ("power: reset: at91-sama5d2_shdwc: Stop using module_platform_driver_probe()")
>>> Signed-off-by: Arnd Bergmann<[email protected]>
>> Thanks, I sent the same change three weeks ago at this point:
>>
>> https://lore.kernel.org/[email protected]/
>>
>> Your commit message is a little better than mine and I don't really care
>> which one goes in but it would be good if this could get cleared up
>> soon...
>>
>> Reviewed-by: Nathan Chancellor<[email protected]>
> I don't care either. Given the change is identical (Nathan's submission
> even has "index 126e774e210c..93eece027865 100644" which exactly matches
> Arnd's patch), I'll forward my Reviewed-by: tag to here:
>
> Reviewed-by: Uwe Kleine-König<[email protected]>

Likewise:
Acked-by: Nicolas Ferre <[email protected]>

Use whichever is preferred.
Regards,
Nicolas