[Qemu-devel] [PATCH] hw/arm/armsse: Fix memory leak in error-exit path

Peter Maydell posted 1 patch 5 years, 1 month ago
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu failed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190215113707.24553-1-peter.maydell@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
hw/arm/armsse.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
[Qemu-devel] [PATCH] hw/arm/armsse: Fix memory leak in error-exit path
Posted by Peter Maydell 5 years, 1 month ago
Coverity points out (CID 1398632, CID 1398650) that we
leak a couple of allocated strings in the error-exit
code path for setting up the MHUs in the ARMSSE.
Fix this bug by moving the allocate-and-free of each
string to be closer to the use, so we do the free before
doing the error-exit check.

Fixes: f8574705f62b38a ("hw/arm/armsse: Add unimplemented-device stubs for MHUs")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/armsse.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 9a8c49547db..d0207dbabc7 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -762,26 +762,28 @@ static void armsse_realize(DeviceState *dev, Error **errp)
 
     if (info->has_mhus) {
         for (i = 0; i < ARRAY_SIZE(s->mhu); i++) {
-            char *name = g_strdup_printf("MHU%d", i);
-            char *port = g_strdup_printf("port[%d]", i + 3);
+            char *name;
+            char *port;
 
+            name = g_strdup_printf("MHU%d", i);
             qdev_prop_set_string(DEVICE(&s->mhu[i]), "name", name);
             qdev_prop_set_uint64(DEVICE(&s->mhu[i]), "size", 0x1000);
             object_property_set_bool(OBJECT(&s->mhu[i]), true,
                                      "realized", &err);
+            g_free(name);
             if (err) {
                 error_propagate(errp, err);
                 return;
             }
+            port = g_strdup_printf("port[%d]", i + 3);
             mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mhu[i]), 0);
             object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr),
                                      port, &err);
+            g_free(port);
             if (err) {
                 error_propagate(errp, err);
                 return;
             }
-            g_free(name);
-            g_free(port);
         }
     }
 
-- 
2.20.1


Re: [Qemu-devel] [PATCH] hw/arm/armsse: Fix memory leak in error-exit path
Posted by Philippe Mathieu-Daudé 5 years, 1 month ago
On 2/15/19 12:37 PM, Peter Maydell wrote:
> Coverity points out (CID 1398632, CID 1398650) that we
> leak a couple of allocated strings in the error-exit
> code path for setting up the MHUs in the ARMSSE.
> Fix this bug by moving the allocate-and-free of each
> string to be closer to the use, so we do the free before
> doing the error-exit check.
> 
> Fixes: f8574705f62b38a ("hw/arm/armsse: Add unimplemented-device stubs for MHUs")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  hw/arm/armsse.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
> index 9a8c49547db..d0207dbabc7 100644
> --- a/hw/arm/armsse.c
> +++ b/hw/arm/armsse.c
> @@ -762,26 +762,28 @@ static void armsse_realize(DeviceState *dev, Error **errp)
>  
>      if (info->has_mhus) {
>          for (i = 0; i < ARRAY_SIZE(s->mhu); i++) {
> -            char *name = g_strdup_printf("MHU%d", i);
> -            char *port = g_strdup_printf("port[%d]", i + 3);
> +            char *name;
> +            char *port;
>  
> +            name = g_strdup_printf("MHU%d", i);
>              qdev_prop_set_string(DEVICE(&s->mhu[i]), "name", name);
>              qdev_prop_set_uint64(DEVICE(&s->mhu[i]), "size", 0x1000);
>              object_property_set_bool(OBJECT(&s->mhu[i]), true,
>                                       "realized", &err);
> +            g_free(name);
>              if (err) {
>                  error_propagate(errp, err);
>                  return;
>              }
> +            port = g_strdup_printf("port[%d]", i + 3);
>              mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mhu[i]), 0);
>              object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr),
>                                       port, &err);
> +            g_free(port);
>              if (err) {
>                  error_propagate(errp, err);
>                  return;
>              }
> -            g_free(name);
> -            g_free(port);
>          }
>      }
>  
>