From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1NwUik-00009e-7o
	for qemu-devel@nongnu.org; Tue, 30 Mar 2010 02:14:38 -0400
Received: from [140.186.70.92] (port=33462 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1NwUig-0008Lb-EA
	for qemu-devel@nongnu.org; Tue, 30 Mar 2010 02:14:35 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <john.cooper@redhat.com>) id 1NwUWf-0000XH-OP
	for qemu-devel@nongnu.org; Tue, 30 Mar 2010 02:02:12 -0400
Received: from mx1.redhat.com ([209.132.183.28]:34146)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <john.cooper@redhat.com>) id 1NwUWf-0000X8-6F
	for qemu-devel@nongnu.org; Tue, 30 Mar 2010 02:02:09 -0400
Message-ID: <4BB191A1.5060101@redhat.com>
Date: Tue, 30 Mar 2010 01:52:33 -0400
From: john cooper <john.cooper@redhat.com>
MIME-Version: 1.0
References: <4BAAF5CA.2010401@redhat.com>
	<201003301553.29570.rusty@rustcorp.com.au>
In-Reply-To: <201003301553.29570.rusty@rustcorp.com.au>
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH 4/4] Add virtio disk identification support
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Rusty Russell <rusty@rustcorp.com.au>, Marc Haber <mh+qemu-devel@zugschlus.de>
Cc: john.cooper@redhat.com, qemu-devel@nongnu.org

Rusty Russell wrote:
> On Thu, 25 Mar 2010 04:04:02 pm john cooper wrote:
>> Return serial string to the guest application via
>> ioctl driver call.
> 
> This is quite nice.  Minor nits:
> 
>> +	if (cmd == 'VBID') {
>> +		void *usr_data = (void __user *)data;
> 
> void __user *usr_data;
> 
>> +		char *id_str;
>> +		int err;
>> +
>> +		if (!(id_str = kmalloc(VIRTIO_BLK_ID_BYTES, GFP_KERNEL)))
>> +			err = -ENOMEM;
>> +		else if ((err = virtblk_get_id(disk, id_str)))
>> +			;
>> +		else if (copy_to_user(usr_data, id_str, VIRTIO_BLK_ID_BYTES))
>> +			err = -EFAULT;
>> +		if (id_str)
>> +			kfree(id_str);
>> +		return err;
>> +	}
> 
> We can't put the id_str on the stack?  Makes it even simpler :)

At a VIRTIO_BLK_ID_BYTES of the current 20 bytes it seems
safe but there was discussion of extending it so I thought
to locate it in safer storage.

Note the above was intended as more of an example to
illustrate the mechanism.  Marc had proposed a /sys
style interface to retrieve a virtio id string which
is what motivated revisiting this issue.

Marc, if you don't foresee tying off that work relatively
soon where a /sys interface would be made available, I'll
rework the above to be a little more general.  The first
version of the S/N patch pulled a user buffer size from
the caller and limited the copy out to that length.

-john

-- 
john.cooper@redhat.com