Re: [PATCH v1 1/3] hw/arm/aspeed_ast2700-fc: Fix null pointer dereference in ca35 init

["Cédric Le Goater" <clg@kaod.org>]

On 5/7/25 12:10, Steven Lee wrote:
> Clang's sanitizer reports a runtime error when booting with
> '-net nic -net user', due to a null pointer being passed
> to memory_region_find(), which subsequently triggers a crash in
> flatview_lookup().
> 
> The root cause is that CA35 memory region is not mapped to system
> memory. In addition, unconfigured NICs (due to missing peers)
> lead to a cascade of warnings and possibly misbehavior.
> 
> Fix by:
> - Reduce ca35 ram size to 1GiB to match the ast2700a1-evb.
> - Map ca35_memory into system memory
> - Add nic configuration in ast2700fc's ca35 init function.
> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> Change-Id: Id9c0e6f16861c64a11f6299afb6ef02eb4086041

As said earlier, please try to remove these tags.

> ---
>   hw/arm/aspeed_ast27x0-fc.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index 125a3ade40..ccba5fc8a1 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -48,7 +48,7 @@ struct Ast2700FCState {
>       bool mmio_exec;
>   };
>   
> -#define AST2700FC_BMC_RAM_SIZE (2 * GiB)
> +#define AST2700FC_BMC_RAM_SIZE (1 * GiB)

why ?

>   #define AST2700FC_CM4_DRAM_SIZE (32 * MiB)
>   
>   #define AST2700FC_HW_STRAP1 0x000000C0
> @@ -59,6 +59,7 @@ struct Ast2700FCState {
>   static void ast2700fc_ca35_init(MachineState *machine)
>   {
>       Ast2700FCState *s = AST2700A1FC(machine);
> +    AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
>       AspeedSoCState *soc;
>       AspeedSoCClass *sc;
>   
> @@ -68,6 +69,7 @@ static void ast2700fc_ca35_init(MachineState *machine)
>   
>       memory_region_init(&s->ca35_memory, OBJECT(&s->ca35), "ca35-memory",
>                          UINT64_MAX);
> +    memory_region_add_subregion(get_system_memory(), 0, &s->ca35_memory);

I think this belongs to another patch. Please also modify the fby35
machine which suffers from the same problem regarding the global
system memory usage.


Thanks,

C.


    
>       if (!memory_region_init_ram(&s->ca35_dram, OBJECT(&s->ca35), "ca35-dram",
>                                   AST2700FC_BMC_RAM_SIZE, &error_abort)) {
> @@ -86,6 +88,14 @@ static void ast2700fc_ca35_init(MachineState *machine)
>                                    AST2700FC_BMC_RAM_SIZE, &error_abort)) {
>           return;
>       }
> +
> +    for (int i = 0; i < sc->macs_num; i++) {
> +        if ((amc->macs_mask & (1 << i)) &&
> +            !qemu_configure_nic_device(DEVICE(&soc->ftgmac100[i]),
> +                                       true, NULL)) {
> +            break;
> +        }
> +    }
>       if (!object_property_set_int(OBJECT(&s->ca35), "hw-strap1",
>                                    AST2700FC_HW_STRAP1, &error_abort)) {
>           return;
> @@ -171,6 +181,7 @@ static void ast2700fc_init(MachineState *machine)
>   static void ast2700fc_class_init(ObjectClass *oc, const void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> +    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
>   
>       mc->alias = "ast2700fc";
>       mc->desc = "ast2700 full core support";
> @@ -178,12 +189,13 @@ static void ast2700fc_class_init(ObjectClass *oc, const void *data)
>       mc->no_floppy = 1;
>       mc->no_cdrom = 1;
>       mc->min_cpus = mc->max_cpus = mc->default_cpus = 6;
> +    amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON | ASPEED_MAC2_ON;
>   }
>   
>   static const TypeInfo ast2700fc_types[] = {
>       {
>           .name           = MACHINE_TYPE_NAME("ast2700fc"),
> -        .parent         = TYPE_MACHINE,
> +        .parent         = TYPE_ASPEED_MACHINE,
>           .class_init     = ast2700fc_class_init,
>           .instance_size  = sizeof(Ast2700FCState),
>       },