linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] of/fdt: Improve error checking
@ 2021-03-27 22:41 Guenter Roeck
  2021-03-29 18:21 ` Rob Herring
  0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2021-03-27 22:41 UTC (permalink / raw)
  To: Rob Herring
  Cc: Frank Rowand, Pantelis Antoniou, devicetree, linux-kernel, Guenter Roeck

If an unaligned pointer is passed to of_fdt_unflatten_tree(),
populate_node() as called from unflatten_dt_nodes() will fail.
unflatten_dt_nodes() will return 0 and set *nodepp to NULL.
This is not expected to happen in __unflatten_device_tree(),
which then tries to write into the NULL pointer, causing a crash
on openrisc if CONFIG_OF_UNITTEST=y.

 ### dt-test ### start of unittest - you will see error messages
Unable to handle kernel NULL pointer dereference
 at virtual address 0x00000064

Oops#: 0000
CPU #: 0
   PC: c03a25d4    SR: 0000807f    SP: c102dd50
GPR00: 00000000 GPR01: c102dd50 GPR02: c102dd78 GPR03: c1704004
GPR04: 00000000 GPR05: c102dc18 GPR06: c102ddc8 GPR07: c102dcf7
GPR08: 00000001 GPR09: c03a25a0 GPR10: c102c000 GPR11: c16fd75c
GPR12: 0000ffb7 GPR13: 00000000 GPR14: 00000008 GPR15: 00000000
GPR16: c16fd75c GPR17: 00000064 GPR18: c1704004 GPR19: 00000004
GPR20: 00000000 GPR21: 00000000 GPR22: c102ddc8 GPR23: 00000018
GPR24: 00000001 GPR25: 00000010 GPR26: c16fd75c GPR27: 00000008
GPR28: deadbeef GPR29: 00000000 GPR30: c0720128 GPR31: 00060000
  RES: c16fd75c oGPR11: ffffffff
Process swapper (pid: 1, stackpage=c1028ba0)

Stack:
Call trace:
[<(ptrval)>] __unflatten_device_tree+0xe0/0x184
[<(ptrval)>] of_fdt_unflatten_tree+0x60/0x90
[<(ptrval)>] of_unittest+0xb4/0x3614
[<(ptrval)>] ? __kernfs_create_file+0x130/0x188
[<(ptrval)>] ? sysfs_add_file_mode_ns+0x13c/0x288
[<(ptrval)>] ? of_unittest+0x0/0x3614
[<(ptrval)>] ? lock_is_held_type+0x160/0x20c
[<(ptrval)>] ? of_unittest+0x0/0x3614
[<(ptrval)>] ? ignore_unknown_bootoption+0x0/0x24
[<(ptrval)>] do_one_initcall+0x98/0x340
[<(ptrval)>] ? parse_args+0x220/0x4e4
[<(ptrval)>] ? lock_is_held_type+0x160/0x20c
[<(ptrval)>] ? ignore_unknown_bootoption+0x0/0x24
[<(ptrval)>] ? rcu_read_lock_sched_held+0x34/0x88
[<(ptrval)>] kernel_init_freeable+0x1c0/0x240
[<(ptrval)>] ? ignore_unknown_bootoption+0x0/0x24
[<(ptrval)>] ? kernel_init+0x0/0x154
[<(ptrval)>] kernel_init+0x1c/0x154
[<(ptrval)>] ? calculate_sigpending+0x54/0x64
[<(ptrval)>] ret_from_fork+0x1c/0x150

This problem affects all architectures with a 4-byte memory alignment.
Since commit 79edff12060f ("scripts/dtc: Update to upstream version
v1.6.0-51-g183df9e9c2b9"), devicetree code in the Linux kernel mandates
an 8-byte memory alignment of devicetree pointers, but it does not take
into account that functions such as kmalloc() or kmemdup() may not return
accordingly aligned pointers.

To fix the immediate crash, check if *mynodes is NULL in
__unflatten_device_tree() before writing into it.

Also affected by this problem is the code calling of_fdt_unflatten_tree().
That code checks for errors using the content of the mynodes pointer,
which is not set by the devicetree code if the alignment problem is
observed. Result is that the callers of of_fdt_unflatten_tree() check
if an uninitialized pointer is set to NULL. Preinitialize that pointer
to avoid the problem.

With this code in place, devicetree code on openrisc (and any other
architecture with 4-byte memory alignment) will still not work properly,
but at least it won't crash anymore. The resulting log message is

 ### dt-test ### start of unittest - you will see error messages
 ### dt-test ### unittest_data_add: No tree to attach; not running tests

when trying to run devicetree unittests.

Fixes: 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/of/fdt.c      | 2 +-
 drivers/of/overlay.c  | 2 +-
 drivers/of/unittest.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index dcc1dd96911a..ab95fdb4461d 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -415,7 +415,7 @@ void *__unflatten_device_tree(const void *blob,
 		pr_warn("End of tree marker overwritten: %08x\n",
 			be32_to_cpup(mem + size));
 
-	if (detached && mynodes) {
+	if (detached && mynodes && *mynodes) {
 		of_node_set_flag(*mynodes, OF_DETACHED);
 		pr_debug("unflattened tree is detached\n");
 	}
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 50bbe0edf538..e12c643b6ba8 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -1017,7 +1017,7 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
 	const void *new_fdt;
 	int ret;
 	u32 size;
-	struct device_node *overlay_root;
+	struct device_node *overlay_root = NULL;
 
 	*ovcs_id = 0;
 	ret = 0;
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index eb100627c186..5dc4d2378bfd 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -1408,7 +1408,7 @@ static void attach_node_and_children(struct device_node *np)
 static int __init unittest_data_add(void)
 {
 	void *unittest_data;
-	struct device_node *unittest_data_node, *np;
+	struct device_node *unittest_data_node = NULL, *np;
 	/*
 	 * __dtb_testcases_begin[] and __dtb_testcases_end[] are magically
 	 * created by cmd_dt_S_dtb in scripts/Makefile.lib
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-29 19:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-27 22:41 [PATCH] of/fdt: Improve error checking Guenter Roeck
2021-03-29 18:21 ` Rob Herring
2021-03-29 19:12   ` Frank Rowand
2021-03-29 19:33   ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).