Re: [XEN][PATCH v7 02/19] common/device_tree.c: unflatten_device_tree() propagate errors

[Michal Orzel <michal.orzel@xxxxxxx>]

Title: s/unflatten_device_tree/__unflatten_device_tree/ or you mean to propagate
errors from unflatten_dt_node?

On 02/06/2023 02:48, Vikram Garhwal wrote:
> This will be useful in dynamic node programming when new dt nodes are 
> unflattend
> during runtime. Invalid device tree node related errors should be propagated
> back to the caller.
> 
> Signed-off-by: Vikram Garhwal <vikram.garhwal@xxxxxxx>
> Reviewed-by: Henry Wang <Henry.Wang@xxxxxxx>
> ---
>  xen/common/device_tree.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index dfdb10e488..117ccccb09 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -2108,6 +2108,9 @@ static int __init __unflatten_device_tree(const void 
> *fdt,
>      /* First pass, scan for size */
>      start = ((unsigned long)fdt) + fdt_off_dt_struct(fdt);
>      size = unflatten_dt_node(fdt, 0, &start, NULL, NULL, 0);
> +    if ( !size )
> +        return -EINVAL;
> +
>      size = (size | 3) + 1;
>  
>      dt_dprintk("  size is %#lx allocating...\n", size);
> @@ -2125,11 +2128,19 @@ static int __init __unflatten_device_tree(const void 
> *fdt,
>      start = ((unsigned long)fdt) + fdt_off_dt_struct(fdt);
>      unflatten_dt_node(fdt, mem, &start, NULL, &allnextp, 0);
>      if ( be32_to_cpup((__be32 *)start) != FDT_END )
> -        printk(XENLOG_WARNING "Weird tag at end of tree: %08x\n",
> +    {
> +        printk(XENLOG_ERR "Weird tag at end of tree: %08x\n",
>                    *((u32 *)start));
> +        return -EINVAL;
What about memory that we allocated? Shouldn't it be freed in case of these two 
errors?
For now it is called from boot so we do panic but later in this series this 
could result
in a memory leak. Am I correct?

> +    }
> +
>      if ( be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeef )
> -        printk(XENLOG_WARNING "End of tree marker overwritten: %08x\n",
> +    {
> +        printk(XENLOG_ERR "End of tree marker overwritten: %08x\n",
>                    be32_to_cpu(((__be32 *)mem)[size / 4]));
> +        return -EINVAL;
> +    }
> +
>      *allnextp = NULL;
>  
>      dt_dprintk(" <- unflatten_device_tree()\n");

~Michal