powerpc/prom: Avoid reference to potentially freed memory

of_get_property() is used inside the loop, but then the reference to the
node is dropped before dereferencing the prop pointer, which could by then
point to junk if the node has been freed.

Instead use of_property_read_u32() to actually read the property
value before dropping the reference.

of_property_read_u32() requires at least one cell (u32) to be present,
which is stricter than the old logic which would happily dereference a
property of any size. However we believe all device trees in the wild
have at least one cell.

Skiboot may produce memory nodes with more than one cell, but that is
OK, of_property_read_u32() will return the first one.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
[mpe: Expand change log with device tree details]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index bef76c5033e4a9073a8a1a006ec6576f260e5d3d..0b0a4166d69dbbb4fb190535a8f6298700d440ed 100644 (file)
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -783,14 +783,17 @@ void __init early_get_first_memblock_info(void *params, phys_addr_t *size)
 int of_get_ibm_chip_id(struct device_node *np)
 {
        of_node_get(np);
-       while(np) {
+       while (np) {
                struct device_node *old = np;
-               const __be32 *prop;
+               u32 chip_id;
 
-               prop = of_get_property(np, "ibm,chip-id", NULL);
-               if (prop) {
+               /*
+                * Skiboot may produce memory nodes that contain more than one
+                * cell in chip-id, we only read the first one here.
+                */
+               if (!of_property_read_u32(np, "ibm,chip-id", &chip_id)) {
                        of_node_put(np);
-                       return be32_to_cpup(prop);
+                       return chip_id;
                }
                np = of_get_parent(np);
                of_node_put(old);