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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] TOCTOU race in `rpi_firmware_get` allows `kref_get_unless_zero` to execute on freed memory during concurrent driver unbinding.
--

--- Patch [3]: [PATCH v3 3/4] firmware: raspberrypi: Add reboot mode support ---
commit 0477b96e458c7ecca585d15b527bfef8d16f88fc
Author: Gregor Herburger <gregor.herburger@linutronix.de>
Subject: firmware: raspberrypi: Add reboot mode support

This commit adds tryboot mode support to the Raspberry Pi firmware driver to
enable an A/B update scheme. It registers a reboot mode driver to send the
RPI_FIRMWARE_SET_REBOOT_FLAGS message. Additionally, it transitions the
firmware context teardown to be devres-managed to safely register the reboot
mode.

> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> index 0aa322e9a2e73..2a0c40b8052e5 100644
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
[ ... ]
> @@ -302,9 +331,14 @@ static int rpi_firmware_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, fw);
>  
> +	ret = devm_add_action_or_reset(dev, devm_rpi_firmware_put, fw);
> +	if (ret)
> +		return ret;
> +

[Severity: High]
This is a pre-existing issue, but modifying the teardown sequence brings up a
question about rpi_firmware_get().

Can a concurrent driver unbind and probe trigger a use-after-free in
rpi_firmware_get()?

If a privileged user triggers unbinding, the driver core executes
device_unbind_cleanup(). This calls devres_release_all(), which now triggers
devm_rpi_firmware_put(), dropping the kref to 0 and freeing the fw object.
Crucially, the driver core clears the drvdata after devres_release_all().

If another thread concurrently probes a dependent driver, rpi_firmware_get()
could locklessly read the drvdata pointer before it is cleared:

drivers/firmware/raspberrypi.c:rpi_firmware_get() {
    ...
    fw = platform_get_drvdata(pdev);
    if (!fw)
        goto err_put_device;

    if (!kref_get_unless_zero(&fw->consumers))
        goto err_put_device;
    ...
}

Since fw could already be freed by devm_rpi_firmware_put(), passing it to
kref_get_unless_zero() would access freed memory. Does the retrieval of
drvdata need synchronization to protect against concurrent unbinding?

>  	rpi_firmware_print_firmware_revision(fw);
>  	rpi_register_hwmon_driver(dev, fw);
>  	rpi_register_clk_driver(dev);
> +	rpi_register_reboot_mode(dev, fw);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-rpi-tryboot-v3-0-fda82fe7ed76@linutronix.de?part=3