[PATCH] iommu: Check return of __iommu_attach_device()
Robin Murphy
robin.murphy at arm.com
Fri Nov 20 09:07:26 EST 2020
On 2020-11-20 11:15, Will Deacon wrote:
> On Thu, Nov 19, 2020 at 04:58:46PM +0000, Shameer Kolothum wrote:
>> Currently iommu_create_device_direct_mappings() is called
>> without checking the return of __iommu_attach_device(). This
>> may result in failures in iommu driver if dev attach returns
>> error.
>>
>> Fixes: ce574c27ae27("iommu: Move iommu_group_create_direct_mappings() out of iommu_group_add_device()")
>> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi at huawei.com>
>> ---
>> Crash log:
>> [ 31.353605] hns3 0000:7d:00.3: Adding to iommu group 10
>> [ 31.358822] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000018
>> [ 31.367567] Mem abort info:
>> [ 31.370350] ESR = 0x96000004
>> [ 31.373391] EC = 0x25: DABT (current EL), IL = 32 bits
>> [ 31.378680] SET = 0, FnV = 0
>> [ 31.381720] EA = 0, S1PTW = 0
>> [ 31.384847] Data abort info:
>> [ 31.387716] ISV = 0, ISS = 0x00000004
>> [ 31.391535] CM = 0, WnR = 0
>> [ 31.394491] [0000000000000018] user address but active_mm is swapper
>> [ 31.400818] Internal error: Oops: 96000004 [#1] PREEMPT SMP
>> [ 31.406365] Modules linked in:
>> [ 31.409409] CPU: 21 PID: 1 Comm: swapper/0 Not tainted 5.10.0-rc4-00008-gdd5aba9d719-dirty #79
>> [ 31.417980] Hardware name: Huawei TaiShan 200 (Model 2280)/BC82AMDD, BIOS 2280-V2 CS V3.B220.01 03/19/2020
>> [ 31.427588] pstate: 00c00009 (nzcv daif +PAN +UAO -TCO BTYPE=--)
>> [ 31.433566] pc : arm_smmu_tlb_inv_range+0x178/0x1f0
>> [ 31.438422] lr : arm_smmu_tlb_inv_range+0x5c/0x1f0
>> [ 31.443190] sp : ffff80001043b4e0
>> ...
>> [ 31.531175] Call trace:
>> [ 31.533613] arm_smmu_tlb_inv_range+0x178/0x1f0
>> [ 31.538122] arm_smmu_iotlb_sync+0x2c/0x38
>> [ 31.542200] iommu_unmap+0x60/0x90
>> [ 31.545585] __iommu_map+0x110/0x1f0
>> [ 31.549144] iommu_create_device_direct_mappings.isra.34+0x1ac/0x250
>> [ 31.555468] iommu_probe_device+0x6c/0x110
>> [ 31.559551] iort_iommu_configure_id+0x114/0x218
>> [ 31.564148] acpi_dma_configure_id+0x94/0xe0
>> [ 31.568402] pci_dma_configure+0xc8/0xf0
>> [ 31.572310] really_probe+0xd4/0x3e0
>> [ 31.575871] driver_probe_device+0x5c/0xc0
>>
>> ---
>> drivers/iommu/iommu.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index b53446bb8c6b..0f4dc25d46c9 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -264,16 +264,18 @@ int iommu_probe_device(struct device *dev)
>> */
>> iommu_alloc_default_domain(group, dev);
>>
>> - if (group->default_domain)
>> + if (group->default_domain) {
>> ret = __iommu_attach_device(group->default_domain, dev);
>> + if (ret) {
>> + iommu_group_put(group);
>> + goto err_release;
>> + }
>> + }
>
> This looks sensible to me, but what I don't understand is where that
> NULL pointer is coming from in the first place. iommu_map() operates
> on the domain, so why does it matter if the attach fails? What is being
> accessed at arm_smmu_tlb_inv_range+0x178/0x1f0 ?
Probably because the domain is a hollow fake until the first successful
attach - even TLB maintenance depends on having decided a pagetable format.
Robin.
More information about the linux-arm-kernel
mailing list