Bug 978356 - valgrind shows an off-by-one error in virCgroupGetValueStr
Summary: valgrind shows an off-by-one error in virCgroupGetValueStr
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: libvirt
Version: 6.4
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: rc
: ---
Assignee: Ján Tomko
QA Contact: Virtualization Bugs
URL:
Whiteboard:
Keywords: ZStream
Depends On:
Blocks: 984553 984561
TreeView+ depends on / blocked
 
Reported: 2013-06-26 12:48 UTC by Ján Tomko
Modified: 2013-11-21 09:04 UTC (History)
11 users (show)

(edit)
Previously, the libvirtd daemon was accessing one byte before the array in the virCgroupGetValueStr() function. This bug has been fixed and libvirtd now stays within array bounds.
Clone Of:
(edit)
Last Closed: 2013-11-21 09:04:24 UTC


Attachments (Terms of Use)
valgrind msg (19.03 KB, text/plain)
2013-07-10 02:39 UTC, zhpeng
no flags Details
downgrade valgrind and test it again. (21.86 KB, text/plain)
2013-07-15 03:17 UTC, zhpeng
no flags Details


External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2013:1581 normal SHIPPED_LIVE libvirt bug fix and enhancement update 2013-11-21 01:11:35 UTC

Description Ján Tomko 2013-06-26 12:48:46 UTC
Description of problem:
libvirt is accessing one byte before the array in virCgroupGetValueStr

Version-Release number of selected component (if applicable):
libvirt-0.10.2-18.el6_4.9.x86_64

How reproducible:
100 %

Steps to Reproduce:
1. run libvirtd under valgrind:
valgrind --leak-check=full libvirtd
2. create a domain:
virsh create /dev/stdin <<EOF
<domain type='qemu'>
  <name>duck</name>
  <memory unit='MiB'>32</memory>
  <os>
    <type arch='x86_64' machine='pc'>hvm</type>
  </os>
</domain>
EOF
Domain duck created from /dev/stdin

Actual results:
Valgrind shows an invalid read:

==4945== Invalid read of size 1
==4945==    at 0x4E6D152: virCgroupGetValueStr (cgroup.c:369)
==4945==    by 0x4E6DD7D: virCgroupMoveTask (cgroup.c:897)
==4945==    by 0x471328: qemuSetupCgroupForEmulator (qemu_cgroup.c:695)
==4945==    by 0x48648E: qemuProcessStart (qemu_process.c:3965)
==4945==    by 0x461678: qemudDomainCreate (qemu_driver.c:1595)
==4945==    by 0x4F0A580: virDomainCreateXML (libvirt.c:1954)
==4945==    by 0x43FBD0: remoteDispatchDomainCreateXMLHelper (remote_dispatch.h:1172)
==4945==    by 0x4F5E0E1: virNetServerProgramDispatch (virnetserverprogram.c:431)
==4945==    by 0x4F5F3CD: virNetServerProcessMsg (virnetserver.c:170)
==4945==    by 0x4F5FA6B: virNetServerHandleJob (virnetserver.c:191)
==4945==    by 0x4E8337B: virThreadPoolWorker (threadpool.c:144)
==4945==    by 0x4E82C68: virThreadHelper (threads-pthread.c:161)
==4945==  Address 0x140ccdaf is 1 bytes before a block of size 8,193 alloc'd
==4945==    at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==4945==    by 0x4A06B62: realloc (vg_replace_malloc.c:662)
==4945==    by 0x4E787CB: virReallocN (memory.c:160)
==4945==    by 0x4E87A89: virFileReadLimFD (util.c:400)
==4945==    by 0x4E87BF7: virFileReadAll (util.c:461)
==4945==    by 0x4E6D140: virCgroupGetValueStr (cgroup.c:363)
==4945==    by 0x4E6DD7D: virCgroupMoveTask (cgroup.c:897)
==4945==    by 0x471328: qemuSetupCgroupForEmulator (qemu_cgroup.c:695)
==4945==    by 0x48648E: qemuProcessStart (qemu_process.c:3965)
==4945==    by 0x461678: qemudDomainCreate (qemu_driver.c:1595)
==4945==    by 0x4F0A580: virDomainCreateXML (libvirt.c:1954)
==4945==    by 0x43FBD0: remoteDispatchDomainCreateXMLHelper (remote_dispatch.h:1172)

Expected results:
libvirtd stays within array bounds.

Comment 2 Ján Tomko 2013-06-26 13:09:54 UTC
Upstream patch proposed:
https://www.redhat.com/archives/libvir-list/2013-June/msg01115.html

Comment 3 Ján Tomko 2013-06-26 13:44:50 UTC
Fixed upstream:
commit 306c49ffd56a1c72b1892d50f2a75531c62f4a1d
Author:     Ján Tomko <jtomko@redhat.com>
AuthorDate: 2013-06-26 13:07:24 +0200
Commit:     Ján Tomko <jtomko@redhat.com>
CommitDate: 2013-06-26 15:05:43 +0200

    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

git describe: v1.1.0-rc1-25-g306c49f

Posted downstream:
http://post-office.corp.redhat.com/archives/rhvirt-patches/2013-June/msg00663.html

Comment 9 zhpeng 2013-07-10 02:03:07 UTC
Test with: libvirt-0.10.2-19.el6
Steps same as comment 0 and no off-by-one error.
So it's verified.

Comment 10 zhpeng 2013-07-10 02:38:42 UTC
Sorry i have to change this to ON_QA, i use the guest xml with <vcpu> and i can create the guest, but if i ignore this part, i can't create the guest, and i found a leak. i'll attach the valgrind msgs and needinfo dev.


virsh define /dev/stdin <<EOF
<domain type='qemu'>
  <name>duck</name>
  <memory unit='MiB'>32</memory>
  <vcpu placement='static' cpuset='1'>1</vcpu>  
  <os>
    <type arch='x86_64' machine='pc'>hvm</type>
  </os>
</domain>
EOF
Domain duck defined from /dev/stdin


virsh create /dev/stdin <<EOF
<domain type='qemu'>
  <name>duck</name>
  <memory unit='MiB'>32</memory>
  <os>
    <type arch='x86_64' machine='pc'>hvm</type>
  </os>
</domain>
EOF
error: Failed to create domain from /dev/stdin
error: internal error process exited while connecting to monitor:

Comment 11 zhpeng 2013-07-10 02:39:18 UTC
Created attachment 771344 [details]
valgrind msg

Comment 12 Jincheng Miao 2013-07-10 10:20:32 UTC
I also can't boot a guest with <vcpu>
# virsh create /dev/stdin <<EOF
<domain type='qemu'>
  <name>duck</name>
  <memory unit='MiB'>32</memory>
  <vcpu placement='static' cpuset='1'>1</vcpu>  
  <os>
    <type arch='x86_64' machine='pc'>hvm</type>
  </os>
</domain>
EOF

error: Failed to create domain from /dev/stdin
error: internal error process exited while connecting to monitor:

It seems that <vcpu> is innocent. But I can define guest with/without <vcpu>.

So the problem is _start_ domain.

In valgrind log, we can see:
==15199== could not unlink /tmp/vgdb-pipe-from-vgdb-to-15199-by-root-on-zhpeng.example.com
==15199== could not unlink /tmp/vgdb-pipe-to-vgdb-from-15199-by-root-on-zhpeng.example.com
==15199== could not unlink /tmp/vgdb-pipe-shared-mem-vgdb-15199-by-root-on-zhpeng.example.com
==15199== execve(0x13e21890(/usr/libexec/qemu-kvm), 0x13e23370, 0x13b7ccb0) failed, errno 13
==15199== EXEC FAILED: I can't recover from execve() failing, so I'm dying.

Maybe valgrind break the start of domain. This is Valgrind-3.8.1.

@Jan, which version of valgrind you use?

Comment 13 Ján Tomko 2013-07-10 12:49:01 UTC
I'm using valgrind-3.8.1-3.2.el6.x86_64 and can't start the domain in valgrind either, unless I disable selinux. And it should be valgrind's fault, because it works for me without it.

None of the leaks seem important to me.

Comment 14 zhpeng 2013-07-15 03:17:39 UTC
Created attachment 773492 [details]
downgrade valgrind and test it again.

Comment 15 zhpeng 2013-07-15 03:21:15 UTC
I tested it again with valgrind-3.6.0-5.el6.x86_64. And it's really a valgrind fault.
I can't find any off-by-one error anymore. So it's verified.

Comment 19 errata-xmlrpc 2013-11-21 09:04:24 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2013-1581.html


Note You need to log in before you can comment on or make changes to this bug.