* [PATCH] drm/msm/dpu: Fix memory leak in msm_mdss_parse_data_bus_icc_path
@ 2022-12-06 7:55 Miaoqian Lin
2022-12-06 15:40 ` Doug Anderson
0 siblings, 1 reply; 2+ messages in thread
From: Miaoqian Lin @ 2022-12-06 7:55 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
David Airlie, Daniel Vetter, Stephen Boyd, Douglas Anderson,
linux-arm-msm, dri-devel, freedreno, linux-kernel
Cc: linmq006
of_icc_get() alloc resources for path1, we should release it when not
need anymore. Early return when IS_ERR_OR_NULL(path0) may leak path1.
Add icc_put(path1) in the error path to fix this.
Fixes: b9364eed9232 ("drm/msm/dpu: Move min BW request and full BW disable back to mdss")
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
drivers/gpu/drm/msm/msm_mdss.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index e13c5c12b775..a38fa9a9a3d6 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -49,8 +49,10 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
struct icc_path *path0 = of_icc_get(dev, "mdp0-mem");
struct icc_path *path1 = of_icc_get(dev, "mdp1-mem");
- if (IS_ERR_OR_NULL(path0))
+ if (IS_ERR_OR_NULL(path0)) {
+ icc_put(path1);
return PTR_ERR_OR_ZERO(path0);
+ }
msm_mdss->path[0] = path0;
msm_mdss->num_paths = 1;
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm/msm/dpu: Fix memory leak in msm_mdss_parse_data_bus_icc_path
2022-12-06 7:55 [PATCH] drm/msm/dpu: Fix memory leak in msm_mdss_parse_data_bus_icc_path Miaoqian Lin
@ 2022-12-06 15:40 ` Doug Anderson
0 siblings, 0 replies; 2+ messages in thread
From: Doug Anderson @ 2022-12-06 15:40 UTC (permalink / raw)
To: Miaoqian Lin
Cc: freedreno, Sean Paul, Abhinav Kumar, dri-devel, Stephen Boyd,
linux-arm-msm, Dmitry Baryshkov, linux-kernel
Hi,
On Mon, Dec 5, 2022 at 11:55 PM Miaoqian Lin <linmq006@gmail.com> wrote:
>
> of_icc_get() alloc resources for path1, we should release it when not
> need anymore. Early return when IS_ERR_OR_NULL(path0) may leak path1.
> Add icc_put(path1) in the error path to fix this.
>
> Fixes: b9364eed9232 ("drm/msm/dpu: Move min BW request and full BW disable back to mdss")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
> drivers/gpu/drm/msm/msm_mdss.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index e13c5c12b775..a38fa9a9a3d6 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -49,8 +49,10 @@ static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
> struct icc_path *path0 = of_icc_get(dev, "mdp0-mem");
> struct icc_path *path1 = of_icc_get(dev, "mdp1-mem");
>
> - if (IS_ERR_OR_NULL(path0))
> + if (IS_ERR_OR_NULL(path0)) {
> + icc_put(path1);
> return PTR_ERR_OR_ZERO(path0);
> + }
>
> msm_mdss->path[0] = path0;
> msm_mdss->num_paths = 1;
Hmmm. I guess the original author of the code (which wasn't me--I just
restored the code that was deleted by a previous change) was assuming
that if mdp0-mem had a problem that mdp1-mem would also have a
problem. That would mean that you wouldn't need to call icc_put() on
it.
...and, in fact, your patch doesn't handle that case, does it? If
_both_ of the two are error or NULL then you'll be calling icc_put()
on something invalid. I guess icc_put() handles those cases without
crashing but it will give a WARN_ON() splat if it happens to be an
error...
Really, there's a better solution anyway. Instead, you should do:
path0 = of_icc_get(dev, "mdp0-mem");
if (IS_ERR_OR_NULL(path0))
return PTR_ERR_OR_ZERO(path0);
msm_mdss->path[0] = path0;
msm_mdss->num_paths = 1;
path1 = of_icc_get(dev, "mdp1-mem");
if (!IS_ERR_OR_NULL(path1)) {
...
}
In other words just defer getting path1 until after you've checked
path0 for an error.
-Doug
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-12-06 15:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-06 7:55 [PATCH] drm/msm/dpu: Fix memory leak in msm_mdss_parse_data_bus_icc_path Miaoqian Lin
2022-12-06 15:40 ` Doug Anderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).