On 10/18/2016 10:47 PM, Alex Williamson wrote:
> On Tue, 18 Oct 2016 10:54:11 +0800
> Dong Jia Shi <bjsdjshi@xxxxxxxxxxxxxxxxxx> wrote:
>
>> * Kirti Wankhede <kwankhede@xxxxxxxxxx> [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.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
