linux-kernel - Re: [PATCH 5/7] drm/msm/dp: fix bridge lifetime

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <75540907-6981-5798-efe3-04fe0466da31@linaro.org>
Date:   Mon, 12 Sep 2022 21:11:09 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Johan Hovold <johan+linaro@...nel.org>,
        Douglas Anderson <dianders@...omium.org>,
        Rob Clark <robdclark@...il.com>
Cc:     Andrzej Hajda <andrzej.hajda@...el.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Robert Foss <robert.foss@...aro.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>, Sean Paul <sean@...rly.run>,
        Stephen Boyd <swboyd@...omium.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        dri-devel@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Subject: Re: [PATCH 5/7] drm/msm/dp: fix bridge lifetime

On 12/09/2022 18:40, Johan Hovold wrote:
> Device-managed resources allocated post component bind must be tied to
> the lifetime of the aggregate DRM device or they will not necessarily be
> released when binding of the aggregate device is deferred.
> 
> This can lead resource leaks or failure to bind the aggregate device
> when binding is later retried and a second attempt to allocate the
> resources is made.
> 
> For the DP bridges, previously allocated bridges will leak on probe
> deferral.
> 
> Fix this by amending the DP parser interface and tying the lifetime of
> the bridge device to the DRM device rather than DP platform device.
> 
> Fixes: c3bf8e21b38a ("drm/msm/dp: Add eDP support via aux_bus")
> Cc: stable@...r.kernel.org      # 5.19
> Signed-off-by: Johan Hovold <johan+linaro@...nel.org>

Seems logical enough.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>

> ---
>   drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
>   drivers/gpu/drm/msm/dp/dp_parser.c  | 6 +++---
>   drivers/gpu/drm/msm/dp/dp_parser.h  | 5 +++--
>   3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index e1aa6355bbf6..393af1ea9ed8 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1576,7 +1576,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>   	 * For DisplayPort interfaces external bridges are optional, so
>   	 * silently ignore an error if one is not present (-ENODEV).
>   	 */
> -	rc = dp_parser_find_next_bridge(dp_priv->parser);
> +	rc = devm_dp_parser_find_next_bridge(dp->drm_dev->dev, dp_priv->parser);
>   	if (!dp->is_edp && rc == -ENODEV)
>   		return 0;
>   
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
> index dd732215d55b..dcbe893d66d7 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
> @@ -240,12 +240,12 @@ static int dp_parser_clock(struct dp_parser *parser)
>   	return 0;
>   }
>   
> -int dp_parser_find_next_bridge(struct dp_parser *parser)
> +int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser)
>   {
> -	struct device *dev = &parser->pdev->dev;
> +	struct platform_device *pdev = parser->pdev;
>   	struct drm_bridge *bridge;
>   
> -	bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> +	bridge = devm_drm_of_get_bridge(dev, pdev->dev.of_node, 1, 0);
>   	if (IS_ERR(bridge))
>   		return PTR_ERR(bridge);
>   
> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h b/drivers/gpu/drm/msm/dp/dp_parser.h
> index 866c1a82bf1a..d30ab773db46 100644
> --- a/drivers/gpu/drm/msm/dp/dp_parser.h
> +++ b/drivers/gpu/drm/msm/dp/dp_parser.h
> @@ -138,8 +138,9 @@ struct dp_parser {
>   struct dp_parser *dp_parser_get(struct platform_device *pdev);
>   
>   /**
> - * dp_parser_find_next_bridge() - find an additional bridge to DP
> + * devm_dp_parser_find_next_bridge() - find an additional bridge to DP
>    *
> + * @dev: device to tie bridge lifetime to
>    * @parser: dp_parser data from client
>    *
>    * This function is used to find any additional bridge attached to
> @@ -147,6 +148,6 @@ struct dp_parser *dp_parser_get(struct platform_device *pdev);
>    *
>    * Return: 0 if able to get the bridge, otherwise negative errno for failure.
>    */
> -int dp_parser_find_next_bridge(struct dp_parser *parser);
> +int devm_dp_parser_find_next_bridge(struct device *dev, struct dp_parser *parser);
>   
>   #endif

-- 
With best wishes
Dmitry