[Qemu-commits] [qemu/qemu] 64e16f: util: fix use-after-free in module_load_one

[Peter Maydell]

  Branch: refs/heads/staging
  Home:   https://github.com/qemu/qemu
  Commit: 64e16fbbf49ce81b37841480d14b0caf5753c98e
      
https://github.com/qemu/qemu/commit/64e16fbbf49ce81b37841480d14b0caf5753c98e
  Author: Marc-André Lureau <marcandre.lureau@redhat.com>
  Date:   2021-04-01 (Thu, 01 Apr 2021)

  Changed paths:
    M util/module.c

  Log Message:
  -----------
  util: fix use-after-free in module_load_one

g_hash_table_add always retains ownership of the pointer passed in as
the key. Its return status merely indicates whether the added entry was
new, or replaced an existing entry. Thus key must never be freed after
this method returns.

Spotted by ASAN:

==2407186==ERROR: AddressSanitizer: heap-use-after-free on address 
0x6020003ac4f0 at pc 0x7ffff766659c bp 0x7fffffffd1d0 sp 0x7fffffffc980
READ of size 1 at 0x6020003ac4f0 thread T0
    #0 0x7ffff766659b  (/lib64/libasan.so.6+0x8a59b)
    #1 0x7ffff6bfa843 in g_str_equal ../glib/ghash.c:2303
    #2 0x7ffff6bf8167 in g_hash_table_lookup_node ../glib/ghash.c:493
    #3 0x7ffff6bf9b78 in g_hash_table_insert_internal ../glib/ghash.c:1598
    #4 0x7ffff6bf9c32 in g_hash_table_add ../glib/ghash.c:1689
    #5 0x5555596caad4 in module_load_one ../util/module.c:233
    #6 0x5555596ca949 in module_load_one ../util/module.c:225
    #7 0x5555596ca949 in module_load_one ../util/module.c:225
    #8 0x5555596cbdf4 in module_load_qom_all ../util/module.c:349

Typical C bug...

Fixes: 90629122d2e ("module: use g_hash_table_add()")
Cc: qemu-stable@nongnu.org
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210316134456.3243102-1-marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


  Commit: 1d9fa7a859ac78e1809a376a4f2a443290e755e0
      
https://github.com/qemu/qemu/commit/1d9fa7a859ac78e1809a376a4f2a443290e755e0
  Author: Priyankar Jain <priyankar.jain@nutanix.com>
  Date:   2021-04-01 (Thu, 01 Apr 2021)

  Changed paths:
    M backends/dbus-vmstate.c

  Log Message:
  -----------
  dbus-vmstate: Increase the size of input stream buffer used during load

This commit fixes an issue where migration is failing in the load phase
because of a false alarm about data unavailability.

Following is the error received when the amount of data to be transferred
exceeds the default buffer size setup by G_BUFFERED_INPUT_STREAM(4KiB),
even when the maximum data size supported by this backend is 1MiB
(DBUS_VMSTATE_SIZE_LIMIT):

  dbus_vmstate_post_load: Invalid vmstate size: 4364
  qemu-kvm: error while loading state for instance 0x0 of device 
'dbus-vmstate/dbus-vmstate'

This commit sets the size of the input stream buffer used during load to
DBUS_VMSTATE_SIZE_LIMIT which is the maximum amount of data a helper can
send during save phase.
Secondly, this commit makes sure that the input stream buffer is loaded before
checking the size of the data available in it, rectifying the false alarm about
data unavailability.

Fixes: 5010cec2bc87 ("Add dbus-vmstate object")
Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
Message-Id: 
<cdaad4718e62bf22fd5e93ef3e252de20da5c17c.1612273156.git.priyankar.jain@nutanix.com>
[ Modified printf format for gsize ]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


  Commit: 816f93b20045f3363a4bc1c31e5e7aebbb6c1087
      
https://github.com/qemu/qemu/commit/816f93b20045f3363a4bc1c31e5e7aebbb6c1087
  Author: Marc-André Lureau <marcandre.lureau@redhat.com>
  Date:   2021-04-01 (Thu, 01 Apr 2021)

  Changed paths:
    M docs/devel/index.rst
    M docs/interop/index.rst
    M docs/specs/index.rst
    M docs/system/index.rst
    M docs/tools/index.rst
    M docs/user/index.rst

  Log Message:
  -----------
  docs: simplify each section title

Now that we merged into one doc, it makes the nav looks nicer.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20210323074704.4078381-1-marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>


  Commit: 1a92d6d500e5de762bad78bee1362a7dafb909fd
      
https://github.com/qemu/qemu/commit/1a92d6d500e5de762bad78bee1362a7dafb909fd
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-04-01 (Thu, 01 Apr 2021)

  Changed paths:
    M MAINTAINERS
    M chardev/char-socket.c
    M include/qemu/yank.h
    M migration/channel.c
    M migration/meson.build
    M migration/multifd.c
    M migration/qemu-file-channel.c
    A migration/yank_functions.c
    A migration/yank_functions.h
    M stubs/yank.c
    M util/yank.c

  Log Message:
  -----------
  yank: Remove dependency on qiochannel

Remove dependency on qiochannel by removing yank_generic_iochannel and
letting migration and chardev use their own yank function for
iochannel.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: 
<20ff143fc2db23e27cd41d38043e481376c9cec1.1616521341.git.lukasstraub2@web.de>


  Commit: e0150291ec87234e1c3d01eb9ad4c6315f5306c4
      
https://github.com/qemu/qemu/commit/e0150291ec87234e1c3d01eb9ad4c6315f5306c4
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-04-01 (Thu, 01 Apr 2021)

  Changed paths:
    M MAINTAINERS
    M stubs/meson.build
    R stubs/yank.c
    M util/meson.build

  Log Message:
  -----------
  yank: Always link full yank code

Yank now only depends on util and can be always linked in. Also remove
the stubs as they are not needed anymore.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: 
<997aa12a28c555d8a3b7a363b3bda5c3cf1821ba.1616521341.git.lukasstraub2@web.de>


  Commit: 789fd6934e1511d7e487c6277ab762189ddd05b3
      
https://github.com/qemu/qemu/commit/789fd6934e1511d7e487c6277ab762189ddd05b3
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-04-01 (Thu, 01 Apr 2021)

  Changed paths:
    M chardev/char.c

  Log Message:
  -----------
  chardev/char.c: Move object_property_try_add_child out of chardev_new

Move object_property_try_add_child out of chardev_new into it's
callers. This is a preparation for the next patches to fix yank
with the chardev-change case.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Li Zhang <li.zhang@cloud.ionos.com>
Message-Id: 
<b2a5092ec681737bc3a21ea16f3c00848b277521.1617127849.git.lukasstraub2@web.de>


  Commit: f36b0efd7f1a4b0383e1e36bc1c450ba4d8b7c6c
      
https://github.com/qemu/qemu/commit/f36b0efd7f1a4b0383e1e36bc1c450ba4d8b7c6c
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-04-01 (Thu, 01 Apr 2021)

  Changed paths:
    M chardev/char.c

  Log Message:
  -----------
  chardev/char.c: Always pass id to chardev_new

Always pass the id to chardev_new, since it is needed to register
the yank instance for the chardev. Also, after checking that
nothing calls chardev_new with id=NULL, assert() that id!=NULL.

This fixes a crash when using chardev-change to change a chardev
to chardev-socket, which attempts to register a yank instance.
This in turn tries to dereference the NULL-pointer.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Li Zhang <li.zhang@cloud.ionos.com>
Message-Id: 
<3e669b6c160aa7278e37c4d95e0445574f96c7b7.1617127849.git.lukasstraub2@web.de>


  Commit: feb774ca3fc08afc1404f75c06fbaeea5fdbcd19
      
https://github.com/qemu/qemu/commit/feb774ca3fc08afc1404f75c06fbaeea5fdbcd19
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-04-01 (Thu, 01 Apr 2021)

  Changed paths:
    M chardev/char-socket.c
    M chardev/char.c
    M include/chardev/char.h

  Log Message:
  -----------
  chardev: Fix yank with the chardev-change case

When changing from chardev-socket (which supports yank) to
chardev-socket again, it fails, because the new chardev attempts
to register a new yank instance. This in turn fails, as there
still is the yank instance from the current chardev. Also,
the old chardev shouldn't unregister the yank instance when it
is freed.

To fix this, now the new chardev only registers a yank instance if
the current chardev doesn't support yank and thus hasn't registered
one already. Also, when the old chardev is freed, it now only
unregisters the yank instance if the new chardev doesn't need it.

If the initialization of the new chardev fails, it still has
chr->handover_yank_instance set and won't unregister the yank
instance when it is freed.

s->registered_yank is always true here, as chardev-change only works
on user-visible chardevs and those are guraranteed to register a
yank instance as they are initialized via
chardev_new()
 qemu_char_open()
  cc->open() (qmp_chardev_open_socket()).

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Li Zhang <li.zhang@cloud.ionos.com>
Message-Id: 
<9637888d7591d2971975188478bb707299a1dc04.1617127849.git.lukasstraub2@web.de>


  Commit: d3a0bb7706520928f8493fabaee76532b5b1adb4
      
https://github.com/qemu/qemu/commit/d3a0bb7706520928f8493fabaee76532b5b1adb4
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-04-01 (Thu, 01 Apr 2021)

  Changed paths:
    M MAINTAINERS
    M tests/unit/meson.build
    A tests/unit/test-yank.c

  Log Message:
  -----------
  tests: Add tests for yank with the chardev-change case

Add tests for yank with the chardev-change case.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Li Zhang <li.zhang@cloud.ionos.com>
Message-Id: 
<697ce111503a8bab011d21519ae0b6b07041ec9a.1617127849.git.lukasstraub2@web.de>


  Commit: 415fa2fe91e2a49fe8d56d6aacc8f8db82c74775
      
https://github.com/qemu/qemu/commit/415fa2fe91e2a49fe8d56d6aacc8f8db82c74775
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2021-04-01 (Thu, 01 Apr 2021)

  Changed paths:
    M MAINTAINERS
    M backends/dbus-vmstate.c
    M chardev/char-socket.c
    M chardev/char.c
    M docs/devel/index.rst
    M docs/interop/index.rst
    M docs/specs/index.rst
    M docs/system/index.rst
    M docs/tools/index.rst
    M docs/user/index.rst
    M include/chardev/char.h
    M include/qemu/yank.h
    M migration/channel.c
    M migration/meson.build
    M migration/multifd.c
    M migration/qemu-file-channel.c
    A migration/yank_functions.c
    A migration/yank_functions.h
    M stubs/meson.build
    R stubs/yank.c
    M tests/unit/meson.build
    A tests/unit/test-yank.c
    M util/meson.build
    M util/module.c
    M util/yank.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/marcandre/tags/for-6.0-pull-request' 
into staging

For 6.0 misc patches under my radar.

V2:
 - "tests: Add tests for yank with the chardev-change case" updated
 - drop the readthedoc theme patch

# gpg: Signature made Thu 01 Apr 2021 12:54:52 BST
# gpg:                using RSA key 87A9BD933F87C606D276F62DDAE8E10975969CE5
# gpg:                issuer "marcandre.lureau@redhat.com"
# gpg: Good signature from "Marc-André Lureau <marcandre.lureau@redhat.com>" 
[full]
# gpg:                 aka "Marc-André Lureau <marcandre.lureau@gmail.com>" 
[full]
# Primary key fingerprint: 87A9 BD93 3F87 C606 D276  F62D DAE8 E109 7596 9CE5

* remotes/marcandre/tags/for-6.0-pull-request:
  tests: Add tests for yank with the chardev-change case
  chardev: Fix yank with the chardev-change case
  chardev/char.c: Always pass id to chardev_new
  chardev/char.c: Move object_property_try_add_child out of chardev_new
  yank: Always link full yank code
  yank: Remove dependency on qiochannel
  docs: simplify each section title
  dbus-vmstate: Increase the size of input stream buffer used during load
  util: fix use-after-free in module_load_one

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Compare: https://github.com/qemu/qemu/compare/00084bab87c4...415fa2fe91e2