diff mbox series

QOM crash via soundhw_init()

Message ID bbd3c42b-5069-d5e8-0b97-70ff5135801c@linaro.org
State New
Headers show
Series QOM crash via soundhw_init() | expand

Commit Message

Philippe Mathieu-Daudé Oct. 12, 2023, 10:53 a.m. UTC
Hi Martin, Paolo, Markus, Marc-André,

With the following changes:

-- >8 --

---

I'm getting:

$ ./qemu-system-ppc -M 40p -S -nodefaults
==3518191==WARNING: ASan doesn't fully support makecontext/swapcontext 
functions and may produce false positives in some cases!
qom/object.c:935:13: runtime error: member access within null pointer of 
type 'struct TypeImpl'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==3518191==ERROR: AddressSanitizer: SEGV on unknown address 
0x000000000000 (pc 0x55f728f5e179 bp 0x7ffd91999220 sp 0x7ffd919991d0 T0)
==3518191==The signal is caused by a READ memory access.
==3518191==Hint: address points to the zero page.
     #0 0x55f728f5e179 in object_class_dynamic_cast qom/object.c:935
     #1 0x55f728f5d9f4 in object_dynamic_cast qom/object.c:876
     #2 0x55f728f6605a in object_resolve_abs_path qom/object.c:2096
     #3 0x55f728f662f1 in object_resolve_partial_path qom/object.c:2120
     #4 0x55f728f6648a in object_resolve_partial_path qom/object.c:2130
     #5 0x55f728f6648a in object_resolve_partial_path qom/object.c:2130
     #6 0x55f728f668b7 in object_resolve_path_type qom/object.c:2159
     #7 0x55f727ab14c8 in soundhw_init hw/audio/soundhw.c:112
     #8 0x55f72858f27e in qemu_create_cli_devices system/vl.c:2604
     #9 0x55f72858f8c7 in qmp_x_exit_preconfig system/vl.c:2685
     #10 0x55f7285955cd in qemu_init system/vl.c:3734
     #11 0x55f72790620a in main system/main.c:47
     #12 0x7fd5fec57d8f in __libc_start_call_main
     #13 0x7fd5fec57e3f in __libc_start_main_impl libc-start.c:392
     #14 0x55f727906104 in _start (build_asan/qemu-system-ppc+0x2ca4104)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV qom/object.c:935 in 
object_class_dynamic_cast
==3518191==ABORTING

O_o

Any idea what I'm missing?

Unified diff running with '-trace obj\* ':

@@ -1,4 +1,4 @@
-==3534948==WARNING: ASan doesn't fully support makecontext/swapcontext 
functions and may produce false positives in some cases!
+==3533425==WARNING: ASan doesn't fully support makecontext/swapcontext 
functions and may produce false positives in some cases!
  object_class_dynamic_cast_assert authz-pam->user-creatable 
(include/qom/object_interfaces.h:11:USER_CREATABLE_CLASS)
  object_class_dynamic_cast_assert authz-pam->authz 
(include/authz/base.h:30:QAUTHZ_CLASS)
  object_class_dynamic_cast_assert device->device 
(include/hw/qdev-core.h:77:DEVICE_CLASS)
@@ -2481,6 +2481,7 @@
  object_class_dynamic_cast_assert PCI->bus 
(include/hw/qdev-core.h:315:BUS_GET_CLASS)
  object_dynamic_cast_assert PCI->PCI (include/hw/pci/pci.h:270:PCI_BUS)
  object_dynamic_cast_assert PCI->PCI (include/hw/pci/pci.h:270:PCI_BUS)
+object_dynamic_cast_assert memory-region->memory-region 
(include/exec/memory.h:37:MEMORY_REGION)
  object_dynamic_cast_assert i82378->device 
(include/hw/qdev-core.h:77:DEVICE)
  object_dynamic_cast_assert i82378->device 
(include/hw/qdev-core.h:77:DEVICE)
  object_dynamic_cast_assert i82378->device 
(include/hw/qdev-core.h:77:DEVICE)
@@ -3014,355 +3015,28 @@
  object_dynamic_cast_assert memory-region->memory-region 
(include/exec/memory.h:37:MEMORY_REGION)
  object_dynamic_cast_assert memory-region->memory-region 
(include/exec/memory.h:37:MEMORY_REGION)
  object_dynamic_cast_assert memory-region->memory-region 
(include/exec/memory.h:37:MEMORY_REGION)
-object_dynamic_cast_assert fw_cfg_mem->fw_cfg 
(include/hw/nvram/fw_cfg.h:15:FW_CFG)
-object_dynamic_cast_assert 40p-machine->machine 
(include/hw/boards.h:23:MACHINE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert or-irq->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_class_dynamic_cast_assert or-irq->device 
(include/hw/qdev-core.h:77:DEVICE_GET_CLASS)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert mc146818rtc->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_class_dynamic_cast_assert mc146818rtc->device 
(include/hw/qdev-core.h:77:DEVICE_GET_CLASS)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert isa-i8259->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_class_dynamic_cast_assert isa-i8259->device 
(include/hw/qdev-core.h:77:DEVICE_GET_CLASS)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert (null)->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_dynamic_cast_assert rs6000-mc->device 
(include/hw/qdev-core.h:77:DEVICE)
-object_class_dynamic_cast_assert rs6000-mc->device 
(include/hw/qdev-core.h:77:DEVICE_GET_CLASS)
[...]

Thanks,

Phil.

Comments

Peng Liang Oct. 12, 2023, 4:26 p.m. UTC | #1
On 10/12/2023 18:53, Philippe Mathieu-Daudé wrote:
> Hi Martin, Paolo, Markus, Marc-André,
> 
> With the following changes:
> 
> -- >8 --
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 137276bcb9..291495f798 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -245,6 +245,7 @@ static void ibm_40p_init(MachineState *machine)
>      uint32_t kernel_base = 0, initrd_base = 0;
>      long kernel_size = 0, initrd_size = 0;
>      char boot_device;
> +    MemoryRegion rom;
> 
>      if (kvm_enabled()) {
>          error_report("machine %s does not support the KVM accelerator",
> @@ -277,6 +278,9 @@ static void ibm_40p_init(MachineState *machine)
>          exit(1);
>      }
> 
> +    memory_region_init_rom_nomigrate(&rom, OBJECT(machine), "test",
> +                                     4 * KiB, &error_fatal);
> +
>      /* PCI -> ISA bridge */
>      i82378_dev = DEVICE(pci_new(PCI_DEVFN(11, 0), "i82378"));
>      qdev_connect_gpio_out(i82378_dev, 0,
> 
> ---

I think it can be fixed by changing the type of rom to MemoryRegion*, such as:
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 137276bcb9..b5c2345ec8 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -245,6 +245,7 @@ static void ibm_40p_init(MachineState *machine)
     uint32_t kernel_base = 0, initrd_base = 0;
     long kernel_size = 0, initrd_size = 0;
     char boot_device;
+    MemoryRegion *rom = g_new0(MemoryRegion, 1);

     if (kvm_enabled()) {
         error_report("machine %s does not support the KVM accelerator",
@@ -277,6 +278,9 @@ static void ibm_40p_init(MachineState *machine)
         exit(1);
     }

+    memory_region_init_rom_nomigrate(rom, OBJECT(machine), "test", 4 * KiB,
+                                     &error_fatal);
+
     /* PCI -> ISA bridge */
     i82378_dev = DEVICE(pci_new(PCI_DEVFN(11, 0), "i82378"));
     qdev_connect_gpio_out(i82378_dev, 0,
---

In the original patch, rom is an object on stack and machine will save a reference
to rom in its properties after memory_region_init_rom_nomigrate. When the function
returns, the stack frame is freed and the data in rom becomes to garbage. After that,
when we call object_resolve_path_type, the properties of machine will be used to
match the specific path and type, then we will use some garbage in rom (which is on
stack).

Thanks,
Peng

> 
> I'm getting:
> 
> $ ./qemu-system-ppc -M 40p -S -nodefaults
> ==3518191==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> qom/object.c:935:13: runtime error: member access within null pointer of type 'struct TypeImpl'
> AddressSanitizer:DEADLYSIGNAL
> =================================================================
> ==3518191==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x55f728f5e179 bp 0x7ffd91999220 sp 0x7ffd919991d0 T0)
> ==3518191==The signal is caused by a READ memory access.
> ==3518191==Hint: address points to the zero page.
>     #0 0x55f728f5e179 in object_class_dynamic_cast qom/object.c:935
>     #1 0x55f728f5d9f4 in object_dynamic_cast qom/object.c:876
>     #2 0x55f728f6605a in object_resolve_abs_path qom/object.c:2096
>     #3 0x55f728f662f1 in object_resolve_partial_path qom/object.c:2120
>     #4 0x55f728f6648a in object_resolve_partial_path qom/object.c:2130
>     #5 0x55f728f6648a in object_resolve_partial_path qom/object.c:2130
>     #6 0x55f728f668b7 in object_resolve_path_type qom/object.c:2159
>     #7 0x55f727ab14c8 in soundhw_init hw/audio/soundhw.c:112
>     #8 0x55f72858f27e in qemu_create_cli_devices system/vl.c:2604
>     #9 0x55f72858f8c7 in qmp_x_exit_preconfig system/vl.c:2685
>     #10 0x55f7285955cd in qemu_init system/vl.c:3734
>     #11 0x55f72790620a in main system/main.c:47
>     #12 0x7fd5fec57d8f in __libc_start_call_main
>     #13 0x7fd5fec57e3f in __libc_start_main_impl libc-start.c:392
>     #14 0x55f727906104 in _start (build_asan/qemu-system-ppc+0x2ca4104)
> 
> AddressSanitizer can not provide additional info.
> SUMMARY: AddressSanitizer: SEGV qom/object.c:935 in object_class_dynamic_cast
> ==3518191==ABORTING
> 
> O_o
> 
> Any idea what I'm missing?
> 
> Unified diff running with '-trace obj\* ':
> 
> @@ -1,4 +1,4 @@
> -==3534948==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> +==3533425==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>  object_class_dynamic_cast_assert authz-pam->user-creatable (include/qom/object_interfaces.h:11:USER_CREATABLE_CLASS)
>  object_class_dynamic_cast_assert authz-pam->authz (include/authz/base.h:30:QAUTHZ_CLASS)
>  object_class_dynamic_cast_assert device->device (include/hw/qdev-core.h:77:DEVICE_CLASS)
> @@ -2481,6 +2481,7 @@
>  object_class_dynamic_cast_assert PCI->bus (include/hw/qdev-core.h:315:BUS_GET_CLASS)
>  object_dynamic_cast_assert PCI->PCI (include/hw/pci/pci.h:270:PCI_BUS)
>  object_dynamic_cast_assert PCI->PCI (include/hw/pci/pci.h:270:PCI_BUS)
> +object_dynamic_cast_assert memory-region->memory-region (include/exec/memory.h:37:MEMORY_REGION)
>  object_dynamic_cast_assert i82378->device (include/hw/qdev-core.h:77:DEVICE)
>  object_dynamic_cast_assert i82378->device (include/hw/qdev-core.h:77:DEVICE)
>  object_dynamic_cast_assert i82378->device (include/hw/qdev-core.h:77:DEVICE)
> @@ -3014,355 +3015,28 @@
>  object_dynamic_cast_assert memory-region->memory-region (include/exec/memory.h:37:MEMORY_REGION)
>  object_dynamic_cast_assert memory-region->memory-region (include/exec/memory.h:37:MEMORY_REGION)
>  object_dynamic_cast_assert memory-region->memory-region (include/exec/memory.h:37:MEMORY_REGION)
> -object_dynamic_cast_assert fw_cfg_mem->fw_cfg (include/hw/nvram/fw_cfg.h:15:FW_CFG)
> -object_dynamic_cast_assert 40p-machine->machine (include/hw/boards.h:23:MACHINE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert or-irq->device (include/hw/qdev-core.h:77:DEVICE)
> -object_class_dynamic_cast_assert or-irq->device (include/hw/qdev-core.h:77:DEVICE_GET_CLASS)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert mc146818rtc->device (include/hw/qdev-core.h:77:DEVICE)
> -object_class_dynamic_cast_assert mc146818rtc->device (include/hw/qdev-core.h:77:DEVICE_GET_CLASS)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert isa-i8259->device (include/hw/qdev-core.h:77:DEVICE)
> -object_class_dynamic_cast_assert isa-i8259->device (include/hw/qdev-core.h:77:DEVICE_GET_CLASS)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert (null)->device (include/hw/qdev-core.h:77:DEVICE)
> -object_dynamic_cast_assert rs6000-mc->device (include/hw/qdev-core.h:77:DEVICE)
> -object_class_dynamic_cast_assert rs6000-mc->device (include/hw/qdev-core.h:77:DEVICE_GET_CLASS)
> [...]
> 
> Thanks,
> 
> Phil.
>
Philippe Mathieu-Daudé Oct. 12, 2023, 4:48 p.m. UTC | #2
On 12/10/23 18:26, Peng Liang wrote:
> On 10/12/2023 18:53, Philippe Mathieu-Daudé wrote:
>> Hi Martin, Paolo, Markus, Marc-André,
>>
>> With the following changes:
>>
>> -- >8 --
>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>> index 137276bcb9..291495f798 100644
>> --- a/hw/ppc/prep.c
>> +++ b/hw/ppc/prep.c
>> @@ -245,6 +245,7 @@ static void ibm_40p_init(MachineState *machine)
>>       uint32_t kernel_base = 0, initrd_base = 0;
>>       long kernel_size = 0, initrd_size = 0;
>>       char boot_device;
>> +    MemoryRegion rom;
>>
>>       if (kvm_enabled()) {
>>           error_report("machine %s does not support the KVM accelerator",
>> @@ -277,6 +278,9 @@ static void ibm_40p_init(MachineState *machine)
>>           exit(1);
>>       }
>>
>> +    memory_region_init_rom_nomigrate(&rom, OBJECT(machine), "test",
>> +                                     4 * KiB, &error_fatal);
>> +
>>       /* PCI -> ISA bridge */
>>       i82378_dev = DEVICE(pci_new(PCI_DEVFN(11, 0), "i82378"));
>>       qdev_connect_gpio_out(i82378_dev, 0,
>>
>> ---
> 
> I think it can be fixed by changing the type of rom to MemoryRegion*, such as:
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 137276bcb9..b5c2345ec8 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -245,6 +245,7 @@ static void ibm_40p_init(MachineState *machine)
>       uint32_t kernel_base = 0, initrd_base = 0;
>       long kernel_size = 0, initrd_size = 0;
>       char boot_device;
> +    MemoryRegion *rom = g_new0(MemoryRegion, 1);
> 
>       if (kvm_enabled()) {
>           error_report("machine %s does not support the KVM accelerator",
> @@ -277,6 +278,9 @@ static void ibm_40p_init(MachineState *machine)
>           exit(1);
>       }
> 
> +    memory_region_init_rom_nomigrate(rom, OBJECT(machine), "test", 4 * KiB,
> +                                     &error_fatal);
> +
>       /* PCI -> ISA bridge */
>       i82378_dev = DEVICE(pci_new(PCI_DEVFN(11, 0), "i82378"));
>       qdev_connect_gpio_out(i82378_dev, 0,
> ---
> 
> In the original patch, rom is an object on stack and machine will save a reference
> to rom in its properties after memory_region_init_rom_nomigrate. When the function
> returns, the stack frame is freed and the data in rom becomes to garbage. After that,
> when we call object_resolve_path_type, the properties of machine will be used to
> match the specific path and type, then we will use some garbage in rom (which is on
> stack).

YES! Stupid mistake... Thank you Peng, you saved my day :)
diff mbox series

Patch

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 137276bcb9..291495f798 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -245,6 +245,7 @@  static void ibm_40p_init(MachineState *machine)
      uint32_t kernel_base = 0, initrd_base = 0;
      long kernel_size = 0, initrd_size = 0;
      char boot_device;
+    MemoryRegion rom;

      if (kvm_enabled()) {
          error_report("machine %s does not support the KVM accelerator",
@@ -277,6 +278,9 @@  static void ibm_40p_init(MachineState *machine)
          exit(1);
      }

+    memory_region_init_rom_nomigrate(&rom, OBJECT(machine), "test",
+                                     4 * KiB, &error_fatal);
+
      /* PCI -> ISA bridge */
      i82378_dev = DEVICE(pci_new(PCI_DEVFN(11, 0), "i82378"));
      qdev_connect_gpio_out(i82378_dev, 0,