On 8/26/2016 7:43 PM, Kirti Wankhede wrote: > * PGP Signed: 08/26/2016 at 07:15:44 AM, Decrypted > On 8/25/2016 2:52 PM, Dong Jia wrote: >> On Thu, 25 Aug 2016 09:23:53 +0530
>>> + >>> +static ssize_t vfio_mdev_read(void *device_data, char __user *buf, >>> + size_t count, loff_t *ppos) >>> +{ >>> + struct vfio_mdev *vmdev = device_data; >>> + struct mdev_device *mdev = vmdev->mdev; >>> + struct parent_device *parent = mdev->parent; >>> + unsigned int done = 0; >>> + int ret; >>> + >>> + if (!parent->ops->read) >>> + return -EINVAL; >>> + >>> + while (count) { >> Here, I have to say sorry to you guys for that I didn't notice the >> bad impact of this change to my patches during the v6 discussion. >> >> For vfio-ccw, I introduced an I/O region to input/output I/O >> instruction parameters and results for Qemu. The @count of these data >> currently is 140. So supporting arbitrary lengths in one shot here, and >> also in vfio_mdev_write, seems the better option for this case. >> >> I believe that if the pci drivers want to iterate in a 4 bytes step, you >> can do that in the parent read/write callbacks instead. >> >> What do you think? >> > > I would like to know Alex's thought on this. He raised concern with this > approach in v6 reviews: > "But I think this is exploitable, it lets the user make the kernel > allocate an arbitrarily sized buffer." > Read/write callbacks are for slow path, emulation of mmio region which are mainly device registers. I do feel it shouldn't support arbitrary lengths. Alex, I would like to know your thoughts. Thanks, Kirti