[Qemu-commits] [qemu/qemu] 37f323: QemuOpts: Fix qemu_opts_foreach() dangling locatio...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 37f32349ea43f41ee8b9a253977ce1e46f576fc7
      
https://github.com/qemu/qemu/commit/37f32349ea43f41ee8b9a253977ce1e46f576fc7
  Author: Markus Armbruster <address@hidden>
  Date:   2016-04-28 (Thu, 28 Apr 2016)

  Changed paths:
    M util/qemu-option.c

  Log Message:
  -----------
  QemuOpts: Fix qemu_opts_foreach() dangling location regression

qemu_opts_foreach() pushes and pops a Location with automatic storage
duration.  Except it fails to pop when @func() returns non-zero.
cur_loc then points to unused stack space, and will most likely get
clobbered in short order.

Clobbered cur_loc can make loc_pop() and error_print_loc() crash or
report bogus locations.

Affects several qemu command line options as well as qemu-img,
qemu-io, qemu-nbd -object, and blkdebug's configuration file.

Broken in commit a4c7367, v2.4.0.

Reproducer:
    $ qemu-system-x86_64 -nodefaults -display none -object secret,id=foo,foo=bar

main() reports "Property '.foo' not found" like this:

    if (qemu_opts_foreach(qemu_find_opts("object"),
                    user_creatable_add_opts_foreach,
                    object_create_delayed, &err)) {
  error_report_err(err);
  exit(1);
    }

cur_loc then points to where qemu_opts_foreach()'s Location used to
be, i.e. unused stack space.  With optimization, this Location doesn't
get clobbered for me, and also happens to be the correct location.
Without optimization, it does get clobbered in a way that makes
error_report_err() report no location.

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: d9d3aaea0b3fbb5028e20316bdb93359487cd01f
      
https://github.com/qemu/qemu/commit/d9d3aaea0b3fbb5028e20316bdb93359487cd01f
  Author: Markus Armbruster <address@hidden>
  Date:   2016-04-28 (Thu, 28 Apr 2016)

  Changed paths:
    M replay/replay.c

  Log Message:
  -----------
  replay: Fix dangling location bug in replay_configure()

replay_configure() pushes and pops a Location with automatic storage
duration.  Except it fails to pop when -icount parameter "rr" isn't
given.  cur_loc then points to unused stack space, and will most
likely get clobbered in short order.

Clobbered cur_loc can make loc_pop() and error_print_loc() crash or
report bogus locations.

Broken in commit 890ad55.

I didn't take the time to find a reproducer.

Cc: Eduardo Habkost <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Eduardo Habkost <address@hidden>


  Commit: 51b9b478cc238ad23a78ffd713f9c18bbc3907e6
      
https://github.com/qemu/qemu/commit/51b9b478cc238ad23a78ffd713f9c18bbc3907e6
  Author: Markus Armbruster <address@hidden>
  Date:   2016-04-28 (Thu, 28 Apr 2016)

  Changed paths:
    M include/qom/object_interfaces.h
    M qemu-img.c
    M qemu-io.c
    M qemu-nbd.c
    M qom/object_interfaces.c
    M vl.c

  Log Message:
  -----------
  qom: -object error messages lost location, restore it

qemu_opts_foreach() runs its callback with the error location set to
the option's location.  Any errors the callback reports use the
option's location automatically.

Commit 90998d5 moved the actual error reporting from "inside"
qemu_opts_foreach() to after it.  Here's a typical hunk:

         if (qemu_opts_foreach(qemu_find_opts("object"),
    -                          object_create,
    -                          object_create_initial, NULL)) {
    +                          user_creatable_add_opts_foreach,
    +                          object_create_initial, &err)) {
    +        error_report_err(err);
             exit(1);
         }

Before, object_create() reports from within qemu_opts_foreach(), using
the option's location.  Afterwards, we do it after
qemu_opts_foreach(), using whatever location happens to be current
there.  Commonly a "none" location.

This is because Error objects don't have location information.
Problematic.

Reproducer:

    $ qemu-system-x86_64 -nodefaults -display none -object secret,id=foo,foo=bar
    qemu-system-x86_64: Property '.foo' not found

Note no location.  This commit restores it:

    qemu-system-x86_64: -object secret,id=foo,foo=bar: Property '.foo' not found

Note that the qemu_opts_foreach() bug just fixed could mask the bug
here: if the location it leaves dangling hasn't been clobbered, yet,
it's the correct one.

Reported-by: Eric Blake <address@hidden>
Cc: Daniel P. Berrange <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[Paragraph on Error added to commit message]


  Commit: 736f85d5db701cc9d464a03b583d4a04606a0dd5
      
https://github.com/qemu/qemu/commit/736f85d5db701cc9d464a03b583d4a04606a0dd5
  Author: Peter Maydell <address@hidden>
  Date:   2016-04-28 (Thu, 28 Apr 2016)

  Changed paths:
    M include/qom/object_interfaces.h
    M qemu-img.c
    M qemu-io.c
    M qemu-nbd.c
    M qom/object_interfaces.c
    M replay/replay.c
    M util/qemu-option.c
    M vl.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2016-04-28' into 
staging

Fix dangling pointers and error message regressions

# gpg: Signature made Thu 28 Apr 2016 07:25:51 BST using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <address@hidden>"
# gpg:                 aka "Markus Armbruster <address@hidden>"

* remotes/armbru/tags/pull-error-2016-04-28:
  qom: -object error messages lost location, restore it
  replay: Fix dangling location bug in replay_configure()
  QemuOpts: Fix qemu_opts_foreach() dangling location regression

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/61861eff6927...736f85d5db70