Re: [PATCH 1/2] drm/imx: Fix error handling for a kmemdup() call in imx_pd_bind()

[Navid Emamdoost]

On Sat, Oct 12, 2019 at 4:07 AM Markus Elfring  wrote:
>
> From: Markus Elfring 
> Date: Sat, 12 Oct 2019 10:30:21 +0200
>
> The return value from a call of the function “kmemdup” was not checked
> in this function implementation. Thus add the corresponding error handling.
>
> Fixes: 19022aaae677dfa171a719e9d1ff04823ce65a65 ("staging: drm/imx: Add 
> parallel display support")
> Signed-off-by: Markus Elfring 
> ---
>  drivers/gpu/drm/imx/parallel-display.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/imx/parallel-display.c 
> b/drivers/gpu/drm/imx/parallel-display.c
> index 35518e5de356..39c4798f56b6 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -210,8 +210,13 @@ static int imx_pd_bind(struct device *dev, struct device 
> *master, void *data)
> return -ENOMEM;
>
> edidp = of_get_property(np, "edid", &imxpd->edid_len);
> -   if (edidp)
> +   if (edidp) {
> imxpd->edid = kmemdup(edidp, imxpd->edid_len, GFP_KERNEL);
> +   if (!imxpd->edid) {
> +   devm_kfree(dev, imxpd);

You should not try to free imxpd here as it is a resource-managed
allocation via devm_kzalloc(). It means memory allocated with this
function is
 automatically freed on driver detach. So, this patch introduces a double-free.

> +   return -ENOMEM;
> +   }
> +   }
>
> ret = of_property_read_string(np, "interface-pix-fmt", &fmt);
> if (!ret) {
> --
> 2.23.0
>


-- 
Navid.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/imx: Fix error handling for a kmemdup() call in imx_pd_bind()

[Julia Lawall]

On Sat, 12 Oct 2019, Navid Emamdoost wrote:

> On Sat, Oct 12, 2019 at 4:07 AM Markus Elfring  wrote:
> >
> > From: Markus Elfring 
> > Date: Sat, 12 Oct 2019 10:30:21 +0200
> >
> > The return value from a call of the function “kmemdup” was not checked
> > in this function implementation. Thus add the corresponding error handling.
> >
> > Fixes: 19022aaae677dfa171a719e9d1ff04823ce65a65 ("staging: drm/imx: Add 
> > parallel display support")
> > Signed-off-by: Markus Elfring 
> > ---
> >  drivers/gpu/drm/imx/parallel-display.c | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/imx/parallel-display.c 
> > b/drivers/gpu/drm/imx/parallel-display.c
> > index 35518e5de356..39c4798f56b6 100644
> > --- a/drivers/gpu/drm/imx/parallel-display.c
> > +++ b/drivers/gpu/drm/imx/parallel-display.c
> > @@ -210,8 +210,13 @@ static int imx_pd_bind(struct device *dev, struct 
> > device *master, void *data)
> > return -ENOMEM;
> >
> > edidp = of_get_property(np, "edid", &imxpd->edid_len);
> > -   if (edidp)
> > +   if (edidp) {
> > imxpd->edid = kmemdup(edidp, imxpd->edid_len, GFP_KERNEL);
> > +   if (!imxpd->edid) {
> > +   devm_kfree(dev, imxpd);
>
> You should not try to free imxpd here as it is a resource-managed
> allocation via devm_kzalloc(). It means memory allocated with this
> function is
>  automatically freed on driver detach. So, this patch introduces a 
> double-free.

No, it's not double freed since the proposed code frees it with a devm
function, removing it from the list of things to free later.  One can
wonder why the free has to be made apparent, though.

julia

>
> > +   return -ENOMEM;
> > +   }
> > +   }
> >
> > ret = of_property_read_string(np, "interface-pix-fmt", &fmt);
> > if (!ret) {
> > --
> > 2.23.0
> >
>
>
> --
> Navid.
>___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/imx: Fix error handling for a kmemdup() call in imx_pd_bind()

[Markus Elfring]

From: Markus Elfring 
Date: Sat, 12 Oct 2019 10:30:21 +0200

The return value from a call of the function “kmemdup” was not checked
in this function implementation. Thus add the corresponding error handling.

Fixes: 19022aaae677dfa171a719e9d1ff04823ce65a65 ("staging: drm/imx: Add 
parallel display support")
Signed-off-by: Markus Elfring 
---
 drivers/gpu/drm/imx/parallel-display.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/parallel-display.c 
b/drivers/gpu/drm/imx/parallel-display.c
index 35518e5de356..39c4798f56b6 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -210,8 +210,13 @@ static int imx_pd_bind(struct device *dev, struct device 
*master, void *data)
return -ENOMEM;

edidp = of_get_property(np, "edid", &imxpd->edid_len);
-   if (edidp)
+   if (edidp) {
imxpd->edid = kmemdup(edidp, imxpd->edid_len, GFP_KERNEL);
+   if (!imxpd->edid) {
+   devm_kfree(dev, imxpd);
+   return -ENOMEM;
+   }
+   }

ret = of_property_read_string(np, "interface-pix-fmt", &fmt);
if (!ret) {
--
2.23.0