All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] media: davinci: vpif: fix use-after-free on driver unbind
@ 2021-12-22 14:20 Johan Hovold
  2021-12-22 14:20 ` [PATCH 1/4] media: davinci: vpif: fix unbalanced runtime PM get Johan Hovold
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Johan Hovold @ 2021-12-22 14:20 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman,
	Greg Kroah-Hartman, linux-media, linux-kernel, Johan Hovold

When scanning the tree for drivers that did not handle platform-device
registration failures properly I stumbled upon this driver, which also
failed to deregister device structures before freeing them.

Included are also some related runtime PM imbalance fixes and a printk
clean up.

Johan


Johan Hovold (4):
  media: davinci: vpif: fix unbalanced runtime PM get
  media: davinci: vpif: fix unbalanced runtime PM enable
  media: davinci: vpif: fix use-after-free on driver unbind
  media: davinci: vpif: drop probe printk

 drivers/media/platform/davinci/vpif.c | 111 +++++++++++++++++++-------
 1 file changed, 81 insertions(+), 30 deletions(-)

-- 
2.32.0


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

* [PATCH 1/4] media: davinci: vpif: fix unbalanced runtime PM get
  2021-12-22 14:20 [PATCH 0/4] media: davinci: vpif: fix use-after-free on driver unbind Johan Hovold
@ 2021-12-22 14:20 ` Johan Hovold
  2022-01-04 18:05   ` Lad, Prabhakar
  2021-12-22 14:20 ` [PATCH 2/4] media: davinci: vpif: fix unbalanced runtime PM enable Johan Hovold
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2021-12-22 14:20 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman,
	Greg Kroah-Hartman, linux-media, linux-kernel, Johan Hovold,
	stable, Lad

Make sure to balance the runtime PM usage counter on driver unbind.

Fixes: 407ccc65bfd2 ("[media] davinci: vpif: add pm_runtime support")
Cc: stable@vger.kernel.org      # 3.9
Cc: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/media/platform/davinci/vpif.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 5a89d885d0e3..9752a5ec36f7 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -495,6 +495,7 @@ static int vpif_probe(struct platform_device *pdev)
 
 static int vpif_remove(struct platform_device *pdev)
 {
+	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	return 0;
 }
-- 
2.32.0


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

* [PATCH 2/4] media: davinci: vpif: fix unbalanced runtime PM enable
  2021-12-22 14:20 [PATCH 0/4] media: davinci: vpif: fix use-after-free on driver unbind Johan Hovold
  2021-12-22 14:20 ` [PATCH 1/4] media: davinci: vpif: fix unbalanced runtime PM get Johan Hovold
@ 2021-12-22 14:20 ` Johan Hovold
  2022-01-04 18:11   ` Lad, Prabhakar
  2021-12-22 14:20 ` [PATCH 3/4] media: davinci: vpif: fix use-after-free on driver unbind Johan Hovold
  2021-12-22 14:20 ` [PATCH 4/4] media: davinci: vpif: drop probe printk Johan Hovold
  3 siblings, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2021-12-22 14:20 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman,
	Greg Kroah-Hartman, linux-media, linux-kernel, Johan Hovold,
	stable

Make sure to disable runtime PM before returning on probe errors.

Fixes: 479f7a118105 ("[media] davinci: vpif: adaptions for DT support")
Cc: stable@vger.kernel.org      # 4.12: 4024d6f601e3c
Cc: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/media/platform/davinci/vpif.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 9752a5ec36f7..1f5eacf48580 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -428,6 +428,7 @@ static int vpif_probe(struct platform_device *pdev)
 	static struct resource *res_irq;
 	struct platform_device *pdev_capture, *pdev_display;
 	struct device_node *endpoint = NULL;
+	int ret;
 
 	vpif_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(vpif_base))
@@ -456,8 +457,8 @@ static int vpif_probe(struct platform_device *pdev)
 	res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (!res_irq) {
 		dev_warn(&pdev->dev, "Missing IRQ resource.\n");
-		pm_runtime_put(&pdev->dev);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_put_rpm;
 	}
 
 	pdev_capture = devm_kzalloc(&pdev->dev, sizeof(*pdev_capture),
@@ -491,6 +492,12 @@ static int vpif_probe(struct platform_device *pdev)
 	}
 
 	return 0;
+
+err_put_rpm:
+	pm_runtime_put(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	return ret;
 }
 
 static int vpif_remove(struct platform_device *pdev)
-- 
2.32.0


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

* [PATCH 3/4] media: davinci: vpif: fix use-after-free on driver unbind
  2021-12-22 14:20 [PATCH 0/4] media: davinci: vpif: fix use-after-free on driver unbind Johan Hovold
  2021-12-22 14:20 ` [PATCH 1/4] media: davinci: vpif: fix unbalanced runtime PM get Johan Hovold
  2021-12-22 14:20 ` [PATCH 2/4] media: davinci: vpif: fix unbalanced runtime PM enable Johan Hovold
@ 2021-12-22 14:20 ` Johan Hovold
  2022-01-04 18:30   ` Lad, Prabhakar
  2021-12-22 14:20 ` [PATCH 4/4] media: davinci: vpif: drop probe printk Johan Hovold
  3 siblings, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2021-12-22 14:20 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman,
	Greg Kroah-Hartman, linux-media, linux-kernel, Johan Hovold,
	stable

The driver allocates and registers two platform device structures during
probe, but the devices were never deregistered on driver unbind.

This results in a use-after-free on driver unbind as the device
structures were allocated using devres and would be freed by driver
core when remove() returns.

Fix this by adding the missing deregistration calls to the remove()
callback and failing probe on registration errors.

Note that the platform device structures must be freed using a proper
release callback to avoid leaking associated resources like device
names.

Fixes: 479f7a118105 ("[media] davinci: vpif: adaptions for DT support")
Cc: stable@vger.kernel.org      # 4.12
Cc: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/media/platform/davinci/vpif.c | 97 ++++++++++++++++++++-------
 1 file changed, 71 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 1f5eacf48580..4a260f4ed236 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -41,6 +41,11 @@ MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
 #define VPIF_CH2_MAX_MODES	15
 #define VPIF_CH3_MAX_MODES	2
 
+struct vpif_data {
+	struct platform_device *capture;
+	struct platform_device *display;
+};
+
 DEFINE_SPINLOCK(vpif_lock);
 EXPORT_SYMBOL_GPL(vpif_lock);
 
@@ -423,17 +428,31 @@ int vpif_channel_getfid(u8 channel_id)
 }
 EXPORT_SYMBOL(vpif_channel_getfid);
 
+static void vpif_pdev_release(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+
+	kfree(pdev);
+}
+
 static int vpif_probe(struct platform_device *pdev)
 {
 	static struct resource *res_irq;
 	struct platform_device *pdev_capture, *pdev_display;
 	struct device_node *endpoint = NULL;
+	struct vpif_data *data;
 	int ret;
 
 	vpif_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(vpif_base))
 		return PTR_ERR(vpif_base);
 
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, data);
+
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get(&pdev->dev);
 
@@ -461,49 +480,75 @@ static int vpif_probe(struct platform_device *pdev)
 		goto err_put_rpm;
 	}
 
-	pdev_capture = devm_kzalloc(&pdev->dev, sizeof(*pdev_capture),
-				    GFP_KERNEL);
-	if (pdev_capture) {
-		pdev_capture->name = "vpif_capture";
-		pdev_capture->id = -1;
-		pdev_capture->resource = res_irq;
-		pdev_capture->num_resources = 1;
-		pdev_capture->dev.dma_mask = pdev->dev.dma_mask;
-		pdev_capture->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
-		pdev_capture->dev.parent = &pdev->dev;
-		platform_device_register(pdev_capture);
-	} else {
-		dev_warn(&pdev->dev, "Unable to allocate memory for pdev_capture.\n");
+	pdev_capture = kzalloc(sizeof(*pdev_capture), GFP_KERNEL);
+	if (!pdev_capture) {
+		ret = -ENOMEM;
+		goto err_put_rpm;
 	}
 
-	pdev_display = devm_kzalloc(&pdev->dev, sizeof(*pdev_display),
-				    GFP_KERNEL);
-	if (pdev_display) {
-		pdev_display->name = "vpif_display";
-		pdev_display->id = -1;
-		pdev_display->resource = res_irq;
-		pdev_display->num_resources = 1;
-		pdev_display->dev.dma_mask = pdev->dev.dma_mask;
-		pdev_display->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
-		pdev_display->dev.parent = &pdev->dev;
-		platform_device_register(pdev_display);
-	} else {
-		dev_warn(&pdev->dev, "Unable to allocate memory for pdev_display.\n");
+	pdev_capture->name = "vpif_capture";
+	pdev_capture->id = -1;
+	pdev_capture->resource = res_irq;
+	pdev_capture->num_resources = 1;
+	pdev_capture->dev.dma_mask = pdev->dev.dma_mask;
+	pdev_capture->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+	pdev_capture->dev.parent = &pdev->dev;
+	pdev_capture->dev.release = vpif_pdev_release;
+
+	ret = platform_device_register(pdev_capture);
+	if (ret)
+		goto err_put_pdev_capture;
+
+	pdev_display = kzalloc(sizeof(*pdev_display), GFP_KERNEL);
+	if (!pdev_display) {
+		ret = -ENOMEM;
+		goto err_put_pdev_capture;
 	}
 
+	pdev_display->name = "vpif_display";
+	pdev_display->id = -1;
+	pdev_display->resource = res_irq;
+	pdev_display->num_resources = 1;
+	pdev_display->dev.dma_mask = pdev->dev.dma_mask;
+	pdev_display->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
+	pdev_display->dev.parent = &pdev->dev;
+	pdev_display->dev.release = vpif_pdev_release;
+
+	ret = platform_device_register(pdev_display);
+	if (ret)
+		goto err_put_pdev_display;
+
+	data->capture = pdev_capture;
+	data->display = pdev_display;
+
 	return 0;
 
+err_put_pdev_display:
+	platform_device_put(pdev_display);
+err_put_pdev_capture:
+	platform_device_put(pdev_capture);
 err_put_rpm:
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+	kfree(data);
 
 	return ret;
 }
 
 static int vpif_remove(struct platform_device *pdev)
 {
+	struct vpif_data *data = platform_get_drvdata(pdev);
+
+	if (data->capture)
+		platform_device_unregister(data->capture);
+	if (data->display)
+		platform_device_unregister(data->display);
+
 	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+
+	kfree(data);
+
 	return 0;
 }
 
-- 
2.32.0


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

* [PATCH 4/4] media: davinci: vpif: drop probe printk
  2021-12-22 14:20 [PATCH 0/4] media: davinci: vpif: fix use-after-free on driver unbind Johan Hovold
                   ` (2 preceding siblings ...)
  2021-12-22 14:20 ` [PATCH 3/4] media: davinci: vpif: fix use-after-free on driver unbind Johan Hovold
@ 2021-12-22 14:20 ` Johan Hovold
  2022-01-04 18:31   ` Lad, Prabhakar
  3 siblings, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2021-12-22 14:20 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman,
	Greg Kroah-Hartman, linux-media, linux-kernel, Johan Hovold

Drivers should generally not print anything for a successful probe, and
printing "success" before probe is done makes no sense.

Drop the unnecessary and misleading dev_info() call from probe.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/media/platform/davinci/vpif.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 4a260f4ed236..03b4e51bb13a 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -456,8 +456,6 @@ static int vpif_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get(&pdev->dev);
 
-	dev_info(&pdev->dev, "vpif probe success\n");
-
 	/*
 	 * If VPIF Node has endpoints, assume "new" DT support,
 	 * where capture and display drivers don't have DT nodes
-- 
2.32.0


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

* Re: [PATCH 1/4] media: davinci: vpif: fix unbalanced runtime PM get
  2021-12-22 14:20 ` [PATCH 1/4] media: davinci: vpif: fix unbalanced runtime PM get Johan Hovold
@ 2022-01-04 18:05   ` Lad, Prabhakar
  0 siblings, 0 replies; 10+ messages in thread
From: Lad, Prabhakar @ 2022-01-04 18:05 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman,
	Greg Kroah-Hartman, linux-media, LKML, stable, Lad

Hi Johan,

Thank you for the patch.

On Wed, Dec 22, 2021 at 2:20 PM Johan Hovold <johan@kernel.org> wrote:
>
> Make sure to balance the runtime PM usage counter on driver unbind.
>
> Fixes: 407ccc65bfd2 ("[media] davinci: vpif: add pm_runtime support")
> Cc: stable@vger.kernel.org      # 3.9
> Cc: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/media/platform/davinci/vpif.c | 1 +
>  1 file changed, 1 insertion(+)
>
Reviewed-by: Lad Prabhakar <prabhakar.csengg@gmail.com>

> diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
> index 5a89d885d0e3..9752a5ec36f7 100644
> --- a/drivers/media/platform/davinci/vpif.c
> +++ b/drivers/media/platform/davinci/vpif.c
> @@ -495,6 +495,7 @@ static int vpif_probe(struct platform_device *pdev)
>
>  static int vpif_remove(struct platform_device *pdev)
>  {
> +       pm_runtime_put(&pdev->dev);
>         pm_runtime_disable(&pdev->dev);
>         return 0;
>  }
> --
> 2.32.0
>

Cheers,
Prabhakar

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

* Re: [PATCH 2/4] media: davinci: vpif: fix unbalanced runtime PM enable
  2021-12-22 14:20 ` [PATCH 2/4] media: davinci: vpif: fix unbalanced runtime PM enable Johan Hovold
@ 2022-01-04 18:11   ` Lad, Prabhakar
  2022-01-31 10:20     ` Johan Hovold
  0 siblings, 1 reply; 10+ messages in thread
From: Lad, Prabhakar @ 2022-01-04 18:11 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman,
	Greg Kroah-Hartman, linux-media, LKML, stable

Hi Johan,

Thank you for the patch.

On Wed, Dec 22, 2021 at 2:20 PM Johan Hovold <johan@kernel.org> wrote:
>
> Make sure to disable runtime PM before returning on probe errors.
>
> Fixes: 479f7a118105 ("[media] davinci: vpif: adaptions for DT support")
> Cc: stable@vger.kernel.org      # 4.12: 4024d6f601e3c
> Cc: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/media/platform/davinci/vpif.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
> index 9752a5ec36f7..1f5eacf48580 100644
> --- a/drivers/media/platform/davinci/vpif.c
> +++ b/drivers/media/platform/davinci/vpif.c
> @@ -428,6 +428,7 @@ static int vpif_probe(struct platform_device *pdev)
>         static struct resource *res_irq;
>         struct platform_device *pdev_capture, *pdev_display;
>         struct device_node *endpoint = NULL;
> +       int ret;
>
>         vpif_base = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(vpif_base))
> @@ -456,8 +457,8 @@ static int vpif_probe(struct platform_device *pdev)
>         res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>         if (!res_irq) {
>                 dev_warn(&pdev->dev, "Missing IRQ resource.\n");
> -               pm_runtime_put(&pdev->dev);
Maybe just add pm_runtime_disable(&pdev->dev); here, rest diff won't
be required.

Cheers,
Prabhakar

> -               return -EINVAL;
> +               ret = -EINVAL;
> +               goto err_put_rpm;
>         }
>
>         pdev_capture = devm_kzalloc(&pdev->dev, sizeof(*pdev_capture),
> @@ -491,6 +492,12 @@ static int vpif_probe(struct platform_device *pdev)
>         }
>
>         return 0;
> +
> +err_put_rpm:
> +       pm_runtime_put(&pdev->dev);
> +       pm_runtime_disable(&pdev->dev);
> +
> +       return ret;
>  }
>
>  static int vpif_remove(struct platform_device *pdev)
> --
> 2.32.0
>

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

* Re: [PATCH 3/4] media: davinci: vpif: fix use-after-free on driver unbind
  2021-12-22 14:20 ` [PATCH 3/4] media: davinci: vpif: fix use-after-free on driver unbind Johan Hovold
@ 2022-01-04 18:30   ` Lad, Prabhakar
  0 siblings, 0 replies; 10+ messages in thread
From: Lad, Prabhakar @ 2022-01-04 18:30 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman,
	Greg Kroah-Hartman, linux-media, LKML, stable

Hi Johan,

Thank you for the patch.

On Wed, Dec 22, 2021 at 2:20 PM Johan Hovold <johan@kernel.org> wrote:
>
> The driver allocates and registers two platform device structures during
> probe, but the devices were never deregistered on driver unbind.
>
> This results in a use-after-free on driver unbind as the device
> structures were allocated using devres and would be freed by driver
> core when remove() returns.
>
> Fix this by adding the missing deregistration calls to the remove()
> callback and failing probe on registration errors.
>
> Note that the platform device structures must be freed using a proper
> release callback to avoid leaking associated resources like device
> names.
>
> Fixes: 479f7a118105 ("[media] davinci: vpif: adaptions for DT support")
> Cc: stable@vger.kernel.org      # 4.12
> Cc: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/media/platform/davinci/vpif.c | 97 ++++++++++++++++++++-------
>  1 file changed, 71 insertions(+), 26 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.csengg@gmail.com>

Cheers,
Prabhakar

> diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
> index 1f5eacf48580..4a260f4ed236 100644
> --- a/drivers/media/platform/davinci/vpif.c
> +++ b/drivers/media/platform/davinci/vpif.c
> @@ -41,6 +41,11 @@ MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>  #define VPIF_CH2_MAX_MODES     15
>  #define VPIF_CH3_MAX_MODES     2
>
> +struct vpif_data {
> +       struct platform_device *capture;
> +       struct platform_device *display;
> +};
> +
>  DEFINE_SPINLOCK(vpif_lock);
>  EXPORT_SYMBOL_GPL(vpif_lock);
>
> @@ -423,17 +428,31 @@ int vpif_channel_getfid(u8 channel_id)
>  }
>  EXPORT_SYMBOL(vpif_channel_getfid);
>
> +static void vpif_pdev_release(struct device *dev)
> +{
> +       struct platform_device *pdev = to_platform_device(dev);
> +
> +       kfree(pdev);
> +}
> +
>  static int vpif_probe(struct platform_device *pdev)
>  {
>         static struct resource *res_irq;
>         struct platform_device *pdev_capture, *pdev_display;
>         struct device_node *endpoint = NULL;
> +       struct vpif_data *data;
>         int ret;
>
>         vpif_base = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(vpif_base))
>                 return PTR_ERR(vpif_base);
>
> +       data = kzalloc(sizeof(*data), GFP_KERNEL);
> +       if (!data)
> +               return -ENOMEM;
> +
> +       platform_set_drvdata(pdev, data);
> +
>         pm_runtime_enable(&pdev->dev);
>         pm_runtime_get(&pdev->dev);
>
> @@ -461,49 +480,75 @@ static int vpif_probe(struct platform_device *pdev)
>                 goto err_put_rpm;
>         }
>
> -       pdev_capture = devm_kzalloc(&pdev->dev, sizeof(*pdev_capture),
> -                                   GFP_KERNEL);
> -       if (pdev_capture) {
> -               pdev_capture->name = "vpif_capture";
> -               pdev_capture->id = -1;
> -               pdev_capture->resource = res_irq;
> -               pdev_capture->num_resources = 1;
> -               pdev_capture->dev.dma_mask = pdev->dev.dma_mask;
> -               pdev_capture->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
> -               pdev_capture->dev.parent = &pdev->dev;
> -               platform_device_register(pdev_capture);
> -       } else {
> -               dev_warn(&pdev->dev, "Unable to allocate memory for pdev_capture.\n");
> +       pdev_capture = kzalloc(sizeof(*pdev_capture), GFP_KERNEL);
> +       if (!pdev_capture) {
> +               ret = -ENOMEM;
> +               goto err_put_rpm;
>         }
>
> -       pdev_display = devm_kzalloc(&pdev->dev, sizeof(*pdev_display),
> -                                   GFP_KERNEL);
> -       if (pdev_display) {
> -               pdev_display->name = "vpif_display";
> -               pdev_display->id = -1;
> -               pdev_display->resource = res_irq;
> -               pdev_display->num_resources = 1;
> -               pdev_display->dev.dma_mask = pdev->dev.dma_mask;
> -               pdev_display->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
> -               pdev_display->dev.parent = &pdev->dev;
> -               platform_device_register(pdev_display);
> -       } else {
> -               dev_warn(&pdev->dev, "Unable to allocate memory for pdev_display.\n");
> +       pdev_capture->name = "vpif_capture";
> +       pdev_capture->id = -1;
> +       pdev_capture->resource = res_irq;
> +       pdev_capture->num_resources = 1;
> +       pdev_capture->dev.dma_mask = pdev->dev.dma_mask;
> +       pdev_capture->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
> +       pdev_capture->dev.parent = &pdev->dev;
> +       pdev_capture->dev.release = vpif_pdev_release;
> +
> +       ret = platform_device_register(pdev_capture);
> +       if (ret)
> +               goto err_put_pdev_capture;
> +
> +       pdev_display = kzalloc(sizeof(*pdev_display), GFP_KERNEL);
> +       if (!pdev_display) {
> +               ret = -ENOMEM;
> +               goto err_put_pdev_capture;
>         }
>
> +       pdev_display->name = "vpif_display";
> +       pdev_display->id = -1;
> +       pdev_display->resource = res_irq;
> +       pdev_display->num_resources = 1;
> +       pdev_display->dev.dma_mask = pdev->dev.dma_mask;
> +       pdev_display->dev.coherent_dma_mask = pdev->dev.coherent_dma_mask;
> +       pdev_display->dev.parent = &pdev->dev;
> +       pdev_display->dev.release = vpif_pdev_release;
> +
> +       ret = platform_device_register(pdev_display);
> +       if (ret)
> +               goto err_put_pdev_display;
> +
> +       data->capture = pdev_capture;
> +       data->display = pdev_display;
> +
>         return 0;
>
> +err_put_pdev_display:
> +       platform_device_put(pdev_display);
> +err_put_pdev_capture:
> +       platform_device_put(pdev_capture);
>  err_put_rpm:
>         pm_runtime_put(&pdev->dev);
>         pm_runtime_disable(&pdev->dev);
> +       kfree(data);
>
>         return ret;
>  }
>
>  static int vpif_remove(struct platform_device *pdev)
>  {
> +       struct vpif_data *data = platform_get_drvdata(pdev);
> +
> +       if (data->capture)
> +               platform_device_unregister(data->capture);
> +       if (data->display)
> +               platform_device_unregister(data->display);
> +
>         pm_runtime_put(&pdev->dev);
>         pm_runtime_disable(&pdev->dev);
> +
> +       kfree(data);
> +
>         return 0;
>  }
>
> --
> 2.32.0
>

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

* Re: [PATCH 4/4] media: davinci: vpif: drop probe printk
  2021-12-22 14:20 ` [PATCH 4/4] media: davinci: vpif: drop probe printk Johan Hovold
@ 2022-01-04 18:31   ` Lad, Prabhakar
  0 siblings, 0 replies; 10+ messages in thread
From: Lad, Prabhakar @ 2022-01-04 18:31 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman,
	Greg Kroah-Hartman, linux-media, LKML

Hi Johan,

Thank you for the patch.

On Wed, Dec 22, 2021 at 2:20 PM Johan Hovold <johan@kernel.org> wrote:
>
> Drivers should generally not print anything for a successful probe, and
> printing "success" before probe is done makes no sense.
>
> Drop the unnecessary and misleading dev_info() call from probe.
>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/media/platform/davinci/vpif.c | 2 --
>  1 file changed, 2 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.csengg@gmail.com>

Cheers,
Prabhakar

> diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
> index 4a260f4ed236..03b4e51bb13a 100644
> --- a/drivers/media/platform/davinci/vpif.c
> +++ b/drivers/media/platform/davinci/vpif.c
> @@ -456,8 +456,6 @@ static int vpif_probe(struct platform_device *pdev)
>         pm_runtime_enable(&pdev->dev);
>         pm_runtime_get(&pdev->dev);
>
> -       dev_info(&pdev->dev, "vpif probe success\n");
> -
>         /*
>          * If VPIF Node has endpoints, assume "new" DT support,
>          * where capture and display drivers don't have DT nodes
> --
> 2.32.0
>

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

* Re: [PATCH 2/4] media: davinci: vpif: fix unbalanced runtime PM enable
  2022-01-04 18:11   ` Lad, Prabhakar
@ 2022-01-31 10:20     ` Johan Hovold
  0 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2022-01-31 10:20 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman,
	Greg Kroah-Hartman, linux-media, LKML, stable

Hi Lad,

and sorry about the late reply.

On Tue, Jan 04, 2022 at 06:11:08PM +0000, Lad, Prabhakar wrote:
> Hi Johan,
> 
> Thank you for the patch.
> 
> On Wed, Dec 22, 2021 at 2:20 PM Johan Hovold <johan@kernel.org> wrote:
> >
> > Make sure to disable runtime PM before returning on probe errors.
> >
> > Fixes: 479f7a118105 ("[media] davinci: vpif: adaptions for DT support")
> > Cc: stable@vger.kernel.org      # 4.12: 4024d6f601e3c
> > Cc: Kevin Hilman <khilman@baylibre.com>
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >  drivers/media/platform/davinci/vpif.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
> > index 9752a5ec36f7..1f5eacf48580 100644
> > --- a/drivers/media/platform/davinci/vpif.c
> > +++ b/drivers/media/platform/davinci/vpif.c
> > @@ -428,6 +428,7 @@ static int vpif_probe(struct platform_device *pdev)
> >         static struct resource *res_irq;
> >         struct platform_device *pdev_capture, *pdev_display;
> >         struct device_node *endpoint = NULL;
> > +       int ret;
> >
> >         vpif_base = devm_platform_ioremap_resource(pdev, 0);
> >         if (IS_ERR(vpif_base))
> > @@ -456,8 +457,8 @@ static int vpif_probe(struct platform_device *pdev)
> >         res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> >         if (!res_irq) {
> >                 dev_warn(&pdev->dev, "Missing IRQ resource.\n");
> > -               pm_runtime_put(&pdev->dev);
> Maybe just add pm_runtime_disable(&pdev->dev); here, rest diff won't
> be required.

I chose to do it this way in order to make the following patches
smaller and easier to review.

Thanks for reviewing.

Johan

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

end of thread, other threads:[~2022-01-31 10:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-22 14:20 [PATCH 0/4] media: davinci: vpif: fix use-after-free on driver unbind Johan Hovold
2021-12-22 14:20 ` [PATCH 1/4] media: davinci: vpif: fix unbalanced runtime PM get Johan Hovold
2022-01-04 18:05   ` Lad, Prabhakar
2021-12-22 14:20 ` [PATCH 2/4] media: davinci: vpif: fix unbalanced runtime PM enable Johan Hovold
2022-01-04 18:11   ` Lad, Prabhakar
2022-01-31 10:20     ` Johan Hovold
2021-12-22 14:20 ` [PATCH 3/4] media: davinci: vpif: fix use-after-free on driver unbind Johan Hovold
2022-01-04 18:30   ` Lad, Prabhakar
2021-12-22 14:20 ` [PATCH 4/4] media: davinci: vpif: drop probe printk Johan Hovold
2022-01-04 18:31   ` 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.