Re: [Qemu-devel] [PATCH v7 2/4] vfio: VFIO driver for mediated devices

[Kirti Wankhede <kwankhede@nvidia.com>]

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