[PATCH 0/4] firmware: raspberrypi: Add support for the tryboot mode

[PATCH 0/4] firmware: raspberrypi: Add support for the tryboot mode

[Gregor Herburger]

This adds support for the tryboot mode on Raspberry Pis. As there is no
documentation other than the downstream implementation [0] the
implementation is based on this.

I tested this on Raspberry Pi 5 and therefore I only added the
properties to this devicetree. But afaik this should work on all
Raspberry Pis. I will add it to the correspondings dts if I get some
hardware to test it.

[0] https://github.com/raspberrypi/linux/commit/eb56da0c1925c07e8929ce4c9fe8aeafa7cb8c7b

---
Gregor Herburger (4):
      firmware: raspberrypi: reorder rpi_firmware_property_tag enum
      dt-bindings: raspberrypi,bcm2835-firmware: Include 'reboot-mode.yaml'
      firmware: raspberrypi: Add reboot mode support
      arm64: dts: broadcom: bcm2712: Add reboot modes to firmware node

 .../arm/bcm/raspberrypi,bcm2835-firmware.yaml      |  5 ++++-
 .../boot/dts/broadcom/bcm2712-rpi-5-b-base.dtsi    |  2 ++
 drivers/firmware/Kconfig                           |  1 +
 drivers/firmware/raspberrypi.c                     | 25 ++++++++++++++++++++++
 include/soc/bcm2835/raspberrypi-firmware.h         | 22 ++++++++++---------
 5 files changed, 44 insertions(+), 11 deletions(-)
---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260623-rpi-tryboot-4292c92b0727

Best regards,
--  
Gregor Herburger <gregor.herburger@linutronix.de>

[PATCH 1/4] firmware: raspberrypi: reorder rpi_firmware_property_tag enum

[Gregor Herburger]

The enum was once ordered by tags. The later added tags where added in
a different order. Reorder the tags again.

No functional change intended.

Signed-off-by: Gregor Herburger <gregor.herburger@linutronix.de>
---
 include/soc/bcm2835/raspberrypi-firmware.h | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index e1f87fbfe5542..66cc5a426c3c5 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -72,26 +72,26 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_GET_EDID_BLOCK =                         0x00030020,
 	RPI_FIRMWARE_GET_CUSTOMER_OTP =                       0x00030021,
 	RPI_FIRMWARE_GET_DOMAIN_STATE =                       0x00030030,
+	RPI_FIRMWARE_GET_GPIO_STATE =                         0x00030041,
+	RPI_FIRMWARE_GET_GPIO_CONFIG =                        0x00030043,
+	RPI_FIRMWARE_GET_PERIPH_REG =                         0x00030045,
 	RPI_FIRMWARE_GET_THROTTLED =                          0x00030046,
 	RPI_FIRMWARE_GET_CLOCK_MEASURED =                     0x00030047,
 	RPI_FIRMWARE_NOTIFY_REBOOT =                          0x00030048,
+	RPI_FIRMWARE_GET_POE_HAT_VAL =                        0x00030049,
+	RPI_FIRMWARE_SET_POE_HAT_VAL =                        0x00030050,
+	RPI_FIRMWARE_NOTIFY_XHCI_RESET =                      0x00030058,
+	RPI_FIRMWARE_NOTIFY_DISPLAY_DONE =                    0x00030066,
 	RPI_FIRMWARE_SET_CLOCK_STATE =                        0x00038001,
 	RPI_FIRMWARE_SET_CLOCK_RATE =                         0x00038002,
 	RPI_FIRMWARE_SET_VOLTAGE =                            0x00038003,
 	RPI_FIRMWARE_SET_TURBO =                              0x00038009,
 	RPI_FIRMWARE_SET_CUSTOMER_OTP =                       0x00038021,
 	RPI_FIRMWARE_SET_DOMAIN_STATE =                       0x00038030,
-	RPI_FIRMWARE_GET_GPIO_STATE =                         0x00030041,
 	RPI_FIRMWARE_SET_GPIO_STATE =                         0x00038041,
 	RPI_FIRMWARE_SET_SDHOST_CLOCK =                       0x00038042,
-	RPI_FIRMWARE_GET_GPIO_CONFIG =                        0x00030043,
 	RPI_FIRMWARE_SET_GPIO_CONFIG =                        0x00038043,
-	RPI_FIRMWARE_GET_PERIPH_REG =                         0x00030045,
 	RPI_FIRMWARE_SET_PERIPH_REG =                         0x00038045,
-	RPI_FIRMWARE_GET_POE_HAT_VAL =                        0x00030049,
-	RPI_FIRMWARE_SET_POE_HAT_VAL =                        0x00030050,
-	RPI_FIRMWARE_NOTIFY_XHCI_RESET =                      0x00030058,
-	RPI_FIRMWARE_NOTIFY_DISPLAY_DONE =                    0x00030066,
 
 	/* Dispmanx TAGS */
 	RPI_FIRMWARE_FRAMEBUFFER_ALLOCATE =                   0x00040001,
@@ -107,7 +107,6 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_FRAMEBUFFER_GET_PALETTE =                0x0004000b,
 	RPI_FIRMWARE_FRAMEBUFFER_GET_TOUCHBUF =               0x0004000f,
 	RPI_FIRMWARE_FRAMEBUFFER_GET_GPIOVIRTBUF =            0x00040010,
-	RPI_FIRMWARE_FRAMEBUFFER_RELEASE =                    0x00048001,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_PHYSICAL_WIDTH_HEIGHT = 0x00044003,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_VIRTUAL_WIDTH_HEIGHT =  0x00044004,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_DEPTH =                 0x00044005,
@@ -117,6 +116,7 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_OVERSCAN =              0x0004400a,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_PALETTE =               0x0004400b,
 	RPI_FIRMWARE_FRAMEBUFFER_TEST_VSYNC =                 0x0004400e,
+	RPI_FIRMWARE_FRAMEBUFFER_RELEASE =                    0x00048001,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_PHYSICAL_WIDTH_HEIGHT =  0x00048003,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_VIRTUAL_WIDTH_HEIGHT =   0x00048004,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_DEPTH =                  0x00048005,
@@ -125,10 +125,10 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_FRAMEBUFFER_SET_VIRTUAL_OFFSET =         0x00048009,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_OVERSCAN =               0x0004800a,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_PALETTE =                0x0004800b,
-	RPI_FIRMWARE_FRAMEBUFFER_SET_TOUCHBUF =               0x0004801f,
-	RPI_FIRMWARE_FRAMEBUFFER_SET_GPIOVIRTBUF =            0x00048020,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_VSYNC =                  0x0004800e,
 	RPI_FIRMWARE_FRAMEBUFFER_SET_BACKLIGHT =              0x0004800f,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_TOUCHBUF =               0x0004801f,
+	RPI_FIRMWARE_FRAMEBUFFER_SET_GPIOVIRTBUF =            0x00048020,
 
 	RPI_FIRMWARE_VCHIQ_INIT =                             0x00048010,
 

-- 
2.47.3

[PATCH 2/4] dt-bindings: raspberrypi,bcm2835-firmware: Include 'reboot-mode.yaml'

[Gregor Herburger]

The Raspberry Pi firmware allows to set a reboot mode called tryboot
that allows to try booting from a different partition to allow updating
of the boot partition. Allow reboot mode properties by referencing the
reboot-mode schema.

Signed-off-by: Gregor Herburger <gregor.herburger@linutronix.de>
---
 .../devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml    | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
index 983ea80eaec97..30b490e0d9fb3 100644
--- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -133,11 +133,14 @@ properties:
     required:
       - compatible
 
+allOf:
+  - $ref: /schemas/power/reset/reboot-mode.yaml#
+
 required:
   - compatible
   - mboxes
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |

-- 
2.47.3

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

[Gregor Herburger]

The Raspberry Pi firmware has a tryboot mode where it tries to boot from
another partition. This can be used to create a A/B update schema.

To enable this on the next boot the RPI_FIRMWARE_SET_REBOOT_FLAGS
message must be send to the firmware.

Add support for this by registering a reboot mode driver.

Signed-off-by: Gregor Herburger <gregor.herburger@linutronix.de>
---
 drivers/firmware/Kconfig                   |  1 +
 drivers/firmware/raspberrypi.c             | 25 +++++++++++++++++++++++++
 include/soc/bcm2835/raspberrypi-firmware.h |  2 ++
 3 files changed, 28 insertions(+)

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index bbd2155d84838..04bc8263e0017 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -115,6 +115,7 @@ config ISCSI_IBFT
 config RASPBERRYPI_FIRMWARE
 	tristate "Raspberry Pi Firmware Driver"
 	depends on BCM2835_MBOX
+	select REBOOT_MODE
 	help
 	  This option enables support for communicating with the firmware on the
 	  Raspberry Pi.
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index 0aa322e9a2e73..ba4f723751ef8 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/reboot-mode.h>
 #include <linux/slab.h>
 #include <soc/bcm2835/raspberrypi-firmware.h>
 
@@ -29,6 +30,7 @@ struct rpi_firmware {
 	struct mbox_client cl;
 	struct mbox_chan *chan; /* The property channel. */
 	struct completion c;
+	struct reboot_mode_driver reboot_mode;
 	u32 enabled;
 
 	struct kref consumers;
@@ -273,10 +275,25 @@ static void devm_rpi_firmware_put(void *data)
 	rpi_firmware_put(fw);
 }
 
+static int rpi_firmware_reboot_mode_write(struct reboot_mode_driver *reboot,
+					  unsigned int magic)
+{
+	struct rpi_firmware *fw = container_of(reboot, struct rpi_firmware,
+					       reboot_mode);
+	int ret = 0;
+
+	if (magic)
+		ret = rpi_firmware_property(fw, RPI_FIRMWARE_SET_REBOOT_FLAGS,
+					    &magic, sizeof(magic));
+
+	return ret;
+}
+
 static int rpi_firmware_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct rpi_firmware *fw;
+	int ret;
 
 	/*
 	 * Memory will be freed by rpi_firmware_delete() once all users have
@@ -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;
 }
 
@@ -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);
+
 	platform_device_unregister(rpi_hwmon);
 	rpi_hwmon = NULL;
 	platform_device_unregister(rpi_clk);
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index 66cc5a426c3c5..f905bff0fb3ea 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -81,6 +81,7 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_GET_POE_HAT_VAL =                        0x00030049,
 	RPI_FIRMWARE_SET_POE_HAT_VAL =                        0x00030050,
 	RPI_FIRMWARE_NOTIFY_XHCI_RESET =                      0x00030058,
+	RPI_FIRMWARE_GET_REBOOT_FLAGS =                       0x00030064,
 	RPI_FIRMWARE_NOTIFY_DISPLAY_DONE =                    0x00030066,
 	RPI_FIRMWARE_SET_CLOCK_STATE =                        0x00038001,
 	RPI_FIRMWARE_SET_CLOCK_RATE =                         0x00038002,
@@ -92,6 +93,7 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_SET_SDHOST_CLOCK =                       0x00038042,
 	RPI_FIRMWARE_SET_GPIO_CONFIG =                        0x00038043,
 	RPI_FIRMWARE_SET_PERIPH_REG =                         0x00038045,
+	RPI_FIRMWARE_SET_REBOOT_FLAGS =                       0x00038064,
 
 	/* Dispmanx TAGS */
 	RPI_FIRMWARE_FRAMEBUFFER_ALLOCATE =                   0x00040001,

-- 
2.47.3

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

[Gregor Herburger]

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

Signed-off-by: Gregor Herburger <gregor.herburger@linutronix.de>
---
 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b-base.dtsi | 2 ++
 1 file changed, 2 insertions(+)

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>;
 
 			firmware_clocks: clocks {
 				compatible = "raspberrypi,firmware-clocks";

-- 
2.47.3

Re: [PATCH 2/4] dt-bindings: raspberrypi,bcm2835-firmware: Include 'reboot-mode.yaml'

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing Device Tree Schema Restriction for reboot mode pattern properties
--

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

dt-bindings: raspberrypi,bcm2835-firmware: Include 'reboot-mode.yaml'

This commit updates the Raspberry Pi firmware binding to reference the
reboot-mode schema. This allows reboot mode properties to be used to set
reboot modes like tryboot.

> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> index 983ea80eaec97..30b490e0d9fb3 100644
> --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> @@ -133,11 +133,14 @@ properties:
>      required:
>        - compatible
>  
> +allOf:
> +  - $ref: /schemas/power/reset/reboot-mode.yaml#

[Severity: Low]
Should this schema restrict the reboot mode pattern properties to a single
item?

The generic reboot-mode.yaml schema defines ^mode-.*$ as a uint32-array
without a length restriction. Because this binding doesn't override it with
maxItems: 1, the device tree schema will accept arrays of arbitrary length,
for example:

mode-tryboot = <1 2>;

Since the reboot-mode.c driver only evaluates a single 32-bit value using
of_property_read_u32(), this could lead to silent truncation of multi-cell
properties without any validation errors. Does it make sense to add a
patternProperties block here to restrict maxItems to 1?

> +
>  required:
>    - compatible
>    - mboxes
>  
> -additionalProperties: false
> +unevaluatedProperties: false
>  
>  examples:
>    - |

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

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

[sashiko-bot]

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

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

[sashiko-bot]

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

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

[Gregor Herburger]

Hi,
On Fri, Jun 26, 2026 at 07:43:33AM +0000, sashiko-bot@kernel.org wrote:
> 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.
> --

yes I will address this points  in a v2. But first I want to wait if I get more
feedback on this series. In particular, whether this is the right approach from
a conceptual standpoint.

> 
> 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

-- 
Gregor Herburger
Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen
Phone: +49 7556 25 999 35; Fax.: +49 7556 25 999 99

Hinweise zum Datenschutz finden Sie hier (Informations on data privacy 
can be found here): https://linutronix.de/legal/data-protection.php

Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen | 
Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 
806 | Geschäftsführer (Managing Directors): Dr. Wilfried Wessner, 
Katharina Kopp, Alexander Gieringer

Re: [PATCH 2/4] dt-bindings: raspberrypi,bcm2835-firmware: Include 'reboot-mode.yaml'

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

On Fri, Jun 26, 2026 at 09:35:05AM +0200, Gregor Herburger wrote:
> The Raspberry Pi firmware allows to set a reboot mode called tryboot
> that allows to try booting from a different partition to allow updating
> of the boot partition. Allow reboot mode properties by referencing the
> reboot-mode schema.
> 
> Signed-off-by: Gregor Herburger <gregor.herburger@linutronix.de>
> ---
>  .../devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml    | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> index 983ea80eaec97..30b490e0d9fb3 100644
> --- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
> @@ -133,11 +133,14 @@ properties:
>      required:
>        - compatible
>  
> +allOf:
> +  - $ref: /schemas/power/reset/reboot-mode.yaml#
> +
>  required:
>    - compatible
>    - mboxes
>  
> -additionalProperties: false
> +unevaluatedProperties: false

I think you should keep additionalProperties: false and add
mode-normal: true
mode-tryboot: true

(I don't know if the latter works though, you may need to have a $ref to
uint32-array).

What you've done permits freeform reboot modes, but I think only normal
and tryboot are valid?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]