| From 229e6ee43d2a160a1592b83aad620d6027084aad Mon Sep 17 00:00:00 2001 |
| From: Pratyush Brahma <quic_pbrahma@quicinc.com> |
| Date: Fri, 4 Oct 2024 14:34:28 +0530 |
| Subject: iommu/arm-smmu: Defer probe of clients after smmu device bound |
| |
| From: Pratyush Brahma <quic_pbrahma@quicinc.com> |
| |
| commit 229e6ee43d2a160a1592b83aad620d6027084aad upstream. |
| |
| Null pointer dereference occurs due to a race between smmu |
| driver probe and client driver probe, when of_dma_configure() |
| for client is called after the iommu_device_register() for smmu driver |
| probe has executed but before the driver_bound() for smmu driver |
| has been called. |
| |
| Following is how the race occurs: |
| |
| T1:Smmu device probe T2: Client device probe |
| |
| really_probe() |
| arm_smmu_device_probe() |
| iommu_device_register() |
| really_probe() |
| platform_dma_configure() |
| of_dma_configure() |
| of_dma_configure_id() |
| of_iommu_configure() |
| iommu_probe_device() |
| iommu_init_device() |
| arm_smmu_probe_device() |
| arm_smmu_get_by_fwnode() |
| driver_find_device_by_fwnode() |
| driver_find_device() |
| next_device() |
| klist_next() |
| /* null ptr |
| assigned to smmu */ |
| /* null ptr dereference |
| while smmu->streamid_mask */ |
| driver_bound() |
| klist_add_tail() |
| |
| When this null smmu pointer is dereferenced later in |
| arm_smmu_probe_device, the device crashes. |
| |
| Fix this by deferring the probe of the client device |
| until the smmu device has bound to the arm smmu driver. |
| |
| Fixes: 021bb8420d44 ("iommu/arm-smmu: Wire up generic configuration support") |
| Cc: stable@vger.kernel.org |
| Co-developed-by: Prakash Gupta <quic_guptap@quicinc.com> |
| Signed-off-by: Prakash Gupta <quic_guptap@quicinc.com> |
| Signed-off-by: Pratyush Brahma <quic_pbrahma@quicinc.com> |
| Link: https://lore.kernel.org/r/20241004090428.2035-1-quic_pbrahma@quicinc.com |
| [will: Add comment] |
| Signed-off-by: Will Deacon <will@kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/iommu/arm/arm-smmu/arm-smmu.c | 11 +++++++++++ |
| 1 file changed, 11 insertions(+) |
| |
| --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c |
| +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c |
| @@ -1437,6 +1437,17 @@ static struct iommu_device *arm_smmu_pro |
| goto out_free; |
| } else { |
| smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode); |
| + |
| + /* |
| + * Defer probe if the relevant SMMU instance hasn't finished |
| + * probing yet. This is a fragile hack and we'd ideally |
| + * avoid this race in the core code. Until that's ironed |
| + * out, however, this is the most pragmatic option on the |
| + * table. |
| + */ |
| + if (!smmu) |
| + return ERR_PTR(dev_err_probe(dev, -EPROBE_DEFER, |
| + "smmu dev has not bound yet\n")); |
| } |
| |
| ret = -EINVAL; |