Re: [PATCH v23 03/13] power: reset: reboot-mode: Add support for predefined reboot modes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardcoded non-unique device naming causes cross-instance destruction of sysfs state on probe failure.
--

--- Patch [3]: [PATCH v23 03/13] power: reset: reboot-mode: Add support for predefined reboot modes ---
commit 7f64e78c4dace557cca8c3399655be137b5cc95f
Author: Shivendra Pratap <shivendra.pratap@oss.qualcomm.com>

power: reset: reboot-mode: Add support for predefined reboot modes
    
Extend the reboot-mode framework to handle predefined modes alongside
those defined in device tree. Drivers can now define their own
reboot-modes and register them via the framework. A centralized init
call has been added to the reboot-mode framework and adopted by
existing drivers.

> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
[ ... ]
> @@ -267,7 +317,7 @@ static inline void reboot_mode_unregister_device(struct reboot_mode_driver *rebo
>  	if (!priv)
>  		return;
>  
> -	reboot_mode_release_list(priv);
> +	reboot_mode_release_list(&priv->head);
>  	kfree(priv);
>  }

[Severity: High]
This is a pre-existing issue, but does the hardcoded use of the generic
driver name for the sysfs device cause cross-instance destruction if
multiple instances of a reboot-mode driver are probed?

If the device tree defines multiple instances (e.g. syscon-reboot-mode),
the first instance successfully creates a sysfs device using
reboot->dev->driver->name:

reboot_mode_create_device() {
        ...
        priv->reboot_mode_device = device_create(&reboot_mode_class, NULL, 0,
                                                 (void *)priv, "%s",
                                                 reboot->dev->driver->name);
}

The second instance would fail device_create() with -EEXIST due to the
name collision. During its error path cleanup,
reboot_mode_unregister_device() searches for the device by the same
generic driver name:

reboot_mode_unregister_device() {
        ...
        reboot_mode_device = class_find_device(&reboot_mode_class, NULL,
                                               reboot->dev->driver->name,
                                               reboot_mode_match_by_name);
        ...
        device_unregister(reboot_mode_device);
        
        if (!priv)
                return;
        
        ...
        kfree(priv);
}

Could this erroneously find the first successfully probed instance's sysfs
device, unregister it, and free its priv data, leaving the first instance
silently corrupted? Would it be safer to use a unique instance name like
dev_name(reboot->dev) for device_create() instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-arm-psci-system_reset2-vendor-reboots-v23-0-e7453c548c21@oss.qualcomm.com?part=3