[PATCH v2 3/3] drm/display: bridge_connector: get/put the panel_bridge

Luca Ceresoli posted 3 patches 3 months, 3 weeks ago
[PATCH v2 3/3] drm/display: bridge_connector: get/put the panel_bridge
Posted by Luca Ceresoli 3 months, 3 weeks ago
The panel_bridge pointer is taken inside the loop and used after the
loop. Being a local variable, use a cleanup action to ensure it is put on
return.

Based on the code structure the panel_bridge pointer might be assigned
during multiple loop iterations. Even though this is probably not possible
in the practice, ensure there is no reference leak by putting the reference
to the old value before overwriting with the new value.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

This patch was added in v2:
- taking the panel_bridge specific code from the original (buggy) patch,
  and split it for clarity from the larger patch covering stored bridge
  pointers
- comapred to the original code, added drm_bridge_put() for extra safety
  even though likely not necessary
---
 drivers/gpu/drm/display/drm_bridge_connector.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 95ccf86527129edaa6fcc75c6202985e73c46da8..a2d30cf9e06df44b89456b5aba8198ee1e5d5601 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -652,7 +652,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 	struct drm_bridge_connector *bridge_connector;
 	struct drm_connector *connector;
 	struct i2c_adapter *ddc = NULL;
-	struct drm_bridge *panel_bridge = NULL;
+	struct drm_bridge *panel_bridge __free(drm_bridge_put) = NULL;
 	unsigned int supported_formats = BIT(HDMI_COLORSPACE_RGB);
 	unsigned int max_bpc = 8;
 	bool support_hdcp = false;
@@ -787,8 +787,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 		if (bridge->ddc)
 			ddc = bridge->ddc;
 
-		if (drm_bridge_is_panel(bridge))
-			panel_bridge = bridge;
+		if (drm_bridge_is_panel(bridge)) {
+			drm_bridge_put(panel_bridge);
+			panel_bridge = drm_bridge_get(bridge);
+		}
 
 		if (bridge->support_hdcp)
 			support_hdcp = true;

-- 
2.51.0
Re: [PATCH v2 3/3] drm/display: bridge_connector: get/put the panel_bridge
Posted by Louis Chauvet 3 months, 1 week ago

Le 17/10/2025 à 18:15, Luca Ceresoli a écrit :
> The panel_bridge pointer is taken inside the loop and used after the
> loop. Being a local variable, use a cleanup action to ensure it is put on
> return.
> 
> Based on the code structure the panel_bridge pointer might be assigned
> during multiple loop iterations. Even though this is probably not possible
> in the practice, ensure there is no reference leak by putting the reference
> to the old value before overwriting with the new value.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> ---
> 
> This patch was added in v2:
> - taking the panel_bridge specific code from the original (buggy) patch,
>    and split it for clarity from the larger patch covering stored bridge
>    pointers
> - comapred to the original code, added drm_bridge_put() for extra safety
>    even though likely not necessary
> ---
>   drivers/gpu/drm/display/drm_bridge_connector.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 95ccf86527129edaa6fcc75c6202985e73c46da8..a2d30cf9e06df44b89456b5aba8198ee1e5d5601 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -652,7 +652,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>   	struct drm_bridge_connector *bridge_connector;
>   	struct drm_connector *connector;
>   	struct i2c_adapter *ddc = NULL;
> -	struct drm_bridge *panel_bridge = NULL;
> +	struct drm_bridge *panel_bridge __free(drm_bridge_put) = NULL;
>   	unsigned int supported_formats = BIT(HDMI_COLORSPACE_RGB);
>   	unsigned int max_bpc = 8;
>   	bool support_hdcp = false;
> @@ -787,8 +787,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>   		if (bridge->ddc)
>   			ddc = bridge->ddc;
>   
> -		if (drm_bridge_is_panel(bridge))
> -			panel_bridge = bridge;
> +		if (drm_bridge_is_panel(bridge)) {
> +			drm_bridge_put(panel_bridge);
> +			panel_bridge = drm_bridge_get(bridge);
> +		}
>   
>   		if (bridge->support_hdcp)
>   			support_hdcp = true;
> 

-- 
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com