Bug 810983 - QAPI may double free on errors
QAPI may double free on errors
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: qemu-kvm (Show other bugs)
6.3
Unspecified Unspecified
unspecified Severity unspecified
: rc
: ---
Assigned To: Luiz Capitulino
Virtualization Bugs
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-09 15:27 EDT by Luiz Capitulino
Modified: 2012-06-20 07:46 EDT (History)
11 users (show)

See Also:
Fixed In Version: qemu-kvm-0.12.1.2-2.273.el6
Doc Type: Bug Fix
Doc Text:
No documentation needed.
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-06-20 07:46:27 EDT
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)
test files (3.42 KB, application/x-gzip)
2012-05-02 10:05 EDT, Laszlo Ersek
no flags Details

  None (edit)
Description Luiz Capitulino 2012-04-09 15:27:13 EDT
This was found by Laszlo Ersek during the QAPI backport review process for 6.3. It's fixed upstream by commit f24582d6ad8a080e008974c000bf0ae635d036ac.

More information can also be found in the upstream thread about the bug:

 http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03276.html
Comment 2 Luiz Capitulino 2012-04-10 10:54:24 EDT
Michen, which information do you need?
Comment 3 Miya Chen 2012-04-10 11:07:17 EDT
sorry, Luiz, I want to know how QE are supposed to verify this issue, thanks.
Comment 4 Luiz Capitulino 2012-04-10 13:05:15 EDT
Miya, upstream we have a test-case for it, but the test-case can't be backported. I'm not sure how easy is to trigger this, actually it's even possible that it's not possible to trigger this currently.

Laszlo, any ideas?

PS: The fact that this _may_ be impossible to trigger lowers the importance of this bug, but I think it's worth it to fix it anyway as some important functionalities (like live snapshots) now depends on the qapi.
Comment 5 juzhang 2012-04-10 19:43:49 EDT
(In reply to comment #4)
> Miya, upstream we have a test-case for it, but the test-case can't be
> backported. I'm not sure how easy is to trigger this, actually it's even
> possible that it's not possible to trigger this currently.
Yes,we found trigger for upstream in http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03276.html. will mark qa_ack+ first. please paste in bz if find the trigger. thanks.
Comment 6 Laszlo Ersek 2012-04-11 07:06:24 EDT
(In reply to comment #4)
> Miya, upstream we have a test-case for it, but the test-case can't be
> backported. I'm not sure how easy is to trigger this, actually it's even
> possible that it's not possible to trigger this currently.
> 
> Laszlo, any ideas?

I can only think of open-coding what the test case does. Paolo's test case is embedded in a test framework which we don't have, which is why it's not possible to backport the test on its own. I think.

To trigger the problem, a C struct with an "invalid" ("unnamed") enum value has to be constructed. If that happened in qemu, that would be a bug in its own right, even without triggering the output visitor failure.

So the only thing I can think of is a standalone C program (mimicking the upstream test suite) attached to this BZ. QE would first have to build qemu-kvm locally, then copy the C program into the source tree, and build it with the QMP output visitor object file (using a gcc command line we'd provide). The double free should crash this standalone binary (or make valgrind complain).

If this would take too much effort, then mark this BZ as SanityOnly perhaps. Plus open another BZ for 6.4 marked TestOnly, according to what Paolo said in <http://post-office.corp.redhat.com/archives/rhvirt-patches/2012-April/msg00200.html>.
Comment 10 Luiz Capitulino 2012-04-23 14:41:18 EDT
    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
No documentation needed.
Comment 11 Chao Yang 2012-05-02 05:38:10 EDT
(In reply to comment #6)
> (In reply to comment #4)
> > Miya, upstream we have a test-case for it, but the test-case can't be
> > backported. I'm not sure how easy is to trigger this, actually it's even
> > possible that it's not possible to trigger this currently.
> > 
> > Laszlo, any ideas?
> 
> I can only think of open-coding what the test case does. Paolo's test case is
> embedded in a test framework which we don't have, which is why it's not
> possible to backport the test on its own. I think.
> 
> To trigger the problem, a C struct with an "invalid" ("unnamed") enum value has
> to be constructed. If that happened in qemu, that would be a bug in its own
> right, even without triggering the output visitor failure.
> 
> So the only thing I can think of is a standalone C program (mimicking the
> upstream test suite) attached to this BZ. QE would first have to build qemu-kvm
> locally, then copy the C program into the source tree, and build it with the
> QMP output visitor object file (using a gcc command line we'd provide). The
> double free should crash this standalone binary (or make valgrind complain).
> 
Hi Laszlo,
 Can you please attach the C program and gcc command you mentioned above? Thanks in advance.
Comment 12 Laszlo Ersek 2012-05-02 10:05:07 EDT
Created attachment 581624 [details]
test files

(1) Start with a RHEL-6.2 (or RHEL-6.3 Beta) user (non-root) account.

(2) On RHEL-6.2, install the following packages (most recent .el6 versions)
    from Brew:

    - spice-protocol
    - spice-server (+ spice-server-devel)
    - usbredir (+ usbredir-devel)

(3) From Brew, download the qemu-kvm *source* RPM that is to be tested.
    Install it (as non-root) with "rpm -ivh". (Any extra dependencies should
    be available to yum, so install those from the normal yum repos.)

    From now on I'll assume that we use "$HOME/rpmbuild" as rpmbuild root
    directory.

(4) Do the %build stage:

    cd $HOME/rpmbuild/SPECS
    rpmbuild -bc qemu-kvm.spec

(5) Once the build is complete, download the attachment and extract it under
    $HOME/tmp. Then issue the following:

    cd $HOME/tmp/bz810983_test
    Q=$HOME/rpmbuild/BUILD/qemu-kvm-0.12.1.2

(6) Generate the required test visitor source files (the Q setting must be
    in effect from step 5):

    for F in c h; do
      for TYPE in types visit; do
        python $Q/scripts/qapi-$TYPE.py -$F -o . -p "test-" \
            < qapi-schema-test.json
      done
    done

(7) Build the test utility (the Q environment variable must be set from step
    5):

    gcc                             -o test-qmp-output-visitor  \
    -I $Q                           -I $Q/fpu                   \
    -I $Q/slirp                     -I /usr/include/glib-2.0    \
    -I /usr/lib64/glib-2.0/include  $Q/async.o                  \
    $Q/compatfd.o                   $Q/cutils.o                 \
    $Q/error.o                      $Q/iohandler.o              \
    $Q/json-lexer.o                 $Q/json-parser.o            \
    $Q/json-streamer.o              $Q/module.o                 \
    $Q/notify.o                     $Q/osdep.o                  \
    $Q/qapi/qapi-dealloc-visitor.o  $Q/qapi/qapi-visit-core.o   \
    $Q/qapi/qmp-dispatch.o          $Q/qapi/qmp-input-visitor.o \
    $Q/qapi/qmp-output-visitor.o    $Q/qapi/qmp-registry.o      \
    $Q/qbool.o                      $Q/qdict.o                  \
    $Q/qemu-error.o                 $Q/qemu-malloc.o            \
    $Q/qemu-tool.o                  $Q/qerror.o                 \
    $Q/qfloat.o                     $Q/qint.o                   \
    $Q/qjson.o                      $Q/qlist.o                  \
    $Q/qstring.o                    $Q/trace-dtrace.o           \
    test-qapi-types.c               test-qapi-visit.c           \
    test-qmp-output-visitor.c       -l glib-2.0                 \
    -l gthread-2.0                  -l rt                       \
    -l uuid                         -l z                        \
    -pthread

(8) Run the test utility with the "gtester" command (part of glib2-devel,
    which must have been installed for step 4):

    gtester -k --verbose -m=quick test-qmp-output-visitor

(9) With the BZ fixed, the output must contain the lines

    [...]
    /visitor/output/struct-errors:                                       OK
    [...]
    PASS: test-qmp-output-visitor
Comment 13 Chao Yang 2012-05-03 02:47:53 EDT
Thanks Laszlo a lot for your reproducer.

Reproduction:
------------
Reproduced on qemu-kvm-0.12.1.2-2.272.el6.src.rpm.
After step (8) in above reproducer:
[test@dhcp-8-162 bz810983_test]$ gtester -k --verbose -m=quick test-qmp-output-visitor
TEST: test-qmp-output-visitor... (pid=6831)
  /visitor/output/int:                                                 OK
  /visitor/output/bool:                                                OK
  /visitor/output/number:                                              OK
  /visitor/output/string:                                              OK
  /visitor/output/no-string:                                           OK
  /visitor/output/enum:                                                OK
  /visitor/output/enum-errors:                                         OK
  /visitor/output/struct:                                              OK
  /visitor/output/struct-nested:                                       OK
  /visitor/output/struct-errors:                                       *** glibc detected *** test-qmp-output-visitor: free(): corrupted unsorted chunks: 0x0000000000fa4f90 ***
*** glibc detected *** test-qmp-output-visitor: corrupted double-linked list: 0x0000000000fa60e0 ***


Verification:
-------------
Verified on qemu-kvm-0.12.1.2-2.290.el6.src.rpm.
After step (8) in above reproducer:
[test@dhcp-8-162 bz810983_test]$ gtester -k --verbose -m=quick test-qmp-output-visitor
TEST: test-qmp-output-visitor... (pid=29027)
  /visitor/output/int:                                                 OK
  /visitor/output/bool:                                                OK
  /visitor/output/number:                                              OK
  /visitor/output/string:                                              OK
  /visitor/output/no-string:                                           OK
  /visitor/output/enum:                                                OK
  /visitor/output/enum-errors:                                         OK
  /visitor/output/struct:                                              OK
  /visitor/output/struct-nested:                                       OK
  /visitor/output/struct-errors:                                       OK
  /visitor/output/list:                                                OK
  /visitor/output/list-qapi-free:                                      OK
  /visitor/output/union:                                               OK
PASS: test-qmp-output-visitor



Conclusion:
-----------
As per above, this issue has been fixed correctly.
Comment 16 errata-xmlrpc 2012-06-20 07:46:27 EDT
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-2012-0746.html

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