Re: [PATCH 3/4] firmware: raspberrypi: Add reboot mode support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Mixing managed (devm) and manual API calls for reboot mode registration causes a double-free and a use-after-free on device removal.
- [High] Ignoring the return value of `devm_reboot_mode_register` leads to a NULL pointer dereference or double free if registration fails.
--

commit c4eb702fd9dc0d615bc8a99fbb83befc50f8971f
Author: Gregor Herburger <gregor.herburger@linutronix.de>

firmware: raspberrypi: Add reboot mode support

The Raspberry Pi firmware has a tryboot mode where it tries to boot from
another partition. This patch adds support for this by registering a reboot
mode driver.

> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -306,6 +323,12 @@ static int rpi_firmware_probe(struct platform_device *pdev)
>  	rpi_register_hwmon_driver(dev, fw);
>  	rpi_register_clk_driver(dev);
>  
> +	fw->reboot_mode.dev = dev;
> +	fw->reboot_mode.write = rpi_firmware_reboot_mode_write;
> +	ret = devm_reboot_mode_register(dev, &fw->reboot_mode);
> +	if (ret)
> +		dev_err(dev, "Failed to register reboot mode: %d\n", ret);
> +
>  	return 0;
>  }

[Severity: High]
Should we return the error code here instead of returning 0?

If devm_reboot_mode_register() fails, the probe succeeds but the internal
list head for the reboot mode might remain uninitialized. 

Later during device removal, rpi_firmware_remove() calls
reboot_mode_unregister() unconditionally. This could lead to a null pointer
dereference when iterating the uninitialized list, or a double free if the
failure occurred inside the register function and it already freed the
resources.

[ ... ]

> @@ -323,6 +346,8 @@ static void rpi_firmware_remove(struct platform_device *pdev)
>  {
>  	struct rpi_firmware *fw = platform_get_drvdata(pdev);
>  
> +	reboot_mode_unregister(&fw->reboot_mode);
> +

[Severity: High]
Could this manual unregister cause a double-free and use-after-free?

Since devm_reboot_mode_register() is used in probe, the devres cleanup will
automatically unregister the reboot mode when the device is removed.

By manually calling reboot_mode_unregister() here:
1. It frees the strings in the list.
2. rpi_firmware_put(fw) drops the reference, potentially freeing the fw
   structure.
3. The devres cleanup runs devm_reboot_mode_release() upon returning,
   accessing the freed fw structure and attempting to unregister and free
   the strings a second time.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rpi-tryboot-v1-0-490b1c4c4970@linutronix.de?part=3