Re: [Xen-devel] [PATCH v6 05/26] xen/arm: check for multiboot nodes only under /chosen

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 9 Nov 2018, Julien Grall wrote:
> Hi,
> 
> On 02/11/2018 23:44, Stefano Stabellini wrote:
> > Make sure to only look for multiboot compatible nodes only under
> > /chosen, not under any other paths (depth <= 3).
> > 
> > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > 
> > ---
> > 
> > Changes in v6:
> > - do not proceed if fdt_get_path returns error != -FDT_ERR_NOSPACE
> > - remove sizeof, use hardcoded value
> > 
> > Changes in v5:
> > - add patch
> > - add check on return value of fdt_get_path
> > ---
> >   xen/arch/arm/bootfdt.c | 14 +++++++++++---
> >   1 file changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> > index 8eba42c..a42fe87 100644
> > --- a/xen/arch/arm/bootfdt.c
> > +++ b/xen/arch/arm/bootfdt.c
> > @@ -173,7 +173,15 @@ static void __init process_multiboot_node(const void
> > *fdt, int node,
> >       bootmodule_kind kind;
> >       paddr_t start, size;
> >       const char *cmdline;
> > -    int len;
> > +    int len = 8; /* sizeof "/chosen" */
> > +    char path[8];
> > +    int ret;
> > +
> > +    /* Check that the node is under "chosen" */
> > +    ret = fdt_get_path(fdt, node, path, len);
> > +    if ( (ret != 0 && ret != -FDT_ERR_NOSPACE) ||
> 
> I don't understand why you specifically check for -FDT_ERR_NOSPACE here.
> 
> Looking at fdt_get_path(...) there are case where the function may return
> -FDT_ERR_NOSPACE yet not filling up path. So you would end up to compare
> garbage.

I am explicitely checking for -FDT_ERR_NOSPACE because it is a valid
condition in this case: -FDT_ERR_NOSPACE is returned where `path' is not
big enough to contain the full device tree path. It is OK and expected,
given that path is only 8 chars long. So, in case of -FDT_ERR_NOSPACE,
we should continue and do the comparison with "/chosen". For other
errors we should return.


> > +         strncmp(path, "/chosen", len - 1) )
> > +        return;
> >         prop = fdt_get_property(fdt, node, "reg", &len);
> >       if ( !prop )
> > @@ -286,8 +294,8 @@ static int __init early_scan_node(const void *fdt,
> >   {
> >       if ( device_tree_node_matches(fdt, node, "memory") )
> >           process_memory_node(fdt, node, name, address_cells, size_cells);
> > -    else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module"
> > ) ||
> > -              device_tree_node_compatible(fdt, node, "multiboot,module" ))
> > +    else if ( depth <= 3 && (device_tree_node_compatible(fdt, node,
> > "xen,multiboot-module" ) ||
> > +              device_tree_node_compatible(fdt, node, "multiboot,module" )))
> >           process_multiboot_node(fdt, node, name, address_cells,
> > size_cells);
> >       else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen")
> > )
> >           process_chosen_node(fdt, node, name, address_cells, size_cells);
> > 

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