linux-kernel - Re: [Freedreno] [PATCH v2 07/10] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <a77be457-7009-02bc-6a67-33b2cd22429a@quicinc.com>
Date:   Thu, 22 Sep 2022 12:56:26 -0700
From:   Kuogee Hsieh <quic_khsieh@...cinc.com>
To:     Johan Hovold <johan+linaro@...nel.org>,
        Douglas Anderson <dianders@...omium.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        "Rob Clark" <robdclark@...il.com>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>
CC:     <dri-devel@...ts.freedesktop.org>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Daniel Vetter <daniel@...ll.ch>,
        Jonas Karlman <jonas@...boo.se>,
        <linux-arm-msm@...r.kernel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        <freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Stephen Boyd <swboyd@...omium.org>,
        Robert Foss <robert.foss@...aro.org>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        <stable@...r.kernel.org>, Sean Paul <sean@...rly.run>,
        Steev Klimaszewski <steev@...i.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>
Subject: Re: [Freedreno] [PATCH v2 07/10] drm/msm/dp: fix bridge lifetime


On 9/13/2022 1:53 AM, 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
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> Signed-off-by: Johan Hovold <johan+linaro@...nel.org>

Tested-by: Kuogee Hsieh <quic_khsieh@...cinc.com>

Reviewed-by: Kuogee Hsieh <quic_khsieh@...cinc.com>

> ---
>   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 4b0a2d4bb61e..808a516e84c5 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1586,7 +1586,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