linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>,
	Pantelis Antoniou <pantelis.antoniou@konsulko.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Guenter Roeck <linux@roeck-us.net>
Subject: [PATCH] of/fdt: Improve error checking
Date: Sat, 27 Mar 2021 15:41:16 -0700	[thread overview]
Message-ID: <20210327224116.69309-1-linux@roeck-us.net> (raw)

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


             reply	other threads:[~2021-03-27 22:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-27 22:41 Guenter Roeck [this message]
2021-03-29 18:21 ` [PATCH] of/fdt: Improve error checking Rob Herring
2021-03-29 19:12   ` Frank Rowand
2021-03-29 19:33   ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210327224116.69309-1-linux@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=devicetree@vger.kernel.org \
    --cc=frowand.list@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pantelis.antoniou@konsulko.com \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).