[libvirt] [PATCH v2 0/2] qemu: blockPeek: Fix invalid buffer usage
v2 fixes bug introduced by attempting to split the change into two patches. Peter Krempa (2): qemu: blockPeek: Fix filling of the return buffer qemu: blockPeek: Enforce buffer filling src/qemu/qemu_driver.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) -- 2.14.1
Commit 3956af495e broke the blockPeek API since virStorageFileRead allocates a return buffer and fills it with the data, while the API fills a user-provided buffer. This did not get caught by the compiler since the API prototype uses a 'void *'. Fix it by transferring the data from the allocated buffer to the user provided buffer. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1491217 --- v2: Actually fill 'tmpbuf' in call to virStorageFileRead src/qemu/qemu_driver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e1a0dd553..3109f8a17 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11415,6 +11415,7 @@ qemuDomainBlockPeek(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainDiskDefPtr disk = NULL; virDomainObjPtr vm; + char *tmpbuf = NULL; int ret = -1; virCheckFlags(0, -1); @@ -11441,15 +11442,18 @@ qemuDomainBlockPeek(virDomainPtr dom, if (qemuDomainStorageFileInit(driver, vm, disk->src) < 0) goto cleanup; - if (virStorageFileRead(disk->src, offset, size, buffer) < 0) + if (virStorageFileRead(disk->src, offset, size, &tmpbuf) < 0) goto cleanup; + memcpy(buffer, tmpbuf, size); + ret = 0; cleanup: if (disk) virStorageFileDeinit(disk->src); virDomainObjEndAPI(&vm); + VIR_FREE(tmpbuf); return ret; } -- 2.14.1
Documentation states: "'offset' and 'size' represent an area which must lie entirely within the device or file." Enforce the that the buffer lies within fully. --- src/qemu/qemu_driver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3109f8a17..bddba6b71 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11416,6 +11416,7 @@ qemuDomainBlockPeek(virDomainPtr dom, virDomainDiskDefPtr disk = NULL; virDomainObjPtr vm; char *tmpbuf = NULL; + ssize_t nread; int ret = -1; virCheckFlags(0, -1); @@ -11442,9 +11443,16 @@ qemuDomainBlockPeek(virDomainPtr dom, if (qemuDomainStorageFileInit(driver, vm, disk->src) < 0) goto cleanup; - if (virStorageFileRead(disk->src, offset, size, &tmpbuf) < 0) + if ((nread = virStorageFileRead(disk->src, offset, size, &tmpbuf)) < 0) goto cleanup; + if (nread < size) { + virReportError(VIR_ERR_INVALID_ARG, + _("'%s' starting from %llu has only %zd bytes available"), + path, offset, nread); + goto cleanup; + } + memcpy(buffer, tmpbuf, size); ret = 0; -- 2.14.1
On 09/19/2017 03:06 AM, Peter Krempa wrote:
v2 fixes bug introduced by attempting to split the change into two patches.
Peter Krempa (2): qemu: blockPeek: Fix filling of the return buffer qemu: blockPeek: Enforce buffer filling
src/qemu/qemu_driver.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
ACK series -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
participants (2)
-
Eric Blake -
Peter Krempa