[PATCH] drm/vc4: Fix firmware reference leak in vc4_hvs_bind()

[PATCH] drm/vc4: Fix firmware reference leak in vc4_hvs_bind()

[Wentao Liang]

The firmware reference taken via rpi_firmware_get() is leaked when
either of the devm_clk_get() calls for the core or display clocks
fails, as both error paths return without releasing it.

Put the firmware reference before returning from both clock lookup
error paths.

Fixes: 2a001ca00ad5 ("drm/vc4: hdmi: Rework hdmi_enable_4kp60 detection code")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/gpu/drm/vc4/vc4_hvs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index ee8d0738501b..20dde4150068 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -1680,6 +1680,7 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 					     (vc4->gen >= VC4_GEN_6_C) ? "core" : NULL);
 		if (IS_ERR(hvs->core_clk)) {
 			dev_err(&pdev->dev, "Couldn't get core clock\n");
+			rpi_firmware_put(firmware);
 			return PTR_ERR(hvs->core_clk);
 		}
 
@@ -1687,6 +1688,7 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 					     (vc4->gen >= VC4_GEN_6_C) ? "disp" : NULL);
 		if (IS_ERR(hvs->disp_clk)) {
 			dev_err(&pdev->dev, "Couldn't get disp clock\n");
+			rpi_firmware_put(firmware);
 			return PTR_ERR(hvs->disp_clk);
 		}
 
-- 
2.34.1

Re: [PATCH] drm/vc4: Fix firmware reference leak in vc4_hvs_bind()

[Maíra Canal]

Hi Wentao,

On 16/09/26 12:41, Wentao Liang wrote:
> The firmware reference taken via rpi_firmware_get() is leaked when
> either of the devm_clk_get() calls for the core or display clocks
> fails, as both error paths return without releasing it.
> 
> Put the firmware reference before returning from both clock lookup
> error paths.
> 
> Fixes: 2a001ca00ad5 ("drm/vc4: hdmi: Rework hdmi_enable_4kp60 detection code")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>   drivers/gpu/drm/vc4/vc4_hvs.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index ee8d0738501b..20dde4150068 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -1680,6 +1680,7 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
>   					     (vc4->gen >= VC4_GEN_6_C) ? "core" : NULL);
>   		if (IS_ERR(hvs->core_clk)) {
>   			dev_err(&pdev->dev, "Couldn't get core clock\n");
> +			rpi_firmware_put(firmware);

Instead of adding this line twice, move the line `firmware =
rpi_firmware_get(node)` closer to its actual use. Then, you don't need
to add these `rpi_firmware_put()` calls.

The same comment applies to your other PATCH: `[PATCH] drm/vc4: Fix
firmware reference leak in vc4_drm_bind()`.

Best regards,
- Maíra

>   			return PTR_ERR(hvs->core_clk);
>   		}
>   
> @@ -1687,6 +1688,7 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
>   					     (vc4->gen >= VC4_GEN_6_C) ? "disp" : NULL);
>   		if (IS_ERR(hvs->disp_clk)) {
>   			dev_err(&pdev->dev, "Couldn't get disp clock\n");
> +			rpi_firmware_put(firmware);
>   			return PTR_ERR(hvs->disp_clk);
>   		}
>   

Re: [PATCH] drm/vc4: Fix firmware reference leak in vc4_hvs_bind()

[Wentao Liang]

Hi Maíra,

Sorry for the very late reply, and thanks for the review.

You are right, and your suggestion is better than what I had.  Moving
the firmware lookup below the clock lookups keeps the reference close to
its use, and the two rpi_firmware_put() calls in the error paths are not
needed anymore.  The rpi_firmware_find_node() call moves with it, since
the device node reference it returns would otherwise be leaked by those
same error paths.  I am sending v2 with that change.

About the other patch, "drm/vc4: Fix firmware reference leak in
vc4_drm_bind()": the same change would work there, since the only thing
between the firmware lookup and rpi_firmware_property() is
aperture_remove_all_conflicting_devices(), which does not use the
reference.  That patch was already applied as b816da5d5df1 before this
comment, so it is in the tree as posted, with the extra
rpi_firmware_put() in the aperture error path.

Thanks,
Wentao

[PATCH] drm/vc4: Fix firmware reference leak in vc4_hvs_bind()

[Wentao Liang]

The firmware reference taken via rpi_firmware_get() is leaked when
either of the devm_clk_get() calls for the core or display clocks
fails, as both error paths return without releasing it.

Fix it by moving the firmware lookup below the clock lookups, so that
the reference is taken right before the firmware property is read and
released immediately afterwards.  The rpi_firmware_find_node() call
moves with it: the device node reference it returns would otherwise be
leaked by those same error paths.

The candidate was found by our static analysis tooling, and this change
was prepared with assistance from DeepSeek-V4.1-Flash and GPT-6.1-Sol.

Fixes: 2a001ca00ad5 ("drm/vc4: hdmi: Rework hdmi_enable_4kp60 detection code")
Cc: stable@vger.kernel.org
Suggested-by: Maíra Canal <mcanal@igalia.com>
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
Changes in v2:
- Sorry for the very late reply, and thanks for the review.
- Move the firmware and device node lookups below the clock lookups
  instead of adding rpi_firmware_put() to the two error paths, as
  suggested by Maíra.
- Link to v1: https://lore.kernel.org/all/20260916154137.2058618-1-vulab@iscas.ac.cn/
---
 drivers/gpu/drm/vc4/vc4_hvs.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index b1fb096858cc..537aaa992129 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -1677,15 +1677,6 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 		struct device_node *node;
 		unsigned int max_rate;
 
-		node = rpi_firmware_find_node();
-		if (!node)
-			return -EINVAL;
-
-		firmware = rpi_firmware_get(node);
-		of_node_put(node);
-		if (!firmware)
-			return -EPROBE_DEFER;
-
 		hvs->core_clk = devm_clk_get(&pdev->dev,
 					     (vc4->gen >= VC4_GEN_6_C) ? "core" : NULL);
 		if (IS_ERR(hvs->core_clk)) {
@@ -1700,6 +1691,15 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 			return PTR_ERR(hvs->disp_clk);
 		}
 
+		node = rpi_firmware_find_node();
+		if (!node)
+			return -EINVAL;
+
+		firmware = rpi_firmware_get(node);
+		of_node_put(node);
+		if (!firmware)
+			return -EPROBE_DEFER;
+
 		max_rate = rpi_firmware_clk_get_max_rate(firmware,
 							 RPI_FIRMWARE_CORE_CLK_ID);
 		rpi_firmware_put(firmware);
-- 
2.34.1