> From: Jason Gunthorpe <[email protected]>
> Sent: Tuesday, March 28, 2023 8:40 PM
> 
> On Mon, Mar 27, 2023 at 02:34:58AM -0700, Yi Liu wrote:
> 
> > +   devices = kcalloc(count, sizeof(*devices), GFP_KERNEL);
> > +   if (!devices) {
> > +           ret = -ENOMEM;
> > +           goto reset_info_exit;
> > +   }
> 
> This doesn't need to be so complicated
> 
> > +   list_for_each_entry(cur, &vdev->vdev.dev_set->device_list,
> vdev.dev_set_list) {
> > +           cur_iommufd = vfio_iommufd_physical_ictx(&cur->vdev);
> > +           if (cur->vdev.open_count) {
> > +                   if (cur_iommufd != iommufd) {
> > +                           ret = -EPERM;
> > +                           break;
> > +                   }
> > +                   ret = vfio_iommufd_physical_devid(&cur->vdev,
> &devices[index]);
> 
> 
> u32 device;
> 
> if (index >= hdr.count)
>    return -ENOSPC;
> 
> ret = vfio_iommufd_physical_devid(&cur->vdev, &devices);

Ok, so the whole point is that if  cur->vdev->iommufd_ctx==null, then the
vfio_iommufd_physical_devid() shall fail as well.

> ...
> 
> if (put_user(&arg->devices[index], device))

Will modify it. let's close the "separate ioctl" v.s. "reuse ioctl + new flag" 
open with
Alex first.

Thanks,
Yi Liu

>    -EFAULT
> 
> index++;
> 
> Jason

Reply via email to