[Qemu-devel] [PATCH] hw/core: fix segmentation fault

Suramya Shah posted 1 patch 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170408150524.2870-1-shah.suramya@gmail.com
Test checkpatch failed
Test docker passed
Test s390x passed
hw/core/qdev.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] hw/core: fix segmentation fault
Posted by Suramya Shah 6 years, 11 months ago
Reproducer:
 $i386-softmmu/qemu-system-i386 -S -machine isapc,accel=tcg -device amd-iommu
Segmentation fault (core dumped)

Partial bt:
#0  bus_add_child (child=0x555556d4e520, bus=0x0) at hw/core/qdev.c:88
#1  qdev_set_parent_bus (dev=0x555556d4e520, bus=bus@entry=0x0)
at hw/core/qdev.c:119

Signed-off-by: Suramya Shah <shah.suramya@gmail.com>
---
 hw/core/qdev.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 1e7fb33..07a211b 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -84,7 +84,11 @@ static void bus_add_child(BusState *bus, DeviceState *child)
 {
     char name[32];
     BusChild *kid = g_malloc0(sizeof(*kid));
-
+    
+    if (!bus) {
+        error_report("bus not found ");
+        exit(0);
+    }
     kid->index = bus->max_index++;
     kid->child = child;
     object_ref(OBJECT(kid->child));
-- 
2.9.3


Re: [Qemu-devel] [PATCH] hw/core: fix segmentation fault
Posted by Peter Maydell 6 years, 11 months ago
On 8 April 2017 at 16:05, Suramya Shah <shah.suramya@gmail.com> wrote:
> Reproducer:
>  $i386-softmmu/qemu-system-i386 -S -machine isapc,accel=tcg -device amd-iommu
> Segmentation fault (core dumped)
>
> Partial bt:
> #0  bus_add_child (child=0x555556d4e520, bus=0x0) at hw/core/qdev.c:88
> #1  qdev_set_parent_bus (dev=0x555556d4e520, bus=bus@entry=0x0)
> at hw/core/qdev.c:119
>
> Signed-off-by: Suramya Shah <shah.suramya@gmail.com>
> ---
>  hw/core/qdev.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 1e7fb33..07a211b 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -84,7 +84,11 @@ static void bus_add_child(BusState *bus, DeviceState *child)
>  {
>      char name[32];
>      BusChild *kid = g_malloc0(sizeof(*kid));
> -
> +
> +    if (!bus) {
> +        error_report("bus not found ");
> +        exit(0);
> +    }
>      kid->index = bus->max_index++;
>      kid->child = child;
>      object_ref(OBJECT(kid->child));

Thanks for the patch, but this doesn't look like the right way to fix
this. Exiting with an error in a low-level function like this is not
really any better than crashing. This function is not supposed to
be called with a NULL bus pointer, so the question is: why is that
happening? Something in one of the calling functions, probably
several layers up the stack, has a bug. To fix this bug we need to
look at that code and identify why it is trying to add the child
device to a NULL bus and what bus it ought to be putting it on
instead.

thanks
-- PMM