Re: [Qemu-arm] [PATCH v9 1/2] hw/arm: Add arm SBSA reference machine, skeleton part

[Peter Maydell]

On Sun, 30 Jun 2019 at 11:21, Hongbo Zhang <address@hidden> wrote:
>
> For the Aarch64, there is one machine 'virt', it is primarily meant to
> run on KVM and execute virtualization workloads, but we need an
> environment as faithful as possible to physical hardware, for supporting
> firmware and OS development for pysical Aarch64 machines.
>
> This patch introduces new machine type 'sbsa-ref' with main features:
>  - Based on 'virt' machine type.
>  - A new memory map.
>  - CPU type cortex-a57.
>  - EL2 and EL3 are enabled.
>  - GIC version 3.
>  - System bus AHCI controller.
>  - System bus EHCI controller.
>  - CDROM and hard disc on AHCI bus.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - No virtio deivces.
>  - No fw_cfg device.
>  - No ACPI table supplied.
>  - Only minimal device tree nodes.
>
> Arm Trusted Firmware and UEFI porting to this are done accordingly, and
> it should supply ACPI tables to load OS, the minimal device tree nodes
> supplied from this platform are only to pass the dynamic info reflecting
> command line input to firmware, not for loading OS.
>
> To make the review easier, this task is split into two patches, the
> fundamental sceleton part and the peripheral devices part, this patch is
> the first part.
>
> Signed-off-by: Hongbo Zhang <address@hidden>

> +static const TypeInfo sbsa_ref_info = {
> +    .name          = TYPE_SBSA_MACHINE,
> +    .parent        = TYPE_MACHINE,
> +    .class_init    = sbsa_ref_class_init,

You need here to also have

    .instance_size = sizeof(SBSAMachineState),

> +};

Otherwise we won't allocate enough memory for the size of
the struct, and will run off the end of the allocation.
You can see this if you do a build with just patch 1 and
try 'make check' on it with a clang-sanitizer build
(ie passing configure  '--cc=clang-7' '--cxx=clang++-7'
'--extra-cflags=-fsanitize=undefined -fno-sanitize=shift-base -Werror')
as it will catch reads off the end of the memory.

thanks
-- PMM