Bug 978356 - valgrind shows an off-by-one error in virCgroupGetValueStr
valgrind shows an off-by-one error in virCgroupGetValueStr
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: libvirt (Show other bugs)
6.4
Unspecified Unspecified
high Severity high
: rc
: ---
Assigned To: Ján Tomko
Virtualization Bugs
: ZStream
Depends On:
Blocks: 984553 984561
  Show dependency treegraph
 
Reported: 2013-06-26 08:48 EDT by Ján Tomko
Modified: 2013-11-21 04:04 EST (History)
11 users (show)

See Also:
Fixed In Version: libvirt-0.10.2-19.el6
Doc Type: Bug Fix
Doc Text:
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.
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-11-21 04:04:24 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
valgrind msg (19.03 KB, text/plain)
2013-07-09 22:39 EDT, zhpeng
no flags Details
downgrade valgrind and test it again. (21.86 KB, text/plain)
2013-07-14 23:17 EDT, zhpeng
no flags Details

  None (edit)
Description Ján Tomko 2013-06-26 08:48:46 EDT
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 09:09:54 EDT
Upstream patch proposed:
https://www.redhat.com/archives/libvir-list/2013-June/msg01115.html
Comment 3 Ján Tomko 2013-06-26 09:44:50 EDT
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-09 22:03:07 EDT
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-09 22:38:42 EDT
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-09 22:39:18 EDT
Created attachment 771344 [details]
valgrind msg
Comment 12 Jincheng Miao 2013-07-10 06:20:32 EDT
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 08:49:01 EDT
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-14 23:17:39 EDT
Created attachment 773492 [details]
downgrade valgrind and test it again.
Comment 15 zhpeng 2013-07-14 23:21:15 EDT
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 04:04:24 EST
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.