Memory node probing is done as part of early_scan_node() that is called
for each node with depth >= 1 (root node is at depth 0). According to
Devicetree Specification v0.4, chapter 3.4, /memory node can only exists
as a top level node. However, Xen incorrectly considers all the nodes with
unit node name "memory" as RAM. This buggy behavior can result in a
failure if there are other nodes in the device tree (at depth >= 2) with
"memory" as unit node name. An example can be a "memory@xxx" node under
/reserved-memory. Fix it by introducing device_tree_is_memory_node() to
perform all the required checks to assess if a node is a proper /memory
node.
Fixes: 3e99c95ba1c8 ("arm, device tree: parse the DTB for RAM location and size")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>
---
Changes in v2:
- improve the length check
---
xen/arch/arm/bootfdt.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 6e060111d95b..f2892fce0a9c 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -83,6 +83,32 @@ static bool __init device_tree_node_compatible(const void *fdt, int node,
return false;
}
+/*
+ * Check if a node is a proper /memory node according to Devicetree
+ * Specification v0.4, chapter 3.4.
+ */
+static bool __init device_tree_is_memory_node(const void *fdt, int node,
+ int depth)
+{
+ const char *type;
+ int len;
+
+ if ( depth != 1 )
+ return false;
+
+ if ( !device_tree_node_matches(fdt, node, "memory") )
+ return false;
+
+ type = fdt_getprop(fdt, node, "device_type", &len);
+ if ( !type )
+ return false;
+
+ if ( (len <= strlen("memory")) || strcmp(type, "memory") )
+ return false;
+
+ return true;
+}
+
void __init device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
uint32_t size_cells, paddr_t *start,
paddr_t *size)
@@ -448,7 +474,7 @@ static int __init early_scan_node(const void *fdt,
* populated. So we should skip the parsing.
*/
if ( !efi_enabled(EFI_BOOT) &&
- device_tree_node_matches(fdt, node, "memory") )
+ device_tree_is_memory_node(fdt, node, depth) )
rc = process_memory_node(fdt, node, name, depth,
address_cells, size_cells, bootinfo_get_mem());
else if ( depth == 1 && !dt_node_cmp(name, "reserved-memory") )
--
2.25.1
Hi, On 10/07/2024 12:22, Michal Orzel wrote: > Memory node probing is done as part of early_scan_node() that is called > for each node with depth >= 1 (root node is at depth 0). According to > Devicetree Specification v0.4, chapter 3.4, /memory node can only exists > as a top level node. However, Xen incorrectly considers all the nodes with > unit node name "memory" as RAM. This buggy behavior can result in a > failure if there are other nodes in the device tree (at depth >= 2) with > "memory" as unit node name. An example can be a "memory@xxx" node under > /reserved-memory. Fix it by introducing device_tree_is_memory_node() to > perform all the required checks to assess if a node is a proper /memory > node. > > Fixes: 3e99c95ba1c8 ("arm, device tree: parse the DTB for RAM location and size") > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> > Tested-by: Luca Fancellu <luca.fancellu@arm.com> Reviewed-by: Julien Grall <julien@xen.org> Cheers, -- Julien Grall
Hi, On 10/07/2024 22:11, Julien Grall wrote: > Hi, > > On 10/07/2024 12:22, Michal Orzel wrote: >> Memory node probing is done as part of early_scan_node() that is called >> for each node with depth >= 1 (root node is at depth 0). According to >> Devicetree Specification v0.4, chapter 3.4, /memory node can only exists >> as a top level node. However, Xen incorrectly considers all the nodes >> with >> unit node name "memory" as RAM. This buggy behavior can result in a >> failure if there are other nodes in the device tree (at depth >= 2) with >> "memory" as unit node name. An example can be a "memory@xxx" node under >> /reserved-memory. Fix it by introducing device_tree_is_memory_node() to >> perform all the required checks to assess if a node is a proper /memory >> node. >> >> Fixes: 3e99c95ba1c8 ("arm, device tree: parse the DTB for RAM location >> and size") >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> >> Tested-by: Luca Fancellu <luca.fancellu@arm.com> > > Reviewed-by: Julien Grall <julien@xen.org> This is now committed. Cheers, -- Julien Grall
© 2016 - 2024 Red Hat, Inc.