[Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2

[Michael S. Tsirkin]

On Wed, Oct 07, 2009 at 08:48:32AM -0500, Anthony Liguori wrote:
> john cooper wrote:
>> Michael S. Tsirkin wrote:
>>   
>>>> +    put_le16(p + 0, 0x0);                            /* ATA device */
>>>> +    padstr((char *)(p + 23), QEMU_VERSION, 8);       /* firmware revision 
>>>> */
>>>>       
>>> QEMU version is currently a string like "0.11.50" which is exactly 8
>>> bytes. What if someone makes it longer?  padstr will not 0
>>> terminate string, and only partial data will be there.
>>>     
>>
>> This code treats the field similar to the logic from which
>> it derives (hw/ide.c) in that the field need not be nul
>> terminated.  Quiet truncation to 8 bytes can occur here
>> and in the existing usage but in a practical sense I don't
>> see much of a recourse.  We can flag a warning but the
>> data is realistically a best-effort attempt to provide
>> relevant information in this field.  IOW overflowing
>> this field probably isn't justification alone to modify
>> a too long qemu version string.
>>   
>
> Hrm, we really shouldn't be exposing a version string to the guest in  
> the first place.
>
> That's a compatibility issue.

Actually, it's a good point. Otherwise e.g. the identity changes with
migration. My understanding is that this isn't the only place where
we do this?

> Really, I strongly dislike passing this identity page via virtio.  Why  
> are we still going this route instead of just passing the S/N?

No opinion on this.

> Regards,
>
> Anthony Liguori