| Message ID | 20230602004824.20731-3-vikram.garhwal@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dynamic node programming using overlay dtbo | expand |
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@amd.com> > Reviewed-by: Henry Wang <Henry.Wang@arm.com> > --- > 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
Hi Michal, On 6/2/23 12:14 AM, Michal Orzel wrote: > 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@amd.com> >> Reviewed-by: Henry Wang <Henry.Wang@arm.com> >> --- >> 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? Yeah, that's correct. Let me add the memory free handling in v8. >> + } >> + >> 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
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; + } + 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");