[PATCH 40/55] hw/arm/armsse: Pass correct child size to sysbus_init_child_obj()

Markus Armbruster posted 55 patches 5 years, 5 months ago
There is a newer version of this series
[PATCH 40/55] hw/arm/armsse: Pass correct child size to sysbus_init_child_obj()
Posted by Markus Armbruster 5 years, 5 months ago
armsse_init() initializes s->armv7m[i] for all i.  It passes the size
of the entire array instead of the array element to
sysbus_init_child_obj().  Harmless, but fix it anyway.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/armsse.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 20bedbe044..b6276b7327 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -258,7 +258,8 @@ static void armsse_init(Object *obj)
 
         name = g_strdup_printf("armv7m%d", i);
         sysbus_init_child_obj(OBJECT(&s->cluster[i]), name,
-                              &s->armv7m[i], sizeof(s->armv7m), TYPE_ARMV7M);
+                              &s->armv7m[i], sizeof(s->armv7m[i]),
+                              TYPE_ARMV7M);
         qdev_prop_set_string(DEVICE(&s->armv7m[i]), "cpu-type",
                              ARM_CPU_TYPE_NAME("cortex-m33"));
         g_free(name);
-- 
2.21.1


Re: [PATCH 40/55] hw/arm/armsse: Pass correct child size to sysbus_init_child_obj()
Posted by Philippe Mathieu-Daudé 5 years, 5 months ago
On 5/19/20 4:55 PM, Markus Armbruster wrote:
> armsse_init() initializes s->armv7m[i] for all i.  It passes the size
> of the entire array instead of the array element to
> sysbus_init_child_obj().  Harmless, but fix it anyway.

Harmless because the size used to initialize the object is the one 
declared by its TypeInfo::instance_size. In this case for TYPE_ARMV7M it is:

static const TypeInfo armv7m_info = {
     .name = TYPE_ARMV7M,
     .parent = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(ARMv7MState),

How did you notice btw?

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   hw/arm/armsse.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
> index 20bedbe044..b6276b7327 100644
> --- a/hw/arm/armsse.c
> +++ b/hw/arm/armsse.c
> @@ -258,7 +258,8 @@ static void armsse_init(Object *obj)
>   
>           name = g_strdup_printf("armv7m%d", i);
>           sysbus_init_child_obj(OBJECT(&s->cluster[i]), name,
> -                              &s->armv7m[i], sizeof(s->armv7m), TYPE_ARMV7M);
> +                              &s->armv7m[i], sizeof(s->armv7m[i]),
> +                              TYPE_ARMV7M);
>           qdev_prop_set_string(DEVICE(&s->armv7m[i]), "cpu-type",
>                                ARM_CPU_TYPE_NAME("cortex-m33"));
>           g_free(name);
> 


Re: [PATCH 40/55] hw/arm/armsse: Pass correct child size to sysbus_init_child_obj()
Posted by Markus Armbruster 5 years, 5 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 5/19/20 4:55 PM, Markus Armbruster wrote:
>> armsse_init() initializes s->armv7m[i] for all i.  It passes the size
>> of the entire array instead of the array element to
>> sysbus_init_child_obj().  Harmless, but fix it anyway.
>
> Harmless because the size used to initialize the object is the one
> declared by its TypeInfo::instance_size. In this case for TYPE_ARMV7M
> it is:
>
> static const TypeInfo armv7m_info = {
>     .name = TYPE_ARMV7M,
>     .parent = TYPE_SYS_BUS_DEVICE,
>     .instance_size = sizeof(ARMv7MState),

Yes.  object_initialize_with_type() checks @size is at least
.instance_size, and writes only up to .instance_size.

> How did you notice btw?

Transform the common, obviously sane patterns with Coccinelle, examine
the untransformed remainder.  I found quite a few the bugs this way.

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!