Re: [PATCH v4 1/7] iommu: Prevent pasid attach if no ops->remove_dev_pasid

[Yi Liu <yi.l.liu@intel.com>]

On 2024/11/10 11:59, Baolu Lu wrote:
> On 11/8/24 20:04, Yi Liu wrote:
>> driver should implement both set_dev_pasid and remove_dev_pasid op, 
>> otherwise
>> it is a problem how to detach pasid. In reality, it is impossible that an
>> iommu driver implements set_dev_pasid() but no remove_dev_pasid() op. 
>> However,
>> it is better to check it.
>>
>> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>>   drivers/iommu/iommu.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 13fcd9d8f2df..1c689e57928e 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -3352,17 +3352,19 @@ int iommu_attach_device_pasid(struct iommu_domain 
>> *domain,
>>                     struct iommu_attach_handle *handle)
>>   {
>>       /* Caller must be a probed driver on dev */
>> +    const struct iommu_ops *ops = dev_iommu_ops(dev);
>>       struct iommu_group *group = dev->iommu_group;
>>       struct group_device *device;
>>       int ret;
>> -    if (!domain->ops->set_dev_pasid)
>> +    if (!domain->ops->set_dev_pasid ||
>> +        !ops->remove_dev_pasid)
>>           return -EOPNOTSUPP;
>>       if (!group)
>>           return -ENODEV;
> 
> If group is NULL, calling dev_iommu_ops() will trigger a kernel NULL
> pointer reference warning, which is unintended. If you need to check
> ops->remove_dev_pasid, it should be done after the group check.
> 

good catch. Let me fix it.

-- 
Regards,
Yi Liu