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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 8 Oct 2019, Julien Grall wrote:
> 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.

It looks like it would be best to change mpdir_aff to uint32_t and
change vcpuid_to_vaffinity to return a uint32_t.

Then of course the buf allocation would be buf[13].

Is that what you have in mind?


> > 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".

Yes, I'll change it


> > 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;

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