All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: davinci: vpif_display: Mix memory leak on probe error path
@ 2018-07-27 11:52 Anton Vasilyev
  2018-08-02  9:51 ` Lad, Prabhakar
  0 siblings, 1 reply; 6+ messages in thread
From: Anton Vasilyev @ 2018-07-27 11:52 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Anton Vasilyev, Mauro Carvalho Chehab, linux-media, linux-kernel,
	ldv-project

If vpif_probe() fails on v4l2_device_register() then memory allocated
at initialize_vpif() for global vpif_obj.dev[i] become unreleased.

The patch adds deallocation of vpif_obj.dev[i] on the error path and
removes duplicated check on platform_data presence.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
---
 drivers/media/platform/davinci/vpif_display.c | 24 ++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 7be636237acf..0f324055cc9f 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1114,6 +1114,14 @@ static int initialize_vpif(void)
 	return err;
 }
 
+static void free_vpif_objs(void)
+{
+	int i;
+
+	for (i = 0; i < VPIF_DISPLAY_MAX_DEVICES; i++)
+		kfree(vpif_obj.dev[i]);
+}
+
 static int vpif_async_bound(struct v4l2_async_notifier *notifier,
 			    struct v4l2_subdev *subdev,
 			    struct v4l2_async_subdev *asd)
@@ -1255,11 +1263,6 @@ static __init int vpif_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	if (!pdev->dev.platform_data) {
-		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
-		return -EINVAL;
-	}
-
 	vpif_dev = &pdev->dev;
 	err = initialize_vpif();
 
@@ -1271,7 +1274,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 	err = v4l2_device_register(vpif_dev, &vpif_obj.v4l2_dev);
 	if (err) {
 		v4l2_err(vpif_dev->driver, "Error registering v4l2 device\n");
-		return err;
+		goto vpif_free;
 	}
 
 	while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, res_idx))) {
@@ -1314,7 +1317,10 @@ static __init int vpif_probe(struct platform_device *pdev)
 			if (vpif_obj.sd[i])
 				vpif_obj.sd[i]->grp_id = 1 << i;
 		}
-		vpif_probe_complete();
+		err = vpif_probe_complete();
+		if (err) {
+			goto probe_subdev_out;
+		}
 	} else {
 		vpif_obj.notifier.subdevs = vpif_obj.config->asd;
 		vpif_obj.notifier.num_subdevs = vpif_obj.config->asd_sizes[0];
@@ -1334,6 +1340,8 @@ static __init int vpif_probe(struct platform_device *pdev)
 	kfree(vpif_obj.sd);
 vpif_unregister:
 	v4l2_device_unregister(&vpif_obj.v4l2_dev);
+vpif_free:
+	free_vpif_objs();
 
 	return err;
 }
@@ -1355,8 +1363,8 @@ static int vpif_remove(struct platform_device *device)
 		ch = vpif_obj.dev[i];
 		/* Unregister video device */
 		video_unregister_device(&ch->video_dev);
-		kfree(vpif_obj.dev[i]);
 	}
+	free_vpif_objs();
 
 	return 0;
 }
-- 
2.18.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] media: davinci: vpif_display: Mix memory leak on probe error path
  2018-07-27 11:52 [PATCH] media: davinci: vpif_display: Mix memory leak on probe error path Anton Vasilyev
@ 2018-08-02  9:51 ` Lad, Prabhakar
  2018-08-06 15:50   ` [PATCH v2 1/2] " Anton Vasilyev
  0 siblings, 1 reply; 6+ messages in thread
From: Lad, Prabhakar @ 2018-08-02  9:51 UTC (permalink / raw)
  To: Anton Vasilyev; +Cc: Mauro Carvalho Chehab, linux-media, LKML, ldv-project

Hi,

thank you for the patch.

On Fri, Jul 27, 2018 at 12:52 PM, Anton Vasilyev <vasilyev@ispras.ru> wrote:
> If vpif_probe() fails on v4l2_device_register() then memory allocated
> at initialize_vpif() for global vpif_obj.dev[i] become unreleased.
>
> The patch adds deallocation of vpif_obj.dev[i] on the error path and
> removes duplicated check on platform_data presence.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
> ---
>  drivers/media/platform/davinci/vpif_display.c | 24 ++++++++++++-------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
> index 7be636237acf..0f324055cc9f 100644
> --- a/drivers/media/platform/davinci/vpif_display.c
> +++ b/drivers/media/platform/davinci/vpif_display.c
> @@ -1114,6 +1114,14 @@ static int initialize_vpif(void)
>         return err;
>  }
>
> +static void free_vpif_objs(void)
> +{
> +       int i;
> +
> +       for (i = 0; i < VPIF_DISPLAY_MAX_DEVICES; i++)
> +               kfree(vpif_obj.dev[i]);
> +}
> +
>  static int vpif_async_bound(struct v4l2_async_notifier *notifier,
>                             struct v4l2_subdev *subdev,
>                             struct v4l2_async_subdev *asd)
> @@ -1255,11 +1263,6 @@ static __init int vpif_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>
> -       if (!pdev->dev.platform_data) {
> -               dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
> -               return -EINVAL;
> -       }
> -
Could make this as a separate patch.

>         vpif_dev = &pdev->dev;
>         err = initialize_vpif();
>
> @@ -1271,7 +1274,7 @@ static __init int vpif_probe(struct platform_device *pdev)
>         err = v4l2_device_register(vpif_dev, &vpif_obj.v4l2_dev);
>         if (err) {
>                 v4l2_err(vpif_dev->driver, "Error registering v4l2 device\n");
> -               return err;
> +               goto vpif_free;
>         }
>
>         while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, res_idx))) {
> @@ -1314,7 +1317,10 @@ static __init int vpif_probe(struct platform_device *pdev)
>                         if (vpif_obj.sd[i])
>                                 vpif_obj.sd[i]->grp_id = 1 << i;
>                 }
> -               vpif_probe_complete();
> +               err = vpif_probe_complete();
> +               if (err) {
> +                       goto probe_subdev_out;
> +               }

{} braces are not needed

>         } else {
>                 vpif_obj.notifier.subdevs = vpif_obj.config->asd;
>                 vpif_obj.notifier.num_subdevs = vpif_obj.config->asd_sizes[0];
> @@ -1334,6 +1340,8 @@ static __init int vpif_probe(struct platform_device *pdev)
>         kfree(vpif_obj.sd);
>  vpif_unregister:
>         v4l2_device_unregister(&vpif_obj.v4l2_dev);
> +vpif_free:
> +       free_vpif_objs();
>
Just put the for loop here instead.

>         return err;
>  }
> @@ -1355,8 +1363,8 @@ static int vpif_remove(struct platform_device *device)
>                 ch = vpif_obj.dev[i];
>                 /* Unregister video device */
>                 video_unregister_device(&ch->video_dev);
> -               kfree(vpif_obj.dev[i]);
>         }
> +       free_vpif_objs();
>
Just leave this as is, as its already looping and freeing up the objects.

>         return 0;
>  }
> --
> 2.18.0
>

Cheers,
--Prabhakar Lad

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] media: davinci: vpif_display: Mix memory leak on probe error path
  2018-08-02  9:51 ` Lad, Prabhakar
@ 2018-08-06 15:50   ` Anton Vasilyev
  2018-08-06 15:50     ` [PATCH v2 2/2] media: davinci: vpif_display: remove duplicate check Anton Vasilyev
  2018-08-13 11:47     ` [PATCH v2 1/2] media: davinci: vpif_display: Mix memory leak on probe error path Lad, Prabhakar
  0 siblings, 2 replies; 6+ messages in thread
From: Anton Vasilyev @ 2018-08-06 15:50 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Anton Vasilyev, Mauro Carvalho Chehab, linux-media, linux-kernel,
	ldv-project

If vpif_probe() fails on v4l2_device_register() then memory allocated
at initialize_vpif() for global vpif_obj.dev[i] become unreleased.

The patch adds deallocation of vpif_obj.dev[i] on the probe error path.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
---
v2: divided the original patch into two and made stylistic fixes based
on the Prabhakar's rewiev.
---
 drivers/media/platform/davinci/vpif_display.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 7be636237acf..d9e578ac79c2 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1271,7 +1271,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 	err = v4l2_device_register(vpif_dev, &vpif_obj.v4l2_dev);
 	if (err) {
 		v4l2_err(vpif_dev->driver, "Error registering v4l2 device\n");
-		return err;
+		goto vpif_free;
 	}
 
 	while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, res_idx))) {
@@ -1314,7 +1314,9 @@ static __init int vpif_probe(struct platform_device *pdev)
 			if (vpif_obj.sd[i])
 				vpif_obj.sd[i]->grp_id = 1 << i;
 		}
-		vpif_probe_complete();
+		err = vpif_probe_complete();
+		if (err)
+			goto probe_subdev_out;
 	} else {
 		vpif_obj.notifier.subdevs = vpif_obj.config->asd;
 		vpif_obj.notifier.num_subdevs = vpif_obj.config->asd_sizes[0];
@@ -1334,6 +1336,9 @@ static __init int vpif_probe(struct platform_device *pdev)
 	kfree(vpif_obj.sd);
 vpif_unregister:
 	v4l2_device_unregister(&vpif_obj.v4l2_dev);
+vpif_free:
+	for (i = 0; i < VPIF_DISPLAY_MAX_DEVICES; i++)
+		kfree(vpif_obj.dev[i]);
 
 	return err;
 }
-- 
2.18.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] media: davinci: vpif_display: remove duplicate check
  2018-08-06 15:50   ` [PATCH v2 1/2] " Anton Vasilyev
@ 2018-08-06 15:50     ` Anton Vasilyev
  2018-08-13 11:46       ` Lad, Prabhakar
  2018-08-13 11:47     ` [PATCH v2 1/2] media: davinci: vpif_display: Mix memory leak on probe error path Lad, Prabhakar
  1 sibling, 1 reply; 6+ messages in thread
From: Anton Vasilyev @ 2018-08-06 15:50 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Anton Vasilyev, Mauro Carvalho Chehab, linux-media, linux-kernel,
	ldv-project

The patch removes the duplicate platform_data check from vpif_probe().

Fixes: 4a5f8ae50b66 ("[media] davinci: vpif_capture: get subdevs from DT when available")

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
---
v2: divided the original patch into two and made stylistic fixes based
on the Prabhakar's rewiev.
---
 drivers/media/platform/davinci/vpif_display.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index d9e578ac79c2..f000fc492ef9 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1255,11 +1255,6 @@ static __init int vpif_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	if (!pdev->dev.platform_data) {
-		dev_warn(&pdev->dev, "Missing platform data.  Giving up.\n");
-		return -EINVAL;
-	}
-
 	vpif_dev = &pdev->dev;
 	err = initialize_vpif();
 
-- 
2.18.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] media: davinci: vpif_display: remove duplicate check
  2018-08-06 15:50     ` [PATCH v2 2/2] media: davinci: vpif_display: remove duplicate check Anton Vasilyev
@ 2018-08-13 11:46       ` Lad, Prabhakar
  0 siblings, 0 replies; 6+ messages in thread
From: Lad, Prabhakar @ 2018-08-13 11:46 UTC (permalink / raw)
  To: Anton Vasilyev; +Cc: Mauro Carvalho Chehab, linux-media, LKML, ldv-project

Hi,

Thanks for the patch.

On Mon, Aug 6, 2018 at 4:51 PM Anton Vasilyev <vasilyev@ispras.ru> wrote:
>
> The patch removes the duplicate platform_data check from vpif_probe().
>
> Fixes: 4a5f8ae50b66 ("[media] davinci: vpif_capture: get subdevs from DT when available")
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
> ---
> v2: divided the original patch into two and made stylistic fixes based
> on the Prabhakar's rewiev.
> ---
>  drivers/media/platform/davinci/vpif_display.c | 5 -----
>  1 file changed, 5 deletions(-)
>
Acked-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>

Cheers,
--Prabhakar Lad

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] media: davinci: vpif_display: Mix memory leak on probe error path
  2018-08-06 15:50   ` [PATCH v2 1/2] " Anton Vasilyev
  2018-08-06 15:50     ` [PATCH v2 2/2] media: davinci: vpif_display: remove duplicate check Anton Vasilyev
@ 2018-08-13 11:47     ` Lad, Prabhakar
  1 sibling, 0 replies; 6+ messages in thread
From: Lad, Prabhakar @ 2018-08-13 11:47 UTC (permalink / raw)
  To: Anton Vasilyev; +Cc: Mauro Carvalho Chehab, linux-media, LKML, ldv-project

Hi,

Thanks for the patch.

On Mon, Aug 6, 2018 at 4:50 PM Anton Vasilyev <vasilyev@ispras.ru> wrote:
>
> If vpif_probe() fails on v4l2_device_register() then memory allocated
> at initialize_vpif() for global vpif_obj.dev[i] become unreleased.
>
> The patch adds deallocation of vpif_obj.dev[i] on the probe error path.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
> ---
> v2: divided the original patch into two and made stylistic fixes based
> on the Prabhakar's rewiev.
> ---
>  drivers/media/platform/davinci/vpif_display.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
Acked-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>

Cheers,
--Prabhakar Lad

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-08-13 11:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-27 11:52 [PATCH] media: davinci: vpif_display: Mix memory leak on probe error path Anton Vasilyev
2018-08-02  9:51 ` Lad, Prabhakar
2018-08-06 15:50   ` [PATCH v2 1/2] " Anton Vasilyev
2018-08-06 15:50     ` [PATCH v2 2/2] media: davinci: vpif_display: remove duplicate check Anton Vasilyev
2018-08-13 11:46       ` Lad, Prabhakar
2018-08-13 11:47     ` [PATCH v2 1/2] media: davinci: vpif_display: Mix memory leak on probe error path Lad, Prabhakar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.