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 <rka...@virtuozzo.com> >> --- >> 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
