RE: [PATCH 1/4] iommu/vt-d: Fix null pointer access in invalidate_range callback

["Zhang, Tina" <tina.zhang@intel.com>]

Hi,
> -----Original Message-----
> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Wednesday, March 29, 2023 9:31 PM
> To: Zhang, Tina <tina.zhang@intel.com>; iommu@lists.linux.dev
> Cc: baolu.lu@linux.intel.com; Ma, Yongwei <yongwei.ma@intel.com>
> Subject: Re: [PATCH 1/4] iommu/vt-d: Fix null pointer access in invalidate_range
> callback
> 
> On 2023/3/29 20:46, Tina Zhang wrote:
> > Add the missing check to avoid null pointer dereference.
> >
> > The patch fixes below issue reported by klocwork tool:
> > Pointer 'info' returned from call to function 'dev_iommu_priv_get'
> > at line 180 may be NULL and may be dereferenced at line 186.
> >
> > This patch can fix a potential null pointer dereference issue of
> > releasing a device working in svm mode.
> 
> The svm logic guarantees that __flush_svm_range_dev() only be called after
> iommu_sva_bind_device() and before iommu_sva_unbind_device(). Thus @info
> should never be NULL. There's no need to add this check.
Yes, if the order can be ensured, no need to worry about the null pointer. How about hot-plugging devices? Can we keep the order all the time? Otherwise, it seems adding a null check makes sense.

Regards,
-Tina


> 
> Best regards,
> baolu
> 
> >
> > Reported-by: Yongwei Ma <yongwei.ma@intel.com>
> > Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> > ---
> >   drivers/iommu/intel/svm.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
> > index 7367f56c3bad..837c1a4642e7 100644
> > --- a/drivers/iommu/intel/svm.c
> > +++ b/drivers/iommu/intel/svm.c
> > @@ -179,7 +179,7 @@ static void __flush_svm_range_dev(struct intel_svm
> *svm,
> >   {
> >   	struct device_domain_info *info = dev_iommu_priv_get(sdev->dev);
> >
> > -	if (WARN_ON(!pages))
> > +	if (WARN_ON(!info || !pages))
> >   		return;
> >
> >   	qi_flush_piotlb(sdev->iommu, sdev->did, svm->pasid, address, pages,
> > ih);