[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.5?] qcow2: always initialize
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.5?] qcow2: always initialize specific image info |
Date: |
Mon, 7 Dec 2015 10:54:03 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 12/07/2015 10:51 AM, Eric Blake wrote:
> [adding qemu-devel - ALL patches should go to qemu-devel, even if they
> are also going to a sub-list like qemu-block]
>
> On 12/07/2015 10:07 AM, Roman Kagan wrote:
>> qcow2_get_specific_info() used to have a code path which would leave
>> pointer to ImageInfoSpecificQCow2 uninitialized.
>>
>> We guess that it caused sporadic crashes on freeing an invalid pointer
>> in response to "query-block" QMP command in
>> visit_type_ImageInfoSpecificQCow2 with QapiDeallocVisitor.
>>
>> Although we have neither a solid proof nor a reproduction scenario,
>> making sure the field is initialized appears a reasonable thing to do.
>>
>> Signed-off-by: Roman Kagan <address@hidden>
>> ---
>> block/qcow2.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Oops; hit send too soon. I added for-2.5? to the subject line, because...
>
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 88f56c8..67c9d3d 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -2739,7 +2739,7 @@ static ImageInfoSpecific
>> *qcow2_get_specific_info(BlockDriverState *bs)
>>
>> *spec_info = (ImageInfoSpecific){
>> .type = IMAGE_INFO_SPECIFIC_KIND_QCOW2,
>> - .u.qcow2 = g_new(ImageInfoSpecificQCow2, 1),
>> + .u.qcow2 = g_new0(ImageInfoSpecificQCow2, 1),
>
> NACK. This makes no difference, except when s->qcow_version is out of spec.
>
>> };
>> if (s->qcow_version == 2) {
>> *spec_info->u.qcow2 = (ImageInfoSpecificQCow2){
>>
>
> If s->qcow_version is exactly 2, then we end up initializing all fields
> due to the assignment here; same if qcow_version is exactly 3. The only
> time qcow2 remains uninitialized is if qcow_version is 0, 1, or > 3; but
> we refuse to handle qcow files with out-of-range versions. So I don't
> see how you are plugging any uninitialized values; and therefore, I
> don't see how this is patching any crashes.
...if you can prove that we aren't gracefully handling an out-of-spec
qcow_version, such that the uninitialized memory in that scenario is
indeed causing a crash, then it is worth respinning a v2 of this patch
with that proof, and worth considering it for 2.5 if it really is a
crash fixer.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature