Re: [PATCH v5 03/12] iommufd: Move the iommufd_handle helpers to device.c

[Baolu Lu <baolu.lu@linux.intel.com>]

On 11/4/24 21:25, Yi Liu wrote:
> The iommu_attach_handle is now only passed when attaching iopf-capable
> domain, while it is not convenient for the iommu core to track the
> attached domain of pasids. To address it, the iommu_attach_handle will
> be passed to iommu core for non-fault-able domain as well. Hence the
> iommufd_handle related helpers are no longer fault specific, it makes
> more sense to move it out of fault.c.
> 
> Signed-off-by: Yi Liu<yi.l.liu@intel.com>
> ---
>   drivers/iommu/iommufd/device.c          | 51 ++++++++++++++++++++++
>   drivers/iommu/iommufd/fault.c           | 56 +------------------------
>   drivers/iommu/iommufd/iommufd_private.h |  8 ++++
>   3 files changed, 61 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index 5fd3dd420290..823c81145214 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -293,6 +293,57 @@ u32 iommufd_device_to_id(struct iommufd_device *idev)
>   }
>   EXPORT_SYMBOL_NS_GPL(iommufd_device_to_id, IOMMUFD);
>   
> +struct iommufd_attach_handle *
> +iommufd_device_get_attach_handle(struct iommufd_device *idev)
> +{
> +	struct iommu_attach_handle *handle;
> +
> +	handle = iommu_attach_handle_get(idev->igroup->group, IOMMU_NO_PASID, 0);
> +	if (IS_ERR(handle))
> +		return NULL;
> +
> +	return to_iommufd_handle(handle);
> +}

I would suggest placing this helper closer to where it is used. Because,
there is currently no locking mechanism to synchronize threads that
access the returned handle with those attaching or replacing the domain.
This lack of synchronization could potentially lead to use-after-free
issue.

By placing the helper near its callers and perhaps adding comments
explaining this limitation, we can improve maintainability and prevent
misuse in the future.

--
baolu