Re: [PATCH] sh: hp6xx: unset apm_get_power_status on unload

[Artur Rojek]

On 2026-04-18 21:43, John Paul Adrian Glaubitz wrote:
> Hi Artur,
>
> On Sat, 2026-04-18 at 21:20 +0200, Artur Rojek wrote:
> > On 2026-04-12 09:30, John Paul Adrian Glaubitz wrote:
> > > Hi Ahelenia,
> > >
> > > On Fri, 2025-10-17 at 00:05 +0200, Ahelenia Ziemiańska wrote:
> > > > The API for apm_get_power_status is "call it if it isn't NULL".
> > > > If the module is unloaded and it's not unset,
> > > > reading /proc/apm will jump into unloaded kernel memory.
> > > >
> > > > The first commit that added this incompletely refactored
> > > > the assigned-to variable in __exit,
> > > > the second deleted it instead of fixing it.
> > > >
> > > > Unset it on unload like drivers/macintosh/apm_emu.c.
> > > >
> > > > Fixes: 0a9b0db19262 ("[APM] SH: Convert to use shared APM emulation.")
> > > > Fixes: 8c8ee8254767 ("sh: hp6xx: APM build fix and new battery
> > > > values.")
> > > > Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@xxxxxxxxxxxxxxxxxx>
> > > > ---
> > > >  arch/sh/boards/mach-hp6xx/hp6xx_apm.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/arch/sh/boards/mach-hp6xx/hp6xx_apm.c
> > > > b/arch/sh/boards/mach-hp6xx/hp6xx_apm.c
> > > > index e5c4c7d34139..089eca39c4e6 100644
> > > > --- a/arch/sh/boards/mach-hp6xx/hp6xx_apm.c
> > > > +++ b/arch/sh/boards/mach-hp6xx/hp6xx_apm.c
> > > > @@ -98,6 +98,9 @@ static int __init hp6x0_apm_init(void)
> > > >
> > > >  static void __exit hp6x0_apm_exit(void)
> > > >  {
> > > > +	if (apm_get_power_status == hp6x0_apm_get_power_status)
> > > > +		apm_get_power_status = NULL;
> > > > +
> > > >  	free_irq(HP680_BTN_IRQ, 0);
> > > >  }
> > > >
> > >
> > > Let's CC Artur Rojek who has worked on the hp6xx code before and should
> > > give his ACK.
> >
> > This driver can't be built as a module, so this code is never called.
> > But since we can't get rid of module_exit(apm_exit):
>
> Isn't hp6x0_apm_exit() called when the device is being shutdown even 
> when
> it's not built as a module?

I went by suggestion found in the kernel docs [1], and after a quick
glance over include/linux/module.h:
> /**
>  * module_exit() - driver exit entry point
>  * @x: function to be run when driver is removed
>  *
>  * module_exit() will wrap the driver clean-up code
>  * with cleanup_module() when used with rmmod when
>  * the driver is a module.  If the driver is statically
>  * compiled into the kernel, module_exit() has no effect.
>  * There can only be one per module.
>  */
> #define module_exit(x)	__exitcall(x);

The symbol is certainly not eliminated at compilation time:
> $ readelf -sW vmlinux | grep hp6x0_apm_exit
>  2586: 8d36b000    36 FUNC    LOCAL  DEFAULT   14 hp6x0_apm_exit

However, the section which should hold it does not exit:
> #define __exitcall(fn)						\
> 	static exitcall_t __exitcall_##fn __exit_call = fn
> (...)
> #define __exit_call	__used __section(".exitcall.exit")

> $ readelf -S vmlinux | grep "exitcall\.exit"
> $ echo $?
> 1

I don't know what to make of it, but nevertheless, I am ok with merging
the patch in question.

Cheers,
Artur

[1] 
https://www.kernel.org/doc/html/latest/driver-api/basics.html#c.module_exit

> Adrian