[libvirt] [PATCH] Fix invalid read in virCgroupGetValueStr

Don't check for '\n' at the end of file if zero bytes were read. Found by valgrind: ==404== Invalid read of size 1 ==404== at 0x529B09F: virCgroupGetValueStr (vircgroup.c:540) ==404== by 0x529AF64: virCgroupMoveTask (vircgroup.c:1079) ==404== by 0x1EB475: qemuSetupCgroupForEmulator (qemu_cgroup.c:1061) ==404== by 0x1D9489: qemuProcessStart (qemu_process.c:3801) ==404== by 0x18557E: qemuDomainObjStart (qemu_driver.c:5787) ==404== by 0x190FA4: qemuDomainCreateWithFlags (qemu_driver.c:5839) Introduced by 0d0b409. https://bugzilla.redhat.com/show_bug.cgi?id=978356 --- src/util/vircgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 0484c71..a0ee3f7 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -537,7 +537,7 @@ static int virCgroupGetValueStr(virCgroupPtr group, VIR_DEBUG("Failed to read %s: %m\n", keypath); } else { /* Terminated with '\n' has sometimes harmful effects to the caller */ - if ((*value)[rc - 1] == '\n') + if (rc > 0 && (*value)[rc - 1] == '\n') (*value)[rc - 1] = '\0'; rc = 0; -- 1.8.1.5

On 06/26/13 15:06, Ján Tomko wrote:
Don't check for '\n' at the end of file if zero bytes were read.
Found by valgrind: ==404== Invalid read of size 1 ==404== at 0x529B09F: virCgroupGetValueStr (vircgroup.c:540) ==404== by 0x529AF64: virCgroupMoveTask (vircgroup.c:1079) ==404== by 0x1EB475: qemuSetupCgroupForEmulator (qemu_cgroup.c:1061) ==404== by 0x1D9489: qemuProcessStart (qemu_process.c:3801) ==404== by 0x18557E: qemuDomainObjStart (qemu_driver.c:5787) ==404== by 0x190FA4: qemuDomainCreateWithFlags (qemu_driver.c:5839)
Introduced by 0d0b409.
https://bugzilla.redhat.com/show_bug.cgi?id=978356 --- src/util/vircgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK, the code before the optimization was immune to this problem. Peter

On 06/26/2013 03:20 PM, Peter Krempa wrote:
On 06/26/13 15:06, Ján Tomko wrote:
Don't check for '\n' at the end of file if zero bytes were read.
Found by valgrind: ==404== Invalid read of size 1 ==404== at 0x529B09F: virCgroupGetValueStr (vircgroup.c:540) ==404== by 0x529AF64: virCgroupMoveTask (vircgroup.c:1079) ==404== by 0x1EB475: qemuSetupCgroupForEmulator (qemu_cgroup.c:1061) ==404== by 0x1D9489: qemuProcessStart (qemu_process.c:3801) ==404== by 0x18557E: qemuDomainObjStart (qemu_driver.c:5787) ==404== by 0x190FA4: qemuDomainCreateWithFlags (qemu_driver.c:5839)
Introduced by 0d0b409.
https://bugzilla.redhat.com/show_bug.cgi?id=978356 --- src/util/vircgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK, the code before the optimization was immune to this problem.
Peter
Actually, the commit message explicitly says 'Allow for multi-line files', so it wasn't just an optimization. Thanks, pushed now. Jan
participants (2)
-
Ján Tomko
-
Peter Krempa