[PATCH -next] iommu/of: Check for valid fwspec after of_pci_iommu_init
The successful return from of_pci_iommu_init doesn't ensure valid
fwspec if it's IOMMU is disabled.
Accessing dev->iommu_fwspec->ops without checking dev->iommu_fwspec
could result in NULL pointer dereference.
Unable to handle kernel NULL pointer dereference at virtual address
task: 80097688 task.stack: 800976888000
PC is at of_iommu_configure+0x130/0x138
LR is at of_iommu_configure+0x118/0x138
of_iommu_configure+0x130/0x138
of_dma_configure+0xa8/0x190
dma_configure+0x40/0xe8
driver_probe_device+0x190/0x2d0
__device_attach_driver+0x9c/0xf8
bus_for_each_drv+0x58/0x98
__device_attach+0xc4/0x138
device_attach+0x10/0x18
pci_bus_add_device+0x4c/0xa8
pci_bus_add_devices+0x44/0x90
pci_bus_add_devices+0x74/0x90
pci_host_common_probe+0x14c/0x3a8
gen_pci_probe+0x2c/0x38
platform_drv_probe+0x58/0xc0
driver_probe_device+0x214/0x2d0
__driver_attach+0xac/0xb0
bus_for_each_dev+0x60/0xa0
driver_attach+0x20/0x28
bus_add_driver+0x110/0x230
driver_register+0x60/0xf8
__platform_driver_register+0x44/0x50
gen_pci_driver_init+0x18/0x20
do_one_initcall+0x38/0x120
kernel_init_freeable+0x184/0x224
kernel_init+0x10/0x100
This patch adds the check for dev->iommu_fwspec and fixes the above.
Fixes: d87beb749281 ("iommu/of: Handle PCI aliases properly")
Cc: Robin Murphy
Signed-off-by: Sudeep Holla
---
drivers/iommu/of_iommu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Hi Robin,
I see this crash on Juno with linux-next. Enabling PCIe SMMU fixes the
issue, but we may need to handle the disabled SMMU case to continue to
work with old DTBs.
Regards,
Sudeep
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 34160e7a8dd7..11e08ff2db57 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -200,7 +200,8 @@ const struct iommu_ops *of_iommu_configure(struct device
*dev,
of_pci_iommu_init, &info);
if (err) /* err > 0 means the walk stopped, but non-fatally */
ops = ERR_PTR(min(err, 0));
- else /* success implies both fwspec and ops are now valid */
+ /* success may not imply fwspec is valid */
+ else if (dev->iommu_fwspec)
ops = dev->iommu_fwspec->ops;
} else {
struct of_phandle_args iommu_spec;
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH -next] iommu/of: Check for valid fwspec after of_pci_iommu_init
On 02/08/17 17:39, Robin Murphy wrote:
> On 02/08/17 17:11, Sudeep Holla wrote:
>> The successful return from of_pci_iommu_init doesn't ensure valid
>> fwspec if it's IOMMU is disabled.
>>
>> Accessing dev->iommu_fwspec->ops without checking dev->iommu_fwspec
>> could result in NULL pointer dereference.
>>
>> Unable to handle kernel NULL pointer dereference at virtual address
>> task: 80097688 task.stack: 800976888000
>> PC is at of_iommu_configure+0x130/0x138
>> LR is at of_iommu_configure+0x118/0x138
>> of_iommu_configure+0x130/0x138
>> of_dma_configure+0xa8/0x190
>> dma_configure+0x40/0xe8
>> driver_probe_device+0x190/0x2d0
>> __device_attach_driver+0x9c/0xf8
>> bus_for_each_drv+0x58/0x98
>> __device_attach+0xc4/0x138
>> device_attach+0x10/0x18
>> pci_bus_add_device+0x4c/0xa8
>> pci_bus_add_devices+0x44/0x90
>> pci_bus_add_devices+0x74/0x90
>> pci_host_common_probe+0x14c/0x3a8
>> gen_pci_probe+0x2c/0x38
>> platform_drv_probe+0x58/0xc0
>> driver_probe_device+0x214/0x2d0
>> __driver_attach+0xac/0xb0
>> bus_for_each_dev+0x60/0xa0
>> driver_attach+0x20/0x28
>> bus_add_driver+0x110/0x230
>> driver_register+0x60/0xf8
>> __platform_driver_register+0x44/0x50
>> gen_pci_driver_init+0x18/0x20
>> do_one_initcall+0x38/0x120
>> kernel_init_freeable+0x184/0x224
>> kernel_init+0x10/0x100
>>
>> This patch adds the check for dev->iommu_fwspec and fixes the above.
>>
>> Fixes: d87beb749281 ("iommu/of: Handle PCI aliases properly")
>> Cc: Robin Murphy
>> Signed-off-by: Sudeep Holla
>> ---
>> drivers/iommu/of_iommu.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> Hi Robin,
>>
>> I see this crash on Juno with linux-next. Enabling PCIe SMMU fixes the
>> issue, but we may need to handle the disabled SMMU case to continue to
>> work with old DTBs.
>
> Ugh, a disabled IOMMU should be conceptually the same as no IOMMU, but I
> guess the phandle chasing fails to fail. Per the design intent, the most
> correct fix should be this:
>
> ->8-
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index be8ac1ddec06..9bc83f94bc10 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -163,6 +163,8 @@ static int of_pci_iommu_init(struct pci_dev *pdev,
> u16 alias, void *data)
>
> if (IS_ERR(ops))
> return PTR_ERR(ops);
> + if (!ops)
> + return 1;
>
> return info->np == pdev->bus->dev.of_node;
> }
> -8<-
>
> But looking again, I think an even better fix involves ripping out much
> of the PTR_ERR shenanigans with ops that leads to this confusion in the
> first place. Give me a moment...
>
Sure, I will leave it to you then ;)
--
Regards,
Sudeep
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/of: Fix of_iommu_configure() for disabled IOMMUs
On 04/08/17 12:16, Robin Murphy wrote: > Sudeep reports that the logic is slightly broken when a PCI iommu-map > entry targets an IOMMU marked as disabled in DT, since of_pci_map_rid() > succeeds in following a phandle, and of_iommu_xlate() doesn't return an > error value, but we miss checking whether ops was actually non-NULL. > Whilst this could be solved with a point fix in of_pci_iommu_init(), it > suggests that all the juggling of ERR_PTR values through the ops pointer > is proving rather too complicated for its own good, so let's instead > simplify the whole flow (with a side-effect of eliminating the cause of > the bug). > > The fact that we now rely on iommu_fwspec means that we no longer need > to pass around an iommu_ops pointer at all - we can simply propagate a > regular int return value until we know whether we have a viable IOMMU, > then retrieve the ops from the fwspec if and when we actually need them. > This makes everything a bit more uniform and certainly easier to follow. > > Reported-by: Sudeep Holla Tested-by: Sudeep Holla -- Regards, Sudeep ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()
Hi, Saravana, On Fri, Jul 01, 2022 at 01:26:12AM -0700, Saravana Kannan wrote: [...] > Can you check if this hack helps? If so, then I can think about > whether we can pick it up without breaking everything else. Copy-paste > tab mess up warning. Sorry for jumping in late and not even sure if this is right thread. I have not bisected anything yet, but I am seeing issues on my Juno R2 with SCMI enabled power domains and Coresight AMBA devices. OF: amba_device_add() failed (-19) for /etf@2001 OF: amba_device_add() failed (-19) for /tpiu@2003 OF: amba_device_add() failed (-19) for /funnel@2004 OF: amba_device_add() failed (-19) for /etr@2007 OF: amba_device_add() failed (-19) for /stm@2010 OF: amba_device_add() failed (-19) for /replicator@2012 OF: amba_device_add() failed (-19) for /cpu-debug@2201 OF: amba_device_add() failed (-19) for /etm@2204 OF: amba_device_add() failed (-19) for /cti@2202 OF: amba_device_add() failed (-19) for /funnel@220c OF: amba_device_add() failed (-19) for /cpu-debug@2211 OF: amba_device_add() failed (-19) for /etm@2214 OF: amba_device_add() failed (-19) for /cti@2212 OF: amba_device_add() failed (-19) for /cpu-debug@2301 OF: amba_device_add() failed (-19) for /etm@2304 OF: amba_device_add() failed (-19) for /cti@2302 OF: amba_device_add() failed (-19) for /funnel@230c OF: amba_device_add() failed (-19) for /cpu-debug@2311 OF: amba_device_add() failed (-19) for /etm@2314 OF: amba_device_add() failed (-19) for /cti@2312 OF: amba_device_add() failed (-19) for /cpu-debug@2321 OF: amba_device_add() failed (-19) for /etm@2324 OF: amba_device_add() failed (-19) for /cti@2322 OF: amba_device_add() failed (-19) for /cpu-debug@2331 OF: amba_device_add() failed (-19) for /etm@2334 OF: amba_device_add() failed (-19) for /cti@2332 OF: amba_device_add() failed (-19) for /cti@2002 OF: amba_device_add() failed (-19) for /cti@2011 OF: amba_device_add() failed (-19) for /funnel@2013 OF: amba_device_add() failed (-19) for /etf@2014 OF: amba_device_add() failed (-19) for /funnel@2015 OF: amba_device_add() failed (-19) for /cti@2016 These are working fine with deferred probe in the mainline. I tried the hack you have suggested here(rather Tony's version), also tried with fw_devlink=0 and fw_devlink=1 && fw_devlink.strict=0 No change in the behaviour. The DTS are in arch/arm64/boot/dts/arm/juno-*-scmi.dts and there coresight devices are mostly in juno-cs-r1r2.dtsi Let me know if there is anything obvious or you want me to bisect which means I need more time. I can do that next week. -- Regards, Sudeep ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
