Re: [PATCH 4/4] arm64: dts: broadcom: bcm2712: Add reboot modes to firmware node

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Adding `mode-*` properties activates a dormant Use-After-Free and Double-Free vulnerability in the driver's unbind path.
--

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

arm64: dts: broadcom: bcm2712: Add reboot modes to firmware node

The raspberry pi firmware driver allows the tryboot reboot mode. Add
this mode and normal boot mode to the node.

> diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-base.dtsi b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-base.dtsi
> index b7a6bc34ae1ab..67095c7ff770d 100644
> --- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-base.dtsi
> +++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-base.dtsi
> @@ -49,6 +49,8 @@ firmware: rpi-firmware {
>  			compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
>  
>  			mboxes = <&mailbox>;
> +			mode-normal = <0>;
> +			mode-tryboot = <1>;

[Severity: Critical]
This is a pre-existing issue, but does adding these properties to the
device tree activate a dormant use-after-free and double-free vulnerability
in the driver's unbind path?

Looking at drivers/firmware/raspberrypi.c, the driver registers the reboot
mode using the devm variant during probe, but also manually unregisters it
during remove:

drivers/firmware/raspberrypi.c:rpi_firmware_remove() {
    ...
    reboot_mode_unregister(&fw->reboot_mode);
    rpi_firmware_put(fw);
    ...
}

The call to rpi_firmware_put() drops the reference and frees the fw object.

Because devm_reboot_mode_register() was used during probe, the devres cleanup
will eventually run devm_reboot_mode_release() which calls
reboot_mode_unregister() a second time. This passes the now-freed
fw->reboot_mode pointer.

Furthermore, the list strings are freed during the first unregister call:

drivers/power/reset/reboot-mode.c:reboot_mode_unregister() {
    ...
    list_for_each_entry(info, &reboot->head, list)
        kfree_const(info->mode);
    ...
}

Because the list pointers are not cleared and the list elements are preserved,
the second unregister call will iterate over the list again. Since this patch
populates that list by adding mode-normal and mode-tryboot, the second call
will try to free the already-freed info->mode pointers.

Should the manual reboot_mode_unregister() call be removed from
rpi_firmware_remove() before adding these device tree properties?

>  
>  			firmware_clocks: clocks {
>  				compatible = "raspberrypi,firmware-clocks";

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