[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 01/13] qemu: fix use-after-free when parsing NBD disk



On 02/25/2013 10:44 AM, Paolo Bonzini wrote:
> disk->src is still used for disks->hosts->name, do not free it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini redhat com>
> ---
>  src/qemu/qemu_command.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index dee493f..5dccaae 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8707,7 +8707,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,

A bit more context helps:

                char *host, *port;

                    host = disk->src;

                    disk->hosts->name = host;

>                      disk->hosts->port = strdup(port);
>                      if (!disk->hosts->port)
>                          goto no_memory;
> -                    VIR_FREE(disk->src);
>                      disk->src = NULL;
>                      break;

So there is definitely a use-after-free bug fixed by your patch.
However, your patch causes a double-free bug on error (if the
strdup(port) fails, then disk->hosts->port and disk->src are the same
pointer, but we attempt to free both of them).  I'm squashing this in
before pushing, to make the transfer semantics more obvious:

diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
index 0a7d4ec..f8f3ade 100644
--- i/src/qemu/qemu_command.c
+++ w/src/qemu/qemu_command.c
@@ -8832,11 +8832,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr
qemuCaps,
                     if (VIR_ALLOC(disk->hosts) < 0)
                         goto no_memory;
                     disk->nhosts = 1;
-                    disk->hosts->name = host;
+                    disk->hosts->name = disk->src;
+                    disk->src = NULL;
                     disk->hosts->port = strdup(port);
                     if (!disk->hosts->port)
                         goto no_memory;
-                    disk->src = NULL;
                     break;
                 case VIR_DOMAIN_DISK_PROTOCOL_RBD:
                     /* old-style CEPH_ARGS env variable is parsed later */


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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]