[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2
From: |
john cooper |
Subject: |
[Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2 |
Date: |
Wed, 07 Oct 2009 01:49:26 -0400 |
User-agent: |
Thunderbird 2.0.0.9 (X11/20071115) |
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.
> Also, identify is pre-initialized to 0, isn't it?
> So just strcpy should be enough, here and elsewhere,
> no need to roll our own padstr.
Actually this is an oversight in the local padstr() which
should be padding the balance of the field with ' ' vs. '\0'.
>> + memcpy(req->elem.in_sg[0].iov_base, s->identify,
>> + req->elem.in_sg[0].iov_len);
>
> Is this safe? Can guest make iov_len bigger than size of s->identity?
Good point, a malicious/buggy guest can. The memcpy
length should be capped.
>> + virtio_identify_template(s);
>> + strncpy((char *)&s->identify[VIRTIO_BLK_ID_SN],
>> + (char *)drive_get_serial(bs), VIRTIO_BLK_ID_SN_BYTES);
>
> This can silently truncate the serial, can't it?
Yes, it is the same disposition as ide/scsi's treatment
of the S/N. My concern was of keeping the behavior
consistent.
Thanks,
-john
--
address@hidden
- [Qemu-devel] [PATCH] fix virtio_blk serial pci config breakage, v2, john cooper, 2009/10/05
- [Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2, Michael S. Tsirkin, 2009/10/05
- [Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2,
john cooper <=
- [Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2, Anthony Liguori, 2009/10/07
- [Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2, Michael S. Tsirkin, 2009/10/07
- [Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2, Anthony Liguori, 2009/10/07
- [Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2, john cooper, 2009/10/07
[Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2, Michael S. Tsirkin, 2009/10/05
[Qemu-devel] Re: [PATCH] fix virtio_blk serial pci config breakage, v2, Anthony Liguori, 2009/10/06