diff mbox series

iommu/omap: Fix regression in probe for NULL pointer dereference

Message ID 20220331062301.24269-1-tony@atomide.com
State New
Headers show
Series iommu/omap: Fix regression in probe for NULL pointer dereference | expand

Commit Message

Tony Lindgren March 31, 2022, 6:23 a.m. UTC
Commit 3f6634d997db ("iommu: Use right way to retrieve iommu_ops") started
triggering a NULL pointer dereference for some omap variants:

__iommu_probe_device from probe_iommu_group+0x2c/0x38
probe_iommu_group from bus_for_each_dev+0x74/0xbc
bus_for_each_dev from bus_iommu_probe+0x34/0x2e8
bus_iommu_probe from bus_set_iommu+0x80/0xc8
bus_set_iommu from omap_iommu_init+0x88/0xcc
omap_iommu_init from do_one_initcall+0x44/0x24

This is caused by omap iommu probe returning 0 instead of ERR_PTR(-ENODEV)
as noted by Jason Gunthorpe <jgg@ziepe.ca>.

Looks like the regression already happened with an earlier commit
6785eb9105e3 ("iommu/omap: Convert to probe/release_device() call-backs")
that changed the function return type and missed converting one place.

Cc: Drew Fustini <dfustini@baylibre.com>
Cc: Lu Baolu <baolu.lu@linux.intel.com>
Cc: Suman Anna <s-anna@ti.com>
Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
Fixes: 6785eb9105e3 ("iommu/omap: Convert to probe/release_device() call-backs")
Fixes: 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/iommu/omap-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tony Lindgren April 7, 2022, 5:39 a.m. UTC | #1
Hi,

* Tony Lindgren <tony@atomide.com> [220331 09:21]:
> Commit 3f6634d997db ("iommu: Use right way to retrieve iommu_ops") started
> triggering a NULL pointer dereference for some omap variants:
> 
> __iommu_probe_device from probe_iommu_group+0x2c/0x38
> probe_iommu_group from bus_for_each_dev+0x74/0xbc
> bus_for_each_dev from bus_iommu_probe+0x34/0x2e8
> bus_iommu_probe from bus_set_iommu+0x80/0xc8
> bus_set_iommu from omap_iommu_init+0x88/0xcc
> omap_iommu_init from do_one_initcall+0x44/0x24
> 
> This is caused by omap iommu probe returning 0 instead of ERR_PTR(-ENODEV)
> as noted by Jason Gunthorpe <jgg@ziepe.ca>.
> 
> Looks like the regression already happened with an earlier commit
> 6785eb9105e3 ("iommu/omap: Convert to probe/release_device() call-backs")
> that changed the function return type and missed converting one place.

Can you guys please get this fix into the -rc series? Or ack it so
I can pick it up into my fixes branch?

Without this fix booting is failing for a bunch of devices.

Regards,

Tony


> Cc: Drew Fustini <dfustini@baylibre.com>
> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> Cc: Suman Anna <s-anna@ti.com>
> Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
> Fixes: 6785eb9105e3 ("iommu/omap: Convert to probe/release_device() call-backs")
> Fixes: 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/iommu/omap-iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1661,7 +1661,7 @@ static struct iommu_device *omap_iommu_probe_device(struct device *dev)
>  	num_iommus = of_property_count_elems_of_size(dev->of_node, "iommus",
>  						     sizeof(phandle));
>  	if (num_iommus < 0)
> -		return 0;
> +		return ERR_PTR(-ENODEV);
>  
>  	arch_data = kcalloc(num_iommus + 1, sizeof(*arch_data), GFP_KERNEL);
>  	if (!arch_data)
> -- 
> 2.35.1
>
H. Nikolaus Schaller April 7, 2022, 6:43 a.m. UTC | #2
Hi,

> Am 07.04.2022 um 07:39 schrieb Tony Lindgren <tony@atomide.com>:
> 
> Hi,
> 
> * Tony Lindgren <tony@atomide.com> [220331 09:21]:
>> Commit 3f6634d997db ("iommu: Use right way to retrieve iommu_ops") started
>> triggering a NULL pointer dereference for some omap variants:
>> 
>> __iommu_probe_device from probe_iommu_group+0x2c/0x38
>> probe_iommu_group from bus_for_each_dev+0x74/0xbc
>> bus_for_each_dev from bus_iommu_probe+0x34/0x2e8
>> bus_iommu_probe from bus_set_iommu+0x80/0xc8
>> bus_set_iommu from omap_iommu_init+0x88/0xcc
>> omap_iommu_init from do_one_initcall+0x44/0x24
>> 
>> This is caused by omap iommu probe returning 0 instead of ERR_PTR(-ENODEV)
>> as noted by Jason Gunthorpe <jgg@ziepe.ca>.
>> 
>> Looks like the regression already happened with an earlier commit
>> 6785eb9105e3 ("iommu/omap: Convert to probe/release_device() call-backs")
>> that changed the function return type and missed converting one place.
> 
> Can you guys please get this fix into the -rc series? Or ack it so
> I can pick it up into my fixes branch?
> 
> Without this fix booting is failing for a bunch of devices.

Yes, I can confirm that v5.18-rc1 does not even boot the GTA04 (omap3)
and OMAP5UEVM to any activity without this patch.

Seems to be an urgent fix.

BR and thanks,
Nikolaus
Joerg Roedel April 8, 2022, 8:25 a.m. UTC | #3
On Thu, Apr 07, 2022 at 08:39:05AM +0300, Tony Lindgren wrote:
> Can you guys please get this fix into the -rc series? Or ack it so
> I can pick it up into my fixes branch?

Sorry for the delay, Covid catched me so I was away from email for
almost 2 week. This patch is picked-up now and I will send it upstream
for -rc2.

Thanks,

	Joerg
Tony Lindgren April 8, 2022, 9:40 a.m. UTC | #4
Hi,

* Joerg Roedel <joro@8bytes.org> [220408 08:23]:
> On Thu, Apr 07, 2022 at 08:39:05AM +0300, Tony Lindgren wrote:
> > Can you guys please get this fix into the -rc series? Or ack it so
> > I can pick it up into my fixes branch?
> 
> Sorry for the delay, Covid catched me so I was away from email for
> almost 2 week. This patch is picked-up now and I will send it upstream
> for -rc2.

OK welcome back then, and hopefully no serious symptoms. Thanks for
picking up the patch.

Regards,

Tony
diff mbox series

Patch

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1661,7 +1661,7 @@  static struct iommu_device *omap_iommu_probe_device(struct device *dev)
 	num_iommus = of_property_count_elems_of_size(dev->of_node, "iommus",
 						     sizeof(phandle));
 	if (num_iommus < 0)
-		return 0;
+		return ERR_PTR(-ENODEV);
 
 	arch_data = kcalloc(num_iommus + 1, sizeof(*arch_data), GFP_KERNEL);
 	if (!arch_data)