[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <15016b7c-f333-4a38-8780-79bbc9ed1b0f@linaro.org>
Date: Tue, 9 Jul 2024 11:06:08 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Yao Zi <ziyao@...root.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
Kevin Hilman <khilman@...libre.com>, Jerome Brunet <jbrunet@...libre.com>,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
Maxime Jourdan <mjourdan@...libre.com>
Cc: dri-devel@...ts.freedesktop.org, linux-amlogic@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/meson: fix canvas release in bind function
On 03/07/2024 17:58, Yao Zi wrote:
> Allocated canvases may not be released on the error exit path of
> meson_drv_bind_master(), leading to resource leaking. Rewrite exit path
> to release canvases on error.
>
> Fixes: 2bf6b5b0e374 ("drm/meson: exclusively use the canvas provider module")
> Signed-off-by: Yao Zi <ziyao@...root.org>
> ---
> drivers/gpu/drm/meson/meson_drv.c | 37 +++++++++++++++----------------
> 1 file changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 17a5cca007e2..4bd0baa2a4f5 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -250,29 +250,20 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> if (ret)
> goto free_drm;
> ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_0);
> - if (ret) {
> - meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> - goto free_drm;
> - }
> + if (ret)
> + goto free_canvas_osd1;
> ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_1);
> - if (ret) {
> - meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> - meson_canvas_free(priv->canvas, priv->canvas_id_vd1_0);
> - goto free_drm;
> - }
> + if (ret)
> + goto free_canvas_vd1_0;
> ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_vd1_2);
> - if (ret) {
> - meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> - meson_canvas_free(priv->canvas, priv->canvas_id_vd1_0);
> - meson_canvas_free(priv->canvas, priv->canvas_id_vd1_1);
> - goto free_drm;
> - }
> + if (ret)
> + goto free_canvas_vd1_1;
>
> priv->vsync_irq = platform_get_irq(pdev, 0);
>
> ret = drm_vblank_init(drm, 1);
> if (ret)
> - goto free_drm;
> + goto free_canvas_vd1_2;
>
> /* Assign limits per soc revision/package */
> for (i = 0 ; i < ARRAY_SIZE(meson_drm_soc_attrs) ; ++i) {
> @@ -288,11 +279,11 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> */
> ret = drm_aperture_remove_framebuffers(&meson_driver);
> if (ret)
> - goto free_drm;
> + goto free_canvas_vd1_2;
>
> ret = drmm_mode_config_init(drm);
> if (ret)
> - goto free_drm;
> + goto free_canvas_vd1_2;
> drm->mode_config.max_width = 3840;
> drm->mode_config.max_height = 2160;
> drm->mode_config.funcs = &meson_mode_config_funcs;
> @@ -307,7 +298,7 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> if (priv->afbcd.ops) {
> ret = priv->afbcd.ops->init(priv);
> if (ret)
> - goto free_drm;
> + goto free_canvas_vd1_2;
> }
>
> /* Encoder Initialization */
> @@ -371,6 +362,14 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> exit_afbcd:
> if (priv->afbcd.ops)
> priv->afbcd.ops->exit(priv);
> +free_canvas_vd1_2:
> + meson_canvas_free(priv->canvas, priv->canvas_id_vd1_2);
> +free_canvas_vd1_1:
> + meson_canvas_free(priv->canvas, priv->canvas_id_vd1_1);
> +free_canvas_vd1_0:
> + meson_canvas_free(priv->canvas, priv->canvas_id_vd1_0);
> +free_canvas_osd1:
> + meson_canvas_free(priv->canvas, priv->canvas_id_osd1);
> free_drm:
> drm_dev_put(drm);
>
This would deserve a devm_meson_canvas_alloc(), but as-is:
Reviewed-by: Neil Armstrong <neil.armstrong@...aro.org>
Powered by blists - more mailing lists