[PATCH v1 1/1] media: ipu3-cio2: Drop reference on error path in cio2_bridge_connect_sensor()

[PATCH v1 1/1] media: ipu3-cio2: Drop reference on error path in cio2_bridge_connect_sensor()

[Andy Shevchenko]

The commit 71f642833284 ("ACPI: utils: Fix reference counting in
for_each_acpi_dev_match()") moved adev assignment outside of error
path and hence made acpi_dev_put(sensor->adev) a no-op. We still
need to drop reference count on error path, and to achieve that,
replace sensor->adev by locally assigned adev.

Fixes: 71f642833284 ("ACPI: utils: Fix reference counting in for_each_acpi_dev_match()")
Depends-on: fc68f42aa737 ("ACPI: fix NULL pointer dereference")
Reported-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/pci/intel/ipu3/cio2-bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c b/drivers/media/pci/intel/ipu3/cio2-bridge.c
index 59a36f922675..30d29b96a339 100644
--- a/drivers/media/pci/intel/ipu3/cio2-bridge.c
+++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
@@ -226,7 +226,7 @@ static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
 err_free_swnodes:
 	software_node_unregister_nodes(sensor->swnodes);
 err_put_adev:
-	acpi_dev_put(sensor->adev);
+	acpi_dev_put(adev);
 	return ret;
 }
 
-- 
2.30.2

Re: [PATCH v1 1/1] media: ipu3-cio2: Drop reference on error path in cio2_bridge_connect_sensor()

[Sakari Ailus]

Hi Andy,

On Mon, Jul 26, 2021 at 11:40:55AM +0300, Andy Shevchenko wrote:
> The commit 71f642833284 ("ACPI: utils: Fix reference counting in
> for_each_acpi_dev_match()") moved adev assignment outside of error
> path and hence made acpi_dev_put(sensor->adev) a no-op. We still
> need to drop reference count on error path, and to achieve that,
> replace sensor->adev by locally assigned adev.
> 
> Fixes: 71f642833284 ("ACPI: utils: Fix reference counting in for_each_acpi_dev_match()")
> Depends-on: fc68f42aa737 ("ACPI: fix NULL pointer dereference")
> Reported-by: Jens Axboe <axboe@kernel.dk>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/media/pci/intel/ipu3/cio2-bridge.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c b/drivers/media/pci/intel/ipu3/cio2-bridge.c
> index 59a36f922675..30d29b96a339 100644
> --- a/drivers/media/pci/intel/ipu3/cio2-bridge.c
> +++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
> @@ -226,7 +226,7 @@ static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
>  err_free_swnodes:
>  	software_node_unregister_nodes(sensor->swnodes);
>  err_put_adev:
> -	acpi_dev_put(sensor->adev);
> +	acpi_dev_put(adev);

adev is assigned to sensor->adev before goto so the two have the same
value. I have no problem with the patch though.

>  	return ret;
>  }
>  

-- 
Regards,

Sakari Ailus

Re: [PATCH v1 1/1] media: ipu3-cio2: Drop reference on error path in cio2_bridge_connect_sensor()

[Andy Shevchenko]

On Mon, Jul 26, 2021 at 2:47 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> On Mon, Jul 26, 2021 at 11:40:55AM +0300, Andy Shevchenko wrote:

...

> >  err_free_swnodes:
> >       software_node_unregister_nodes(sensor->swnodes);
> >  err_put_adev:
> > -     acpi_dev_put(sensor->adev);
> > +     acpi_dev_put(adev);
>
> adev is assigned to sensor->adev before goto so the two have the same
> value. I have no problem with the patch though.

Are we reading the same version? Or am I missing something?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] media: ipu3-cio2: Drop reference on error path in cio2_bridge_connect_sensor()

[Sakari Ailus]

On Mon, Jul 26, 2021 at 02:55:51PM +0300, Andy Shevchenko wrote:
> On Mon, Jul 26, 2021 at 2:47 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> > On Mon, Jul 26, 2021 at 11:40:55AM +0300, Andy Shevchenko wrote:
> 
> ...
> 
> > >  err_free_swnodes:
> > >       software_node_unregister_nodes(sensor->swnodes);
> > >  err_put_adev:
> > > -     acpi_dev_put(sensor->adev);
> > > +     acpi_dev_put(adev);
> >
> > adev is assigned to sensor->adev before goto so the two have the same
> > value. I have no problem with the patch though.
> 
> Are we reading the same version? Or am I missing something?

I'm looking at the one in media-tree master.

-- 
Sakari Ailus

Re: [PATCH v1 1/1] media: ipu3-cio2: Drop reference on error path in cio2_bridge_connect_sensor()

[Sakari Ailus]

On Mon, Jul 26, 2021 at 02:55:51PM +0300, Andy Shevchenko wrote:
> On Mon, Jul 26, 2021 at 2:47 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> > On Mon, Jul 26, 2021 at 11:40:55AM +0300, Andy Shevchenko wrote:
> 
> ...
> 
> > >  err_free_swnodes:
> > >       software_node_unregister_nodes(sensor->swnodes);
> > >  err_put_adev:
> > > -     acpi_dev_put(sensor->adev);
> > > +     acpi_dev_put(adev);
> >
> > adev is assigned to sensor->adev before goto so the two have the same
> > value. I have no problem with the patch though.
> 
> Are we reading the same version? Or am I missing something?

Ah. I noticed the adev assignment was removed (and added later) by the
other patch. Yeah, agreed; this one's needed.

-- 
Sakari Ailus

Re: [PATCH v1 1/1] media: ipu3-cio2: Drop reference on error path in cio2_bridge_connect_sensor()

[Andy Shevchenko]

On Mon, Jul 26, 2021 at 3:01 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> On Mon, Jul 26, 2021 at 02:55:51PM +0300, Andy Shevchenko wrote:
> > On Mon, Jul 26, 2021 at 2:47 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > > On Mon, Jul 26, 2021 at 11:40:55AM +0300, Andy Shevchenko wrote:
> >
> > ...
> >
> > > >  err_free_swnodes:
> > > >       software_node_unregister_nodes(sensor->swnodes);
> > > >  err_put_adev:
> > > > -     acpi_dev_put(sensor->adev);
> > > > +     acpi_dev_put(adev);
> > >
> > > adev is assigned to sensor->adev before goto so the two have the same
> > > value. I have no problem with the patch though.
> >
> > Are we reading the same version? Or am I missing something?
>
> I'm looking at the one in media-tree master.

Maintainers with a push access may FF merge the v5.14-rc3 as far as I
can tell and then you will see the difference.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] media: ipu3-cio2: Drop reference on error path in cio2_bridge_connect_sensor()

[Andy Shevchenko]

On Mon, Jul 26, 2021 at 03:03:35PM +0300, Sakari Ailus wrote:
> On Mon, Jul 26, 2021 at 02:55:51PM +0300, Andy Shevchenko wrote:
> > On Mon, Jul 26, 2021 at 2:47 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > > On Mon, Jul 26, 2021 at 11:40:55AM +0300, Andy Shevchenko wrote:
> > 
> > ...
> > 
> > > >  err_free_swnodes:
> > > >       software_node_unregister_nodes(sensor->swnodes);
> > > >  err_put_adev:
> > > > -     acpi_dev_put(sensor->adev);
> > > > +     acpi_dev_put(adev);
> > >
> > > adev is assigned to sensor->adev before goto so the two have the same
> > > value. I have no problem with the patch though.
> > 
> > Are we reading the same version? Or am I missing something?
> 
> Ah. I noticed the adev assignment was removed (and added later) by the
> other patch. Yeah, agreed; this one's needed.

Thanks! Can we have your tag?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] media: ipu3-cio2: Drop reference on error path in cio2_bridge_connect_sensor()

[Sakari Ailus]

On Tue, Jul 27, 2021 at 02:35:10PM +0300, Andy Shevchenko wrote:
> On Mon, Jul 26, 2021 at 03:03:35PM +0300, Sakari Ailus wrote:
> > On Mon, Jul 26, 2021 at 02:55:51PM +0300, Andy Shevchenko wrote:
> > > On Mon, Jul 26, 2021 at 2:47 PM Sakari Ailus
> > > <sakari.ailus@linux.intel.com> wrote:
> > > > On Mon, Jul 26, 2021 at 11:40:55AM +0300, Andy Shevchenko wrote:
> > > 
> > > ...
> > > 
> > > > >  err_free_swnodes:
> > > > >       software_node_unregister_nodes(sensor->swnodes);
> > > > >  err_put_adev:
> > > > > -     acpi_dev_put(sensor->adev);
> > > > > +     acpi_dev_put(adev);
> > > >
> > > > adev is assigned to sensor->adev before goto so the two have the same
> > > > value. I have no problem with the patch though.
> > > 
> > > Are we reading the same version? Or am I missing something?
> > 
> > Ah. I noticed the adev assignment was removed (and added later) by the
> > other patch. Yeah, agreed; this one's needed.
> 
> Thanks! Can we have your tag?

Apologies for the delay.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus