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

[Peter Maydell]

  Branch: refs/heads/staging
  Home:   https://github.com/qemu/qemu
  Commit: c3c6f66b292ba9b62cc01f0535e38d57107c727c
      
https://github.com/qemu/qemu/commit/c3c6f66b292ba9b62cc01f0535e38d57107c727c
  Author: Marc-André Lureau <marcandre.lureau@redhat.com>
  Date:   2021-03-27 (Sat, 27 Mar 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: 7d1dd9c47321d84bfa23db01b7f7e21f5058b22c
      
https://github.com/qemu/qemu/commit/7d1dd9c47321d84bfa23db01b7f7e21f5058b22c
  Author: Priyankar Jain <priyankar.jain@nutanix.com>
  Date:   2021-03-27 (Sat, 27 Mar 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: cadadb4464dbdbd612985a02022b687f0ef98ddb
      
https://github.com/qemu/qemu/commit/cadadb4464dbdbd612985a02022b687f0ef98ddb
  Author: Marc-André Lureau <marcandre.lureau@redhat.com>
  Date:   2021-03-27 (Sat, 27 Mar 2021)

  Changed paths:
    R docs/_templates/editpage.html
    M docs/conf.py
    R docs/devel/_templates/editpage.html
    R docs/interop/_templates/editpage.html
    M docs/meson.build
    R docs/specs/_templates/editpage.html
    A docs/sphinx-static/theme_overrides.css
    R docs/system/_templates/editpage.html
    R docs/tools/_templates/editpage.html
    R docs/user/_templates/editpage.html
    M tests/docker/dockerfiles/alpine.docker
    M tests/docker/dockerfiles/debian10.docker
    M tests/docker/dockerfiles/fedora.docker
    M tests/docker/dockerfiles/ubuntu.docker
    M tests/docker/dockerfiles/ubuntu1804.docker
    M tests/docker/dockerfiles/ubuntu2004.docker

  Log Message:
  -----------
  sphinx: adopt kernel readthedoc theme

The default "alabaster" sphinx theme has a couple shortcomings:
- the navbar moves along the page
- the search bar is not always at the same place
- it lacks some contrast and colours

The "rtd" theme from readthedocs.org is a popular third party theme used
notably by the kernel, with a custom style sheet. I like it better,
perhaps others do too. It also simplifies the "Edit on Gitlab" links.

Tweak a bit the custom theme to match qemu.org style, use the
QEMU logo, and favicon etc.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210323115328.4146052-1-marcandre.lureau@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>


  Commit: 79b35718563a1a52f476bb87ffb2b1c2601c6bde
      
https://github.com/qemu/qemu/commit/79b35718563a1a52f476bb87ffb2b1c2601c6bde
  Author: Marc-André Lureau <marcandre.lureau@redhat.com>
  Date:   2021-03-27 (Sat, 27 Mar 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: 4485084a4456591b17f03169bcaca6c29b1ca8ca
      
https://github.com/qemu/qemu/commit/4485084a4456591b17f03169bcaca6c29b1ca8ca
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-03-27 (Sat, 27 Mar 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: 463bac02bf701f466586846ff16cf6d3f3581e11
      
https://github.com/qemu/qemu/commit/463bac02bf701f466586846ff16cf6d3f3581e11
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-03-27 (Sat, 27 Mar 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: 97446946d96749fc8a111b694b718fbd4cb128c1
      
https://github.com/qemu/qemu/commit/97446946d96749fc8a111b694b718fbd4cb128c1
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-03-27 (Sat, 27 Mar 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.1616794852.git.lukasstraub2@web.de>


  Commit: 3ef6717694af2f8b6624561215e9c022fbd6d8cf
      
https://github.com/qemu/qemu/commit/3ef6717694af2f8b6624561215e9c022fbd6d8cf
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-03-27 (Sat, 27 Mar 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.1616794852.git.lukasstraub2@web.de>


  Commit: 4a22b8d272ae76df7e4f90f30de813371af38c7d
      
https://github.com/qemu/qemu/commit/4a22b8d272ae76df7e4f90f30de813371af38c7d
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-03-27 (Sat, 27 Mar 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.1616794852.git.lukasstraub2@web.de>


  Commit: f57d44b452e11d8b7c9743476c30a8d0f80926de
      
https://github.com/qemu/qemu/commit/f57d44b452e11d8b7c9743476c30a8d0f80926de
  Author: Lukas Straub <lukasstraub2@web.de>
  Date:   2021-03-27 (Sat, 27 Mar 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: 
<7c2f1ddcaa1c97462cb0b834d5aa7368283aa67d.1616794852.git.lukasstraub2@web.de>


  Commit: 5b1ddcbd0c5ee1260017c0248f54ea559eec51b9
      
https://github.com/qemu/qemu/commit/5b1ddcbd0c5ee1260017c0248f54ea559eec51b9
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2021-03-29 (Mon, 29 Mar 2021)

  Changed paths:
    M MAINTAINERS
    M backends/dbus-vmstate.c
    M chardev/char-socket.c
    M chardev/char.c
    R docs/_templates/editpage.html
    M docs/conf.py
    R docs/devel/_templates/editpage.html
    M docs/devel/index.rst
    R docs/interop/_templates/editpage.html
    M docs/interop/index.rst
    M docs/meson.build
    R docs/specs/_templates/editpage.html
    M docs/specs/index.rst
    A docs/sphinx-static/theme_overrides.css
    R docs/system/_templates/editpage.html
    M docs/system/index.rst
    R docs/tools/_templates/editpage.html
    M docs/tools/index.rst
    R docs/user/_templates/editpage.html
    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/docker/dockerfiles/alpine.docker
    M tests/docker/dockerfiles/debian10.docker
    M tests/docker/dockerfiles/fedora.docker
    M tests/docker/dockerfiles/ubuntu.docker
    M tests/docker/dockerfiles/ubuntu1804.docker
    M tests/docker/dockerfiles/ubuntu2004.docker
    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.

Peter, let me know if you would rather split that PR.

# gpg: Signature made Sun 28 Mar 2021 19:44:40 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
  sphinx: adopt kernel readthedoc theme
  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/ec2e6e016d24...5b1ddcbd0c5e