[PATCH 6.12.y] iommu/vt-d: Avoid use of NULL after WARN_ON_ONCE

[PATCH 6.12.y] iommu/vt-d: Avoid use of NULL after WARN_ON_ONCE

[Rajani kantha]

From: Kees Bakker <kees@ijzerbout.nl>

[ Upstream commit 60f030f7418d3f1d94f2fb207fe3080e1844630b ]

There is a WARN_ON_ONCE to catch an unlikely situation when
domain_remove_dev_pasid can't find the `pasid`. In case it nevertheless
happens we must avoid using a NULL pointer.

Signed-off-by: Kees Bakker <kees@ijzerbout.nl>
Link: https://lore.kernel.org/r/20241218201048.E544818E57E@bout3.ijzerbout.nl
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Rajani Kantha <rajanikantha@engineer.com>
---
 drivers/iommu/intel/iommu.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 56e9f125cda9..7c351274d004 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4306,13 +4306,14 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid,
                        break;
                }
        }
-       WARN_ON_ONCE(!dev_pasid);
        spin_unlock_irqrestore(&dmar_domain->lock, flags);

        cache_tag_unassign_domain(dmar_domain, dev, pasid);
        domain_detach_iommu(dmar_domain, iommu);
-       intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
-       kfree(dev_pasid);
+       if (!WARN_ON_ONCE(!dev_pasid)) {
+               intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
+               kfree(dev_pasid);
+       }
        intel_pasid_tear_down_entry(iommu, dev, pasid, false);
        intel_drain_pasid_prq(dev, pasid);
 }
--
2.34.1

Re: [PATCH 6.12.y] iommu/vt-d: Avoid use of NULL after WARN_ON_ONCE

[Greg KH]

On Wed, Jul 02, 2025 at 04:47:24AM +0200, Rajani kantha wrote:
> From: Kees Bakker <kees@ijzerbout.nl>
> 
> [ Upstream commit 60f030f7418d3f1d94f2fb207fe3080e1844630b ]
> 
> There is a WARN_ON_ONCE to catch an unlikely situation when
> domain_remove_dev_pasid can't find the `pasid`. In case it nevertheless
> happens we must avoid using a NULL pointer.
> 
> Signed-off-by: Kees Bakker <kees@ijzerbout.nl>
> Link: https://lore.kernel.org/r/20241218201048.E544818E57E@bout3.ijzerbout.nl
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> Signed-off-by: Rajani Kantha <rajanikantha@engineer.com>
> ---
>  drivers/iommu/intel/iommu.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 56e9f125cda9..7c351274d004 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4306,13 +4306,14 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid,
>                         break;
>                 }
>         }
> -       WARN_ON_ONCE(!dev_pasid);
>         spin_unlock_irqrestore(&dmar_domain->lock, flags);
> 
>         cache_tag_unassign_domain(dmar_domain, dev, pasid);
>         domain_detach_iommu(dmar_domain, iommu);
> -       intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
> -       kfree(dev_pasid);
> +       if (!WARN_ON_ONCE(!dev_pasid)) {
> +               intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
> +               kfree(dev_pasid);
> +       }

Meta-comment about this patch.  If this does trigger, it will still
crash the billions of Linux instances that run with panic-on-warn
enabled.  So you really haven't "solved" the issue here.  If this can be
NULL, then properly handle it please, don't crash boxes...

thanks,

greg k-h

Re: [PATCH 6.12.y] iommu/vt-d: Avoid use of NULL after WARN_ON_ONCE

[Greg KH]

On Wed, Jul 02, 2025 at 04:47:24AM +0200, Rajani kantha wrote:
> From: Kees Bakker <kees@ijzerbout.nl>
> 
> [ Upstream commit 60f030f7418d3f1d94f2fb207fe3080e1844630b ]
> 
> There is a WARN_ON_ONCE to catch an unlikely situation when
> domain_remove_dev_pasid can't find the `pasid`. In case it nevertheless
> happens we must avoid using a NULL pointer.
> 
> Signed-off-by: Kees Bakker <kees@ijzerbout.nl>
> Link: https://lore.kernel.org/r/20241218201048.E544818E57E@bout3.ijzerbout.nl
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> Signed-off-by: Rajani Kantha <rajanikantha@engineer.com>
> ---
>  drivers/iommu/intel/iommu.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 56e9f125cda9..7c351274d004 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4306,13 +4306,14 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid,
>                         break;
>                 }
>         }
> -       WARN_ON_ONCE(!dev_pasid);
>         spin_unlock_irqrestore(&dmar_domain->lock, flags);
> 
>         cache_tag_unassign_domain(dmar_domain, dev, pasid);
>         domain_detach_iommu(dmar_domain, iommu);
> -       intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
> -       kfree(dev_pasid);
> +       if (!WARN_ON_ONCE(!dev_pasid)) {
> +               intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
> +               kfree(dev_pasid);
> +       }
>         intel_pasid_tear_down_entry(iommu, dev, pasid, false);
>         intel_drain_pasid_prq(dev, pasid);
>  }
> --
> 2.34.1
> 

Does not apply to the 6.12.y branch at all, what was this made against?

thanks,

greg k-h