Re: [PATCH] iommu: fix an incorrect NULL check on list iterator

[Joerg Roedel]

On Sun, Mar 27, 2022 at 01:35:58PM +0800, Xiaomeng Tong wrote:
> @@ -617,23 +617,17 @@ static int qcom_iommu_of_xlate(struct device *dev,
>  {
>       struct msm_iommu_dev *iommu;
>       unsigned long flags;
> -     int ret = 0;
>  
>       spin_lock_irqsave(&msm_iommu_lock, flags);
>       list_for_each_entry(iommu, &qcom_iommu_devices, dev_node)
> -             if (iommu->dev->of_node == spec->np)
> -                     break;
> -
> -     if (!iommu || iommu->dev->of_node != spec->np) {
> -             ret = -ENODEV;
> -             goto fail;
> -     }
> -
> -     insert_iommu_master(dev, &iommu, spec);
> -fail:
> +             if (iommu->dev->of_node == spec->np) {
> +                     insert_iommu_master(dev, &iommu, spec);
> +                     spin_unlock_irqrestore(&msm_iommu_lock, flags);
> +                     return 0;
> +             }
>       spin_unlock_irqrestore(&msm_iommu_lock, flags);
>  
> -     return ret;
> +     return -ENODEV;

This looks a bit clumsy, a better fix is below:

diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 50f57624610f..98d23c52537b 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -610,14 +610,16 @@ static void insert_iommu_master(struct device *dev,
 static int qcom_iommu_of_xlate(struct device *dev,
                               struct of_phandle_args *spec)
 {
-       struct msm_iommu_dev *iommu;
+       struct msm_iommu_dev *iommu = NULL, *it;
        unsigned long flags;
        int ret = 0;
 
        spin_lock_irqsave(&msm_iommu_lock, flags);
-       list_for_each_entry(iommu, &qcom_iommu_devices, dev_node)
-               if (iommu->dev->of_node == spec->np)
+       list_for_each_entry(it, &qcom_iommu_devices, dev_node)
+               if (it->dev->of_node == spec->np) {
+                       iommu = it;
                        break;
+               }
 
        if (!iommu || iommu->dev->of_node != spec->np) {
                ret = -ENODEV;

Can you please verify this and re-submit?

Thanks,

        Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu