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.coo...@redhat.com
