[libvirt] [PATCH 0/2] Couple of recently found Coverity issues
Nuisance for some, but both have negative repurcussions although one is "just" a test. John Ferlan (2): tests: Fix virmacmaptest when allocation fails nss: Need to check error condition on virJSONValueArraySize tests/virmacmaptest.c | 9 +++++++-- tools/nss/libvirt_nss.c | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) -- 2.7.4
If the allocation fails in DO_TEST_FLUSH_PROLOGUE, then 'mgr == NULL', but the code continues on - which won't be good. So modify the macro to cause an immediate failure and jump to a cleanup label. Found by Coverity as FORWARD_NULL event. Signed-off-by: John Ferlan <jferlan@redhat.com> --- tests/virmacmaptest.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/virmacmaptest.c b/tests/virmacmaptest.c index 34609ad..4daa048 100644 --- a/tests/virmacmaptest.c +++ b/tests/virmacmaptest.c @@ -170,8 +170,12 @@ mymain(void) } while (0) #define DO_TEST_FLUSH_PROLOGUE \ - if (!(mgr = virMacMapNew(NULL))) \ - ret = -1; + do { \ + if (!(mgr = virMacMapNew(NULL))) { \ + ret = -1; \ + goto cleanup; \ + } \ + } while (0) #define DO_TEST_FLUSH(d, ...) \ do { \ @@ -226,6 +230,7 @@ mymain(void) DO_TEST_FLUSH("dom1", "9e:89:49:99:51:0e", "89:b4:3f:08:88:2c", "54:0b:4c:e2:0a:39"); DO_TEST_FLUSH("dom1", "bb:88:07:19:51:9d", "b7:f1:1a:40:a2:95", "88:94:39:a3:90:b4"); DO_TEST_FLUSH_EPILOGUE("complex"); + cleanup: return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.7.4
If the 'nleases < 0' on return, then the subsequent call to findLeaseInJSON will not produce the expected results (passed in as a size_t, but nleases is a ssize_t). So check if the returned value < 0 and if so, goto cleanup. Found by Coverity as a NEGATIVE_RETURNS event Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/nss/libvirt_nss.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index 418c11f..b69e62c 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -309,7 +309,8 @@ findLease(const char *name, } VIR_DIR_CLOSE(dir); - nleases = virJSONValueArraySize(leases_array); + if ((nleases = virJSONValueArraySize(leases_array)) < 0) + goto cleanup; DEBUG("Read %zd leases", nleases); #if !defined(LIBVIRT_NSS_GUEST) -- 2.7.4
On 07.12.2016 14:20, John Ferlan wrote:
If the 'nleases < 0' on return, then the subsequent call to findLeaseInJSON will not produce the expected results (passed in as a size_t, but nleases is a ssize_t). So check if the returned value < 0 and if so, goto cleanup.
Found by Coverity as a NEGATIVE_RETURNS event
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/nss/libvirt_nss.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index 418c11f..b69e62c 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -309,7 +309,8 @@ findLease(const char *name, } VIR_DIR_CLOSE(dir);
- nleases = virJSONValueArraySize(leases_array); + if ((nleases = virJSONValueArraySize(leases_array)) < 0) + goto cleanup; DEBUG("Read %zd leases", nleases);
Well, this one could happen iff @leases_array wouldn't be a JSON array. Thus I'm okay with skipping DEBUG() in that case. Michal
On 07.12.2016 14:20, John Ferlan wrote:
Nuisance for some, but both have negative repurcussions although one is "just" a test.
John Ferlan (2): tests: Fix virmacmaptest when allocation fails nss: Need to check error condition on virJSONValueArraySize
tests/virmacmaptest.c | 9 +++++++-- tools/nss/libvirt_nss.c | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-)
ACK Michal
participants (2)
-
John Ferlan -
Michal Privoznik