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

[Kirti Wankhede]

On 10/18/2016 10:47 PM, Alex Williamson wrote:
> On Tue, 18 Oct 2016 10:54:11 +0800
> Dong Jia Shi <address@hidden> wrote:
> 
>> * Kirti Wankhede <address@hidden> [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.