[Qemu-devel] [PATCH v1 0/5] Fix crashes with introspection

Thomas Huth posted 5 patches 7 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1531409463-3843-1-git-send-email-thuth@redhat.com
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
hw/arm/armv7m.c       |  9 ++++---
hw/arm/bcm2836.c      | 19 +++++--------
hw/arm/iotkit.c       | 74 ++++++++++++++++++++++-----------------------------
hw/core/sysbus.c      |  8 ++++++
hw/cpu/a15mpcore.c    |  8 +++---
hw/intc/armv7m_nvic.c |  5 ++--
include/hw/sysbus.h   |  3 +++
include/qom/object.h  | 19 +++++++++++++
qom/object.c          | 14 ++++++++++
9 files changed, 93 insertions(+), 66 deletions(-)
[Qemu-devel] [PATCH v1 0/5] Fix crashes with introspection
Posted by Thomas Huth 7 years, 3 months ago
As discovered recently, you can crash QEMU with a lot of devices
that do not get the reference counting of child objects right.
You just have to run 'device-list-properties' and call 'info qtree'
afterwards.
This patch series fixes a bunch of these problems in the ARM code.
I did not fix all problems yet, since it is quite time consuming
and I first want to get some feedback whether this is the right
way to go or not.

Thomas Huth (5):
  qom/object: Add a new function object_initialize_as_child()
  hw/core/sysbus: Add a function for creating and attaching an object
  hw/arm/bcm2836: Fix crash with device_add bcm2837 on unsupported
    machines
  hw/arm/armv7: Fix crash when introspecting the "iotkit" device
  hw/cpu/a15mpcore: Fix introspection problem with the a15mpcore_priv
    device

 hw/arm/armv7m.c       |  9 ++++---
 hw/arm/bcm2836.c      | 19 +++++--------
 hw/arm/iotkit.c       | 74 ++++++++++++++++++++++-----------------------------
 hw/core/sysbus.c      |  8 ++++++
 hw/cpu/a15mpcore.c    |  8 +++---
 hw/intc/armv7m_nvic.c |  5 ++--
 include/hw/sysbus.h   |  3 +++
 include/qom/object.h  | 19 +++++++++++++
 qom/object.c          | 14 ++++++++++
 9 files changed, 93 insertions(+), 66 deletions(-)

-- 
1.8.3.1


Re: [Qemu-devel] [PATCH v1 0/5] Fix crashes with introspection
Posted by Paolo Bonzini 7 years, 3 months ago
On 12/07/2018 17:30, Thomas Huth wrote:
> As discovered recently, you can crash QEMU with a lot of devices
> that do not get the reference counting of child objects right.
> You just have to run 'device-list-properties' and call 'info qtree'
> afterwards.
> This patch series fixes a bunch of these problems in the ARM code.
> I did not fix all problems yet, since it is quite time consuming
> and I first want to get some feedback whether this is the right
> way to go or not.

Patches 1-3 look like the way to go to me.

(FWIW, this is indeed an instance of the bug that Eduardo envisioned:
you have a reference to the inner object, and it becomes invalid after
the outer object is freed.  However, in this case the reference is
unnecessary so this is the right fix).

Thanks,

Paolo

> Thomas Huth (5):
>   qom/object: Add a new function object_initialize_as_child()
>   hw/core/sysbus: Add a function for creating and attaching an object
>   hw/arm/bcm2836: Fix crash with device_add bcm2837 on unsupported
>     machines
>   hw/arm/armv7: Fix crash when introspecting the "iotkit" device
>   hw/cpu/a15mpcore: Fix introspection problem with the a15mpcore_priv
>     device
> 
>  hw/arm/armv7m.c       |  9 ++++---
>  hw/arm/bcm2836.c      | 19 +++++--------
>  hw/arm/iotkit.c       | 74 ++++++++++++++++++++++-----------------------------
>  hw/core/sysbus.c      |  8 ++++++
>  hw/cpu/a15mpcore.c    |  8 +++---
>  hw/intc/armv7m_nvic.c |  5 ++--
>  include/hw/sysbus.h   |  3 +++
>  include/qom/object.h  | 19 +++++++++++++
>  qom/object.c          | 14 ++++++++++
>  9 files changed, 93 insertions(+), 66 deletions(-)
> 


Re: [Qemu-devel] [PATCH v1 0/5] Fix crashes with introspection
Posted by Peter Maydell 7 years, 3 months ago
On 12 July 2018 at 16:30, Thomas Huth <thuth@redhat.com> wrote:
> As discovered recently, you can crash QEMU with a lot of devices
> that do not get the reference counting of child objects right.
> You just have to run 'device-list-properties' and call 'info qtree'
> afterwards.
> This patch series fixes a bunch of these problems in the ARM code.
> I did not fix all problems yet, since it is quite time consuming
> and I first want to get some feedback whether this is the right
> way to go or not.
>
> Thomas Huth (5):
>   qom/object: Add a new function object_initialize_as_child()
>   hw/core/sysbus: Add a function for creating and attaching an object
>   hw/arm/bcm2836: Fix crash with device_add bcm2837 on unsupported
>     machines
>   hw/arm/armv7: Fix crash when introspecting the "iotkit" device
>   hw/cpu/a15mpcore: Fix introspection problem with the a15mpcore_priv
>     device

Seems good to me. I'm cc'ing Igor since he had some opinions when
I sent a previous (and rather more ad-hoc) attempt at reducing
the boilerplate around creating and parenting sysbus objects:
http://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04493.html

thanks
-- PMM