On Thu, Nov 03, 2022 at 05:11:00AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > Sent: Wednesday, October 26, 2022 2:12 AM
> >
> > From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> >
> > These complement the group interfaces and are for use by VFIO. The main
>
> s/VFIO/iommufd/
Done:
These complement the group interfaces used by VFIO and are for use by
iommufd. The main difference is that multiple devices in the same group
can all share the ownership by passing the same ownership pointer.
> > difference is that multiple devices in the same group can all share the
> > ownership by passing the same ownership pointer.
> >
> > Move the common code into shared functions.
> >
> > Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > ---
> > drivers/iommu/iommu.c | 116 +++++++++++++++++++++++++++++++++-------
>
> remove the space before -EBUSY
Done
> > +int iommu_device_claim_dma_owner(struct device *dev, void *owner)
> > {
> > - int ret;
> > + struct iommu_group *group = iommu_group_get(dev);
> > + int ret = 0;
> > +
> > + if (!group)
> > + return -ENODEV;
> >
> > mutex_lock(&group->mutex);
> > - if (WARN_ON(!group->owner_cnt || !group->owner))
> > + if (group->owner_cnt) {
> > + if (group->owner != owner) {
> > + ret = -EPERM;
> > + goto unlock_out;
> > + }
>
> check owner!=NULL otherwise this call may inadvertently succeed
> if the caller assigns a NULL owner while the group has already been
> grabbed by a kernel driver.
Ah, this is a missed assertion we have already - basically owner ==
NULL is not a valid user owner since we are using it to mean 'dma API
owner'.
So, like this:
@@ -3112,6 +3112,9 @@ static int __iommu_take_dma_ownership(struct iommu_group *group, void *owner)
{
int ret;
+ if (WARN_ON(!owner))
+ return -EINVAL;
+
if ((group->domain && group->domain != group->default_domain) ||
!xa_empty(&group->pasid_array))
return -EBUSY;
@@ -3141,6 +3144,9 @@ int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner)
{
int ret = 0;
+ if (WARN_ON(!owner))
+ return -EINVAL;
+
mutex_lock(&group->mutex);
if (group->owner_cnt) {
ret = -EPERM;
Thanks,
Jason
