[libvirt] [PATCH] test_driver: sanitize user-provided array in testDomainGetDiskErrors
Zero out the user provided memory in order to avoid passing random pointers to VIR_FREE later. Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 7c58d2c8ce..8ee1f0cad6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3275,6 +3275,7 @@ static int testDomainGetDiskErrors(virDomainPtr dom, virDomainObjPtr vm = NULL; int ret = -1; size_t i; + size_t nerrors; virCheckFlags(0, -1); @@ -3284,8 +3285,13 @@ static int testDomainGetDiskErrors(virDomainPtr dom, if (virDomainObjCheckActive(vm) < 0) goto cleanup; + nerrors = MIN(vm->def->ndisks, maxerrors); + if (errors) { - for (i = 0; i < MIN(vm->def->ndisks, maxerrors); i++) { + /* sanitize input */ + memset(errors, 0, sizeof(virDomainDiskError) * nerrors); + + for (i = 0; i < nerrors; i++) { if (VIR_STRDUP(errors[i].disk, vm->def->disks[i]->dst) < 0) goto cleanup; errors[i].error = (i % (VIR_DOMAIN_DISK_ERROR_LAST - 1)) + 1; @@ -3297,7 +3303,7 @@ static int testDomainGetDiskErrors(virDomainPtr dom, cleanup: if (ret < 0) { - for (i = 0; i < MIN(vm->def->ndisks, maxerrors); i++) + for (i = 0; i < nerrors; i++) VIR_FREE(errors[i].disk); } virDomainObjEndAPI(&vm); -- 2.22.0
On Thu, Jul 04, 2019 at 01:31:34PM +0200, Ilias Stamatis wrote:
Zero out the user provided memory in order to avoid passing random
"to a avoid potentially freeing uninitialized memory"
pointers to VIR_FREE later.
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
On Thu, Jul 04, 2019 at 01:31:34PM +0200, Ilias Stamatis wrote:
Zero out the user provided memory in order to avoid passing random pointers to VIR_FREE later.
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 7c58d2c8ce..8ee1f0cad6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3275,6 +3275,7 @@ static int testDomainGetDiskErrors(virDomainPtr dom, virDomainObjPtr vm = NULL; int ret = -1; size_t i; + size_t nerrors;
Not initialized...
virCheckFlags(0, -1);
@@ -3284,8 +3285,13 @@ static int testDomainGetDiskErrors(virDomainPtr dom, if (virDomainObjCheckActive(vm) < 0) goto cleanup;
...error path jumps over the initialization...
+ nerrors = MIN(vm->def->ndisks, maxerrors); + if (errors) { - for (i = 0; i < MIN(vm->def->ndisks, maxerrors); i++) { + /* sanitize input */ + memset(errors, 0, sizeof(virDomainDiskError) * nerrors); + + for (i = 0; i < nerrors; i++) { if (VIR_STRDUP(errors[i].disk, vm->def->disks[i]->dst) < 0) goto cleanup; errors[i].error = (i % (VIR_DOMAIN_DISK_ERROR_LAST - 1)) + 1; @@ -3297,7 +3303,7 @@ static int testDomainGetDiskErrors(virDomainPtr dom,
cleanup: if (ret < 0) { - for (i = 0; i < MIN(vm->def->ndisks, maxerrors); i++) + for (i = 0; i < nerrors; i++)
...loops with limit set to an uninitialized value :-) ../../src/test/test_driver.c: In function 'testDomainGetDiskErrors': ../../src/test/test_driver.c:3306:9: error: 'nerrors' may be used uninitialized in this function [-Werror=maybe-uninitialized] for (i = 0; i < nerrors; i++) ^~~ cc1: all warnings being treated as errors
VIR_FREE(errors[i].disk); } virDomainObjEndAPI(&vm);
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Daniel P. Berrangé -
Erik Skultety -
Ilias Stamatis