[PATCH] reboot: don't use cb_data to tell if the static sys-off handler is free

[PATCH] reboot: don't use cb_data to tell if the static sys-off handler is free

[Bradley Morgan]

There is one static handler for SYS_OFF_PRIO_PLATFORM, shared by all
modes, so the second platform priority registration has to fail with
-EBUSY. alloc_sys_off_handler() decides that by testing cb_data.

cb_data is caller data though, and nothing says it has to be set. A
registration that passes NULL for it leaves the static handler
looking free, so the next one gets the same struct back while the
first is still live: sys_off_cb and list get overwritten, and the
same notifier_block is registered into a second chain. A
notifier_block has one next pointer, so both chains end up wrong.

register_platform_power_off() always passes the power off callback as
cb_data, which is why this held up when the static handler was added.
Callers using register_sys_off_handler() directly have no such rule.

Test sys_off_cb instead. Every handler has one, it is what the
callback path actually needs, and it is set for as long as the
handler is registered.

Fixes: 587b9bfe0668 ("kernel/reboot: Use static handler for register_platform_power_off()")
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
 kernel/reboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/reboot.c b/kernel/reboot.c
index f070c5c1103a..42fe2708f5d9 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -366,7 +366,7 @@ static struct sys_off_handler *alloc_sys_off_handler(int priority)
 	 */
 	if (priority == SYS_OFF_PRIO_PLATFORM) {
 		handler = &platform_sys_off_handler;
-		if (handler->cb_data)
+		if (handler->sys_off_cb)
 			return ERR_PTR(-EBUSY);
 	} else {
 		if (system_state > SYSTEM_RUNNING)
-- 
2.47.3

Re: [PATCH] reboot: don't use cb_data to tell if the static sys-off handler is free

[Andrew Morton]

On Sun, 26 Jul 2026 22:55:41 +0000 Bradley Morgan <include@grrlz.net> wrote:

> There is one static handler for SYS_OFF_PRIO_PLATFORM, shared by all
> modes, so the second platform priority registration has to fail with
> -EBUSY. alloc_sys_off_handler() decides that by testing cb_data.
> 
> cb_data is caller data though, and nothing says it has to be set. A
> registration that passes NULL for it leaves the static handler
> looking free, so the next one gets the same struct back while the
> first is still live: sys_off_cb and list get overwritten, and the
> same notifier_block is registered into a second chain. A
> notifier_block has one next pointer, so both chains end up wrong.
> 
> register_platform_power_off() always passes the power off callback as
> cb_data, which is why this held up when the static handler was added.
> Callers using register_sys_off_handler() directly have no such rule.
> 
> Test sys_off_cb instead. Every handler has one, it is what the
> callback path actually needs, and it is set for as long as the
> handler is registered.
> 
> Fixes: 587b9bfe0668 ("kernel/reboot: Use static handler for register_platform_power_off()")

Please do try to hunt down the people who are best situated to review
proposed changes.  `git show 587b9bfe0668' ain't hard!

I assume you saw the possible pre-existing thing:
	https://sashiko.dev/#/patchset/20260726225541.13304-1-include@grrlz.net

>
> ...
>
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -366,7 +366,7 @@ static struct sys_off_handler *alloc_sys_off_handler(int priority)
>  	 */
>  	if (priority == SYS_OFF_PRIO_PLATFORM) {
>  		handler = &platform_sys_off_handler;
> -		if (handler->cb_data)
> +		if (handler->sys_off_cb)
>  			return ERR_PTR(-EBUSY);
>  	} else {
>  		if (system_state > SYSTEM_RUNNING)
> -- 
> 2.47.3