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

2023-08-22 Thread Julien Grall

Hi Vikram,

On 19/08/2023 01:28, 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 

---
Changes from v7:
 Free allocated memory in case of errors when calling unflatten_dt_node.
---
---
  xen/common/device_tree.c | 17 +++--
  1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index c91d54c493..cd9a6a5c93 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, , NULL, NULL, 0);
+if ( !size )
+return -EINVAL;
+
  size = (size | 3) + 1;
  
  dt_dprintk("  size is %#lx allocating...\n", size);

@@ -2125,11 +2128,21 @@ static int __init __unflatten_device_tree(const void 
*fdt,
  start = ((unsigned long)fdt) + fdt_off_dt_struct(fdt);
  unflatten_dt_node(fdt, mem, , NULL, , 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));
+xfree((__be64 *)mem);


I know that 'mem' is an 'unsigned long' so you need to cast it to a 
pointer. But '__be64 *' seems a rather odd choice. Why not using 'void *'?


Better would be to convert 'mem' to a 'void *' as we allow pointer 
arithmetic in Xen. But that can be dealt separately.



+return -EINVAL;
+}
+
  if ( be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeefU )
-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]));
+xfree((__be64 *)mem);


Same remark here.


+return -EINVAL;
+}
+
  *allnextp = NULL;
  
  dt_dprintk(" <- unflatten_device_tree()\n");


Cheers,

--
Julien Grall



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

2023-08-18 Thread Vikram Garhwal
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 

---
Changes from v7:
Free allocated memory in case of errors when calling unflatten_dt_node.
---
---
 xen/common/device_tree.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index c91d54c493..cd9a6a5c93 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, , NULL, NULL, 0);
+if ( !size )
+return -EINVAL;
+
 size = (size | 3) + 1;
 
 dt_dprintk("  size is %#lx allocating...\n", size);
@@ -2125,11 +2128,21 @@ static int __init __unflatten_device_tree(const void 
*fdt,
 start = ((unsigned long)fdt) + fdt_off_dt_struct(fdt);
 unflatten_dt_node(fdt, mem, , NULL, , 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));
+xfree((__be64 *)mem);
+return -EINVAL;
+}
+
 if ( be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeefU )
-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]));
+xfree((__be64 *)mem);
+return -EINVAL;
+}
+
 *allnextp = NULL;
 
 dt_dprintk(" <- unflatten_device_tree()\n");
-- 
2.17.1