>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
>Sent: Thursday, December 12, 2024 3:45 PM
>Subject: Re: [PATCH] iommu/vt-d: Fix kernel NULL pointer dereference in
>cache_tag_flush_range_np()
>
>On 2024/12/12 15:24, Zhenzhong Duan wrote:
>> When setup mapping on an paging domain before the domain is attached to
>any
>> device, a NULL pointer dereference triggers as below:
>>
>> BUG: kernel NULL pointer dereference, address: 0000000000000200
>> #PF: supervisor read access in kernel mode
>> #PF: error_code(0x0000) - not-present page
>> RIP: 0010:cache_tag_flush_range_np+0x114/0x1f0
>> ...
>> Call Trace:
>> <TASK>
>> ? __die+0x20/0x70
>> ? page_fault_oops+0x80/0x150
>> ? do_user_addr_fault+0x5f/0x670
>> ? pfn_to_dma_pte+0xca/0x280
>> ? exc_page_fault+0x78/0x170
>> ? asm_exc_page_fault+0x22/0x30
>> ? cache_tag_flush_range_np+0x114/0x1f0
>> intel_iommu_iotlb_sync_map+0x16/0x20
>> iommu_map+0x59/0xd0
>> batch_to_domain+0x154/0x1e0
>> iopt_area_fill_domains+0x106/0x300
>> iopt_map_pages+0x1bc/0x290
>> iopt_map_user_pages+0xe8/0x1e0
>> ? xas_load+0x9/0xb0
>> iommufd_ioas_map+0xc9/0x1c0
>> iommufd_fops_ioctl+0xff/0x1b0
>> __x64_sys_ioctl+0x87/0xc0
>> do_syscall_64+0x50/0x110
>> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>
>> qi_batch structure is allocated when domain is attached to a device for the
>> first time, when a mapping is setup before that, qi_batch is referenced to
>> do batched flush and trigger above issue.
>>
>> Fix it by checking qi_batch pointer and bypass batched flushing if no
>> device is attached.
>>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Fixes: 705c1cdf1e73 ("iommu/vt-d: Introduce batched cache invalidation")
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxx>
>> ---
>> drivers/iommu/intel/cache.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c
>> index e5b89f728ad3..bb9dae9a7fba 100644
>> --- a/drivers/iommu/intel/cache.c
>> +++ b/drivers/iommu/intel/cache.c
>> @@ -264,7 +264,7 @@ static unsigned long
>calculate_psi_aligned_address(unsigned long start,
>>
>> static void qi_batch_flush_descs(struct intel_iommu *iommu, struct qi_batch
>*batch)
>> {
>> - if (!iommu || !batch->index)
>> + if (!iommu || !batch || !batch->index)
>
>this is the same issue in the below link. :) And we should fix it by
>allocating the qi_batch for parent domains. The nested parent domains is
>not going to be attached to device at all. It acts more as a background
>domain, so this fix will miss the future cache flushes. It would have
>bigger problems. :)
>
>https://lore.kernel.org/linux-iommu/20241210130322.17175-1-
>yi.l.liu@xxxxxxxxx/
Ah, just see this😊
This patch tries to fix an issue that mapping setup on a paging domain before
it's attached to a device, your patch fixed an issue that nested parent
domain's qi_batch is not allocated even if nested domain is attached to
a device. I think they are different issues?
Thanks
Zhenzhong
