Re: [PATCH v10 3/3] rtc: Add Raspberry Pi 5 RTC driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kernel panic due to passing ERR_PTR to platform_device_unregister
--

commit 871d9696731b8e115094c715d59226e0d29be3d2
Author: Sander Speetjens <sander.speetjens@gmail.com>

rtc: Add Raspberry Pi 5 RTC driver

This commit adds support for the Raspberry Pi 5 RTC by registering a new
platform device through the existing BCM2835 firmware driver.

> diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
> --- a/drivers/firmware/raspberrypi.c
> +++ b/drivers/firmware/raspberrypi.c
> @@ -231,6 +232,26 @@ static void rpi_register_clk_driver(struct device *dev)
[ ... ]
> +static void rpi_register_rtc_driver(struct device *dev)
> +{
> +	struct device_node *firmware;
> +
> +	// Check if our model of Raspberry Pi board is from the 5th gen
> +	// as the RTC is only present on that model.
> +	if (!of_machine_is_compatible("brcm,bcm2712"))
> +		return;
> +
> +	firmware = of_get_compatible_child(dev->of_node,
> +					   "raspberrypi,firmware-rtc");
> +	if (firmware) {
> +		of_node_put(firmware);
> +		return;
> +	}
> +
> +	rpi_rtc = platform_device_register_data(dev, "raspberrypi-rtc",
> +						-1, NULL, 0);
> +}

[Severity: High]
If platform_device_register_data() fails (for example, due to an out of
memory condition), it returns an ERR_PTR. Is it safe to store this error
pointer directly in the global rpi_rtc variable without an IS_ERR() check?

[ ... ]

> @@ -327,6 +349,8 @@ static void rpi_firmware_remove(struct platform_device *pdev)
>  	rpi_hwmon = NULL;
>  	platform_device_unregister(rpi_clk);
>  	rpi_clk = NULL;
> +	platform_device_unregister(rpi_rtc);
> +	rpi_rtc = NULL;

[Severity: High]
Can this cause a kernel panic during unbind or module removal?

If platform_device_register_data() previously failed and returned an
ERR_PTR, rpi_firmware_probe() still returns 0 (success) so the firmware
driver binds successfully. Later, when rpi_firmware_remove() is called,
platform_device_unregister() is invoked on rpi_rtc.

While platform_device_unregister() safely ignores NULL pointers, it
attempts to dereference ERR_PTRs, which would lead to an invalid memory
access. Does this path need a check to ensure rpi_rtc is valid before
unregistering?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002102419.830595-1-sander.speetjens@gmail.com?part=3