Re: [PATCH 1/4] iommu/sva: Set handle->dev before the SVA handle is visible

Shuai Xue <[email protected]> Mon, 27 Jul 2026 18:14:57 +0800
Newsgroups dev.linux.lists.iommu,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

On 7/27/26 5:38 PM, Tian, Kevin wrote:
>> From: Shuai Xue <[email protected]>
>> Sent: Sunday, July 26, 2026 3:43 PM
>>
>> iommu_attach_device_pasid() installs the new SVA attach handle in the
>> group PASID lookup before iommu_sva_bind_device() returns. A concurrent
>> bind can therefore find and reuse the same handle after iommu_sva_lock is
>> dropped.
>>
>> handle->dev was initialized after dropping iommu_sva_lock. This leaves a
>> window where a racing bind can return a handle whose dev pointer is still
>> NULL. A subsequent iommu_sva_unbind_device() can then dereference it via
>> handle->dev->iommu_group.
> 
> ideally this will bail out early:
> 
>          mutex_lock(&iommu_sva_lock);
>          if (!refcount_dec_and_test(&handle->users)) {
>                  mutex_unlock(&iommu_sva_lock);
>                  return;
>          }
> 

Thanks Kevin for pointing this out.

Yes, the non-last reference case should bail out before using
handle->dev. I will wait a bit for other review comments and then decide
whether a v2 is needed to clarify the commit message.

>>
>> Initialize handle->dev before releasing iommu_sva_lock so any visible SVA
>> handle is fully initialized.
>>
>> Fixes: be51b1d6bbff ("iommu/sva: Refactoring
>> iommu_sva_bind/unbind_device()")
>> Cc: [email protected]
>> Assisted-by: Qoder:Qwen-3.8-MAX-Preview
>> Signed-off-by: Shuai Xue <[email protected]>
> 
> but the fix is correct.
> 
> Reviewed-by: Kevin Tian <[email protected]>

Thanks for the review.

Best Regards,
Shuai