[libvirt] [PATCH 1/2] test_driver: Don't access @vm after it was set to NULL

Michal Privoznik posted 2 patches 6 years, 9 months ago
[libvirt] [PATCH 1/2] test_driver: Don't access @vm after it was set to NULL
Posted by Michal Privoznik 6 years, 9 months ago
If something goes wrong in testDomainGetDiskErrors() then we try
to free any strings that were previously allocated in return
array. Problem is, in my review of original patch (89320788ac4)
I've mistakenly did some changes which result in possible NULL
dereference (@vm is set to NULL as the first thing under cleanup
label).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/test/test_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a4c17ef0df..cfa739ecca 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3058,7 +3058,7 @@ static int testDomainGetDiskErrors(virDomainPtr dom,
     virCheckFlags(0, -1);
 
     if (!(vm = testDomObjFromDomain(dom)))
-        goto cleanup;
+        return -1;
 
     if (virDomainObjCheckActive(vm) < 0)
         goto cleanup;
@@ -3075,11 +3075,11 @@ static int testDomainGetDiskErrors(virDomainPtr dom,
     }
 
  cleanup:
-    virDomainObjEndAPI(&vm);
     if (ret < 0) {
         for (i = 0; i < MIN(vm->def->ndisks, maxerrors); i++)
             VIR_FREE(errors[i].disk);
     }
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] test_driver: Don't access @vm after it was set to NULL
Posted by Andrea Bolognani 6 years, 7 months ago
On Tue, 2019-05-14 at 17:12 +0200, Michal Privoznik wrote:
> If something goes wrong in testDomainGetDiskErrors() then we try
> to free any strings that were previously allocated in return
> array. Problem is, in my review of original patch (89320788ac4)
> I've mistakenly did some changes which result in possible NULL
> dereference (@vm is set to NULL as the first thing under cleanup
> label).
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/test/test_driver.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list