Re: [REGRESSION] qcom: iommu: nullpointer dereference on boot on apq8064

[Robin Murphy <robin.murphy@arm.com>]

On 2026-01-05 6:02 pm, Jason Gunthorpe wrote:
> On Mon, Dec 29, 2025 at 11:26:42PM +0100, Christian Schrefl wrote:
>> Hi everyone,
>>
>> I've found a panic on boot with v6.19-rc3 on the asus-nexus7-flo tablet with a APQ8064 CPU.
>>
>> I've bisected it down to commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
>> probe path"). Reverting the drivers/iommu/iommu.c changes (removing the added if block)
>> fixes the crash, but that presumably exists for a reason.
>> The diff for the fix:
>> ```
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 2ca990dfbb88..9f32d70b207d 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -453,14 +453,6 @@ static int iommu_init_device(struct device *dev)
>>           * already having a driver bound means dma_configure has already run and
>>           * found no IOMMU to wait for, so there's no point calling it again.
>>           */
>> -       if (!dev->iommu->fwspec && !dev->driver && dev->bus->dma_configure) {
>> -               mutex_unlock(&iommu_probe_device_lock);
>> -               dev->bus->dma_configure(dev);
>> -               mutex_lock(&iommu_probe_device_lock);
>> -               /* If another instance finished the job for us, skip it */
>> -               if (!dev->iommu || dev->iommu_group)
>> -                       return -ENODEV;
>> -       }
>>          /*
>>           * At this point, relevant devices either now have a fwspec which will
>>           * match ops registered with a non-NULL fwnode, or we can reasonably
>> ```
> 
>> [    5.900971] Call trace:
>> [    5.900999]  qcom_iommu_of_xlate from of_iommu_xlate+0x7c/0x9c
> 
> Did you look at what line in qcom_iommu_of_xlate() is crashing with
> NULL pointer?
> 
> It wasn't so obvious to me what it could be..

Comparing the "Code:" line from the dump against a local build, this
looks to be the dereference of master->num_mids at the start of the
loop.

> Though this looks really weird:
> 
>          struct msm_iommu_ctx_dev *master = dev_iommu_priv_get(dev);
>          int sid;
> 
>          if (list_empty(&(*iommu)->ctx_list)) {
>                  master = kzalloc(sizeof(*master), GFP_ATOMIC);
> 
> So maybe master is NULL and !list_empty()?

AFAICS that could happen if of_xlate has run once, and then for whatever
reason dev->iommu is torn down and the whole process started from
scratch a second time - although I'm struggling to see any obvious cause
for that in the absence of any other visible errors or async probe
races (and assuming that the IOMMUs on this platform ever actually
worked at all, of course...)

However it certainly stands out as a bit wrong that of_xlate is touching
the IOMMU instance itself - that should never have been expected to work
well. Does the diff below help?

Thanks,
Robin.

----->8-----
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 819add75a665..e57780fc3287 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -360,14 +360,11 @@ static int msm_iommu_domain_config(struct msm_priv *priv)
  /* Must be called under msm_iommu_lock */
  static struct msm_iommu_dev *find_iommu_for_dev(struct device *dev)
  {
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
  	struct msm_iommu_dev *iommu, *ret = NULL;
-	struct msm_iommu_ctx_dev *master;
  
  	list_for_each_entry(iommu, &qcom_iommu_devices, dev_node) {
-		master = list_first_entry(&iommu->ctx_list,
-					  struct msm_iommu_ctx_dev,
-					  list);
-		if (master->of_node == dev->of_node) {
+		if (iommu->dev->fwnode == fwspec->iommu_fwnode) {
  			ret = iommu;
  			break;
  		}
@@ -378,6 +375,7 @@ static struct msm_iommu_dev *find_iommu_for_dev(struct device *dev)
  
  static struct iommu_device *msm_iommu_probe_device(struct device *dev)
  {
+	struct msm_iommu_ctx_dev *master;
  	struct msm_iommu_dev *iommu;
  	unsigned long flags;
  
@@ -388,6 +386,8 @@ static struct iommu_device *msm_iommu_probe_device(struct device *dev)
  	if (!iommu)
  		return ERR_PTR(-ENODEV);
  
+	master = dev_iommu_priv_get(dev);
+	list_add(&master->list, &iommu->ctx_list);
  	return &iommu->iommu;
  }
  
@@ -604,14 +604,13 @@ static int insert_iommu_master(struct device *dev,
  	struct msm_iommu_ctx_dev *master = dev_iommu_priv_get(dev);
  	int sid;
  
-	if (list_empty(&(*iommu)->ctx_list)) {
+	if (!master) {
  		master = kzalloc(sizeof(*master), GFP_ATOMIC);
  		if (!master) {
  			dev_err(dev, "Failed to allocate iommu_master\n");
  			return -ENOMEM;
  		}
  		master->of_node = dev->of_node;
-		list_add(&master->list, &(*iommu)->ctx_list);
  		dev_iommu_priv_set(dev, master);
  	}