Re: [Qemu-devel] [Qemu-block] [PATCH for-2.5] qcow2: always initialize specific image info

[Eric Blake]

[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(-)


> 
> 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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature