[libvirt] [PATCH] storage: avoid null deref on qemu-img failure
Detected by Coverity. Only possible if qemu-img gives bogus output, but we might as well be robust. * src/storage/storage_backend.c (virStorageBackendQEMUImgBackingFormat): Check for strstr failure. --- src/storage/storage_backend.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 64c35c2..7c8bfdc 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -631,8 +631,9 @@ static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg) if (virCommandRun(cmd, &exitstatus) < 0) goto cleanup; - start = strstr(help, " create "); - end = strstr(start, "\n"); + if ((start = strstr(help, " create ")) == NULL || + (end = strstr(start, "\n")) == NULL) + goto cleanup; if (((tmp = strstr(start, "-F fmt")) && tmp < end) || ((tmp = strstr(start, "-F backing_fmt")) && tmp < end)) ret = QEMU_IMG_BACKING_FORMAT_FLAG; -- 1.7.4.4
Dňa 21.10.2011 23:39, Eric Blake wrote / napísal(a):
Detected by Coverity. Only possible if qemu-img gives bogus output, but we might as well be robust.
* src/storage/storage_backend.c (virStorageBackendQEMUImgBackingFormat): Check for strstr failure. --- src/storage/storage_backend.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 64c35c2..7c8bfdc 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -631,8 +631,9 @@ static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg) if (virCommandRun(cmd,&exitstatus)< 0) goto cleanup;
- start = strstr(help, " create "); - end = strstr(start, "\n"); + if ((start = strstr(help, " create ")) == NULL || + (end = strstr(start, "\n")) == NULL) + goto cleanup;
At least one call graph to this function does not contain adding a error message of any kind. I think it would be fair to inform the user if his qemu-img is somehow broken (although unlikely to happen), if we check for this now.
if (((tmp = strstr(start, "-F fmt"))&& tmp< end) || ((tmp = strstr(start, "-F backing_fmt"))&& tmp< end)) ret = QEMU_IMG_BACKING_FORMAT_FLAG;
ACK, Peter.
On 10/21/2011 04:17 PM, Peter Krempa wrote:
Dňa 21.10.2011 23:39, Eric Blake wrote / napísal(a):
Detected by Coverity. Only possible if qemu-img gives bogus output, but we might as well be robust.
* src/storage/storage_backend.c (virStorageBackendQEMUImgBackingFormat): Check for strstr failure. ---
- start = strstr(help, " create "); - end = strstr(start, "\n"); + if ((start = strstr(help, " create ")) == NULL || + (end = strstr(start, "\n")) == NULL) + goto cleanup;
At least one call graph to this function does not contain adding a error message of any kind. I think it would be fair to inform the user if his qemu-img is somehow broken (although unlikely to happen), if we check for this now.
I squashed this in: diff --git i/src/storage/storage_backend.c w/src/storage/storage_backend.c index 7c8bfdc..93c98d6 100644 --- i/src/storage/storage_backend.c +++ w/src/storage/storage_backend.c @@ -632,8 +632,12 @@ static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg) goto cleanup; if ((start = strstr(help, " create ")) == NULL || - (end = strstr(start, "\n")) == NULL) + (end = strstr(start, "\n")) == NULL) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to parse qemu-img output '%s'"), + help); goto cleanup; + } if (((tmp = strstr(start, "-F fmt")) && tmp < end) || ((tmp = strstr(start, "-F backing_fmt")) && tmp < end)) ret = QEMU_IMG_BACKING_FORMAT_FLAG;
ACK,
and pushed. Thanks for the review. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Peter Krempa