> From: Jason Gunthorpe <jgg@xxxxxxxxxx> > 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