qemu-devel
[Top][All Lists]
Advanced

[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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]