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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 20 Oct 2016 00:49:19 +0530
From:   Kirti Wankhede <kwankhede@...dia.com>
To:     Alex Williamson <alex.williamson@...hat.com>,
        Dong Jia Shi <bjsdjshi@...ux.vnet.ibm.com>
CC:     <pbonzini@...hat.com>, <kraxel@...hat.com>, <cjia@...dia.com>,
        <qemu-devel@...gnu.org>, <kvm@...r.kernel.org>,
        <kevin.tian@...el.com>, <jike.song@...el.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 12/12] docs: Sample driver to demonstrate how to use
 Mediated device framework.



On 10/18/2016 10:47 PM, Alex Williamson wrote:
> On Tue, 18 Oct 2016 10:54:11 +0800
> Dong Jia Shi <bjsdjshi@...ux.vnet.ibm.com> wrote:
> 
>> * Kirti Wankhede <kwankhede@...dia.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.