[Qemu-devel] [PATCH] sockets: Plug memory leak in socket_address_flatten()

[Qemu-devel] [PATCH] sockets: Plug memory leak in socket_address_flatten()

[Markus Armbruster]

socket_address_flatten() leaks a SocketAddress when its argument is
null.  Happens when opening a ChardevBackend of type 'udp' that is
configured without a local address.  Screwed up in commit bd269ebc due
to last minute semantic conflict resolution.  Spotted by Coverity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 util/qemu-sockets.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index d8183f7..b39ae74 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1338,12 +1338,14 @@ char *socket_address_to_string(struct SocketAddress *addr, Error **errp)
 
 SocketAddress *socket_address_flatten(SocketAddressLegacy *addr_legacy)
 {
-    SocketAddress *addr = g_new(SocketAddress, 1);
+    SocketAddress *addr;
 
     if (!addr_legacy) {
         return NULL;
     }
 
+    addr = g_new(SocketAddress, 1);
+
     switch (addr_legacy->type) {
     case SOCKET_ADDRESS_LEGACY_KIND_INET:
         addr->type = SOCKET_ADDRESS_TYPE_INET;
-- 
2.7.4

Re: [Qemu-devel] [PATCH] sockets: Plug memory leak in socket_address_flatten()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

On 05/15/2017 11:39 AM, Markus Armbruster wrote:
> socket_address_flatten() leaks a SocketAddress when its argument is
> null.  Happens when opening a ChardevBackend of type 'udp' that is
> configured without a local address.  Screwed up in commit bd269ebc due
> to last minute semantic conflict resolution.  Spotted by Coverity.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  util/qemu-sockets.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

Matches the fix I had proposed here against the v2 pull request:
https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg01825.html

I guess in the confusion of the rebasing, you didn't quite implement it
in v3 the way I had proposed.

> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index d8183f7..b39ae74 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1338,12 +1338,14 @@ char *socket_address_to_string(struct SocketAddress *addr, Error **errp)
>  
>  SocketAddress *socket_address_flatten(SocketAddressLegacy *addr_legacy)
>  {
> -    SocketAddress *addr = g_new(SocketAddress, 1);
> +    SocketAddress *addr;
>  
>      if (!addr_legacy) {
>          return NULL;
>      }
>  
> +    addr = g_new(SocketAddress, 1);
> +
>      switch (addr_legacy->type) {
>      case SOCKET_ADDRESS_LEGACY_KIND_INET:
>          addr->type = SOCKET_ADDRESS_TYPE_INET;
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] sockets: Plug memory leak in socket_address_flatten()

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 05/15/2017 11:39 AM, Markus Armbruster wrote:
>> socket_address_flatten() leaks a SocketAddress when its argument is
>> null.  Happens when opening a ChardevBackend of type 'udp' that is
>> configured without a local address.  Screwed up in commit bd269ebc due
>> to last minute semantic conflict resolution.  Spotted by Coverity.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  util/qemu-sockets.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> Matches the fix I had proposed here against the v2 pull request:
> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg01825.html
>
> I guess in the confusion of the rebasing, you didn't quite implement it
> in v3 the way I had proposed.

I had started to resolve conflicts myself when the message carrying your
solution arrived, and I obviously screwed up merging it into my unclean
tree.  In other words: it's all my fault.