Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S944191AbcJSTTf (ORCPT <rfc822;w@1wt.eu>);
        Wed, 19 Oct 2016 15:19:35 -0400
Received: from hqemgate15.nvidia.com ([216.228.121.64]:7625 "EHLO
        hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S932977AbcJSTTd (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 19 Oct 2016 15:19:33 -0400
X-PGP-Universal: processed;
        by hqnvupgp07.nvidia.com on Wed, 19 Oct 2016 12:12:22 -0700
Subject: Re: [PATCH v9 12/12] docs: Sample driver to demonstrate how to use
 Mediated device framework.
To: Alex Williamson <alex.williamson@redhat.com>,
        Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
References: <1476739332-4911-1-git-send-email-kwankhede@nvidia.com>
 <1476739332-4911-13-git-send-email-kwankhede@nvidia.com>
 <20161018025411.GA22572@bjsdjshi@linux.vnet.ibm.com>
 <20161018111719.141fcdaa@t450s.home>
CC: <pbonzini@redhat.com>, <kraxel@redhat.com>, <cjia@nvidia.com>,
        <qemu-devel@nongnu.org>, <kvm@vger.kernel.org>, <kevin.tian@intel.com>,
        <jike.song@intel.com>, <linux-kernel@vger.kernel.org>
From: Kirti Wankhede <kwankhede@nvidia.com>
Message-ID: <2b279b5a-c54b-b088-9329-d0ae5c690d76@nvidia.com>
Date: Thu, 20 Oct 2016 00:49:19 +0530
MIME-Version: 1.0
In-Reply-To: <20161018111719.141fcdaa@t450s.home>
X-Originating-IP: [10.24.71.144]
X-ClientProxiedBy: BGMAIL103.nvidia.com (10.25.59.12) To bgmail102.nvidia.com
 (10.25.59.11)
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1770
Lines: 60



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.