[libvirt] [PATCH 0/2] qemu: block: Fix issues pointed out by coverity
Few unlikely memleaks and a more likely NULL deref. Peter Krempa (2): qemu: block: Break out early on invalid storage sources qemu: block: Don't leak server JSON object from protocol generators src/qemu/qemu_block.c | 76 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 28 deletions(-) -- 2.14.3
Return NULL right away in qemuBlockStorageSourceGetBackendProps when an invalid storage source is presented so that virJSONValueObjectAdd isn't called with a NULL argument. Found by coverity. --- src/qemu/qemu_block.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index e46a455af..600f315fe 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -958,7 +958,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: - break; + return NULL; case VIR_STORAGE_TYPE_NETWORK: switch ((virStorageNetProtocol) src->protocol) { @@ -1008,7 +1008,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_LAST: - break; + return NULL; } break; } -- 2.14.3
If creation of the main JSON object containing the storage portion of a virStorageSource would fail but we'd allocate the server structure we'd leak it. Found by coverity. --- src/qemu/qemu_block.c | 72 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 600f315fe..8b23df822 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -833,13 +833,18 @@ qemuBlockStorageSourceGetNBDProps(virStorageSourcePtr src) if (!serverprops) return NULL; - ignore_value(virJSONValueObjectCreate(&ret, - "s:driver", "nbd", - "a:server", serverprops, - "S:export", src->path, - "S:tls-creds", src->tlsAlias, - NULL)); + if (virJSONValueObjectCreate(&ret, + "s:driver", "nbd", + "a:server", serverprops, + "S:export", src->path, + "S:tls-creds", src->tlsAlias, + NULL) < 0) + goto cleanup; + + serverprops = NULL; + cleanup: + virJSONValueFree(serverprops); return ret; } @@ -859,16 +864,21 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src) if (src->auth) username = srcPriv->secinfo->s.aes.username; - ignore_value(virJSONValueObjectCreate(&ret, - "s:driver", "rbd", - "s:pool", src->volume, - "s:image", src->path, - "S:snapshot", src->snapshot, - "S:conf", src->configFile, - "A:server", servers, - "S:user", username, - NULL)); + if (virJSONValueObjectCreate(&ret, + "s:driver", "rbd", + "s:pool", src->volume, + "s:image", src->path, + "S:snapshot", src->snapshot, + "S:conf", src->configFile, + "A:server", servers, + "S:user", username, + NULL) < 0) + goto cleanup; + servers = NULL; + + cleanup: + virJSONValueFree(servers); return ret; } @@ -891,12 +901,17 @@ qemuBlockStorageSourceGetSheepdogProps(virStorageSourcePtr src) return NULL; /* libvirt does not support the 'snap-id' and 'tag' properties */ - ignore_value(virJSONValueObjectCreate(&ret, - "s:driver", "sheepdog", - "a:server", serverprops, - "s:vdi", src->path, - NULL)); + if (virJSONValueObjectCreate(&ret, + "s:driver", "sheepdog", + "a:server", serverprops, + "s:vdi", src->path, + NULL) < 0) + goto cleanup; + serverprops = NULL; + + cleanup: + virJSONValueFree(serverprops); return ret; } @@ -921,13 +936,18 @@ qemuBlockStorageSourceGetSshProps(virStorageSourcePtr src) if (src->auth) username = src->auth->username; - ignore_value(virJSONValueObjectCreate(&ret, - "s:driver", "ssh", - "s:path", src->path, - "a:server", serverprops, - "S:user", username, - NULL)); + if (virJSONValueObjectCreate(&ret, + "s:driver", "ssh", + "s:path", src->path, + "a:server", serverprops, + "S:user", username, + NULL) < 0) + goto cleanup; + + serverprops = NULL; + cleanup: + virJSONValueFree(serverprops); return ret; } -- 2.14.3
On 11/09/2017 04:43 AM, Peter Krempa wrote:
Few unlikely memleaks and a more likely NULL deref.
Peter Krempa (2): qemu: block: Break out early on invalid storage sources qemu: block: Don't leak server JSON object from protocol generators
src/qemu/qemu_block.c | 76 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 28 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> (series) John
participants (2)
-
John Ferlan -
Peter Krempa