Re: [Xen-devel] [PATCH v2 1/3] xen/arm: fix buf size in make_cpus_node

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 10/8/19 2:14 AM, Stefano Stabellini wrote:
> The size of buf is calculated wrongly: the number is 64bit, not 32bit.
While the variable mpdir_aff is 64-bit, we only write the first 32-bit 
in the property reg (#address-cells == 1 and fdt_property_cell()). So 
what needs to be modified is the format here.
Also, looking the CPU bindings (see 
linux/Documentation/devicetree/bindings/arm/cpus.yaml), technically only 
the bits [23:0] of the mpidr should be used. The rest is zeroed.
This is ok because vcpuid_to_vaffinity() is always returning a value 
following the requirements above. However, for correctness, this may 
want to be fixed.
> Also the number is printed as a hexadecimal number, so we need 8 bytes
> for 32bit, not 10 bytes.
>
> As a result, it should be sizeof("cpu@") + 16 bytes for a 64-bit number
> + 1 byte for \0. Total = 21.
>
> Fixes: fafd682c3e (xen/arm: Create a fake cpus node in dom0 device tree)
I am afraid this is not fixing this patch:

snprintf(buf, sizeof(buf), "cpu@%u", cpu);

So the 10 bytes were actually correct back then.

The problem was introduced by commit c81a791d34 "xen/arm: Set 'reg' of 
cpu node for dom0 to match MPIDR's affinity".
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> ---
> Changes in v2:
> - patch added
> ---
>   xen/arch/arm/domain_build.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 921b054520..60923a7051 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -788,8 +788,8 @@ static int __init make_cpus_node(const struct domain *d, 
> void *fdt)
>       unsigned int cpu;
>       const void *compatible = NULL;
>       u32 len;
> -    /* Placeholder for cpu@ + a 32-bit number + \0 */
> -    char buf[15];
> +    /* Placeholder for cpu@ + a 64-bit number + \0 */
> +    char buf[21];
>       u32 clock_frequency;
>       bool clock_valid;
>       uint64_t mpidr_aff;
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel