Re: [PATCH v9 12/12] docs: Sample driver to demonstrate how to use Mediated device framework.

[Kirti Wankhede <kwankhede@nvidia.com>]

On 10/18/2016 10:47 PM, Alex Williamson wrote:
> On Tue, 18 Oct 2016 10:54:11 +0800
> Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote:
> 
>> * Kirti Wankhede <kwankhede@nvidia.com> [2016-10-18 02:52:12 +0530]:
>>
>> ...snip...
>>
>>> +static ssize_t mdev_access(struct mdev_device *mdev, char *buf,
>>> +		size_t count, loff_t pos, bool is_write)
>>> +{
>>> +	struct mdev_state *mdev_state;
>>> +	unsigned int index;
>>> +	loff_t offset;
>>> +	int ret = 0;
>>> +
>>> +	if (!mdev || !buf)
>>> +		return -EINVAL;
>>> +
>>> +	mdev_state = mdev_get_drvdata(mdev);
>>> +	if (!mdev_state) {
>>> +		pr_err("%s mdev_state not found\n", __func__);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	mutex_lock(&mdev_state->ops_lock);
>>> +
>>> +	index = MTTY_VFIO_PCI_OFFSET_TO_INDEX(pos);
>>> +	offset = pos & MTTY_VFIO_PCI_OFFSET_MASK;
>>> +	switch (index) {
>>> +	case VFIO_PCI_CONFIG_REGION_INDEX:
>>> +
>>> +#if defined(DEBUG)
>>> +		pr_info("%s: PCI config space %s at offset 0x%llx\n",
>>> +			 __func__, is_write ? "write" : "read", offset);
>>> +#endif
>>> +		if (is_write) {
>>> +			dump_buffer(buf, count);
>>> +			handle_pci_cfg_write(mdev_state, offset, buf, count);
>>> +		} else {
>>> +			memcpy(buf, (mdev_state->vconfig + offset), count);
>>> +			dump_buffer(buf, count);  
>> Dear Kirti:
>>
>> Shouldn't we use copy_from_user instead of memcpy on @buf here? And I'm
>> wondering if dump_buffer could really work since it tries to dereference
>> a *__user* marked pointor.
> 
> I agree, the __user attribute is getting lost here and we're operating
> on user buffers as if they were kernel buffers.  That's a bug.  Thanks,
> 

Oh, yes. Thanks for catching that. While transiting all changes from v7
to v8 I missed to update these. I'll have this corrected.

Kirti.