[PATCH v4 1/6] hw/arm/aspeed_ast2700-fc: Fix null pointer dereference in ca35 init

Steven Lee via posted 6 patches 5 months, 3 weeks ago
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>
There is a newer version of this series
[PATCH v4 1/6] hw/arm/aspeed_ast2700-fc: Fix null pointer dereference in ca35 init
Posted by Steven Lee via 5 months, 3 weeks ago
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().

Root cause:
- Missing NIC configuration in the CA35 initialization.

Fix:
- Add nic configuration in ast2700fc's ca35 init function.

Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
---
 hw/arm/aspeed_ast27x0-fc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
index 125a3ade40..7bf4f2a52d 100644
--- a/hw/arm/aspeed_ast27x0-fc.c
+++ b/hw/arm/aspeed_ast27x0-fc.c
@@ -86,6 +86,13 @@ static void ast2700fc_ca35_init(MachineState *machine)
                                  AST2700FC_BMC_RAM_SIZE, &error_abort)) {
         return;
     }
+
+    for (int i = 0; i < sc->macs_num; i++) {
+        if (!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;
-- 
2.43.0
Re: [PATCH v4 1/6] hw/arm/aspeed_ast2700-fc: Fix null pointer dereference in ca35 init
Posted by Cédric Le Goater 5 months, 3 weeks ago
On 5/22/25 11:16, 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().

I don't see such errors.

> Root cause:
> - Missing NIC configuration in the CA35 initialization.
> 
> Fix:
> - Add nic configuration in ast2700fc's ca35 init function.

However it would be nice to have network support.

Could you please rephrase the commit log ?


Thanks,

C.



> 
> Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast27x0-fc.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> index 125a3ade40..7bf4f2a52d 100644
> --- a/hw/arm/aspeed_ast27x0-fc.c
> +++ b/hw/arm/aspeed_ast27x0-fc.c
> @@ -86,6 +86,13 @@ static void ast2700fc_ca35_init(MachineState *machine)
>                                    AST2700FC_BMC_RAM_SIZE, &error_abort)) {
>           return;
>       }
> +
> +    for (int i = 0; i < sc->macs_num; i++) {
> +        if (!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;
RE: [PATCH v4 1/6] hw/arm/aspeed_ast2700-fc: Fix null pointer dereference in ca35 init
Posted by Steven Lee 5 months, 3 weeks ago
Hi Cédric,

> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Friday, May 23, 2025 4:09 PM
> To: Steven Lee <steven_lee@aspeedtech.com>; Peter Maydell
> <peter.maydell@linaro.org>; Troy Lee <leetroy@gmail.com>; Jamin Lin
> <jamin_lin@aspeedtech.com>; Andrew Jeffery
> <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open
> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Troy Lee <troy_lee@aspeedtech.com>; longzl2@lenovo.com; Yunlin Tang
> <yunlin.tang@aspeedtech.com>
> Subject: Re: [PATCH v4 1/6] hw/arm/aspeed_ast2700-fc: Fix null pointer
> dereference in ca35 init
> 
> On 5/22/25 11:16, 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().
> 
> I don't see such errors.
> 
> > Root cause:
> > - Missing NIC configuration in the CA35 initialization.
> >
> > Fix:
> > - Add nic configuration in ast2700fc's ca35 init function.
> 
> However it would be nice to have network support.
> 
> Could you please rephrase the commit log ?
> 

Will rewrite commit log to "Add network support for ast2700fc"

Regards,
Steven

> >
> > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com>
> > ---
> >   hw/arm/aspeed_ast27x0-fc.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/hw/arm/aspeed_ast27x0-fc.c b/hw/arm/aspeed_ast27x0-fc.c
> > index 125a3ade40..7bf4f2a52d 100644
> > --- a/hw/arm/aspeed_ast27x0-fc.c
> > +++ b/hw/arm/aspeed_ast27x0-fc.c
> > @@ -86,6 +86,13 @@ static void ast2700fc_ca35_init(MachineState
> *machine)
> >                                    AST2700FC_BMC_RAM_SIZE,
> &error_abort)) {
> >           return;
> >       }
> > +
> > +    for (int i = 0; i < sc->macs_num; i++) {
> > +        if (!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;