Commit 649cab56 authored by Frank Rowand's avatar Frank Rowand Committed by Rob Herring

of: properly check for error returned by fdt_get_name()

fdt_get_name() returns error values via a parameter pointer
instead of in function return.  Fix check for this error value
in populate_node() and callers of populate_node().

Chasing up the caller tree showed callers of various functions
failing to initialize the value of pointer parameters that
can return error values.  Initialize those values to NULL.

The bug was introduced by
commit e6a6928c ("of/fdt: Convert FDT functions to use libfdt")
but this patch can not be backported directly to that commit
because the relevant code has further been restructured by
commit dfbd4c6e ("drivers/of: Split unflatten_dt_node()")

The bug became visible by triggering a crash on openrisc with:
commit 79edff12 ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
as reported in:
https://lore.kernel.org/lkml/20210327224116.69309-1-linux@roeck-us.net/

Fixes: 79edff12 ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
Reported-by: default avatarGuenter Roeck <linux@roeck-us.net>
Signed-off-by: default avatarFrank Rowand <frank.rowand@sony.com>
Tested-by: default avatarGuenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20210405032845.1942533-1-frowand.list@gmail.comSigned-off-by: default avatarRob Herring <robh@kernel.org>
parent b5a95bb1
...@@ -205,7 +205,7 @@ static void populate_properties(const void *blob, ...@@ -205,7 +205,7 @@ static void populate_properties(const void *blob,
*pprev = NULL; *pprev = NULL;
} }
static bool populate_node(const void *blob, static int populate_node(const void *blob,
int offset, int offset,
void **mem, void **mem,
struct device_node *dad, struct device_node *dad,
...@@ -214,24 +214,24 @@ static bool populate_node(const void *blob, ...@@ -214,24 +214,24 @@ static bool populate_node(const void *blob,
{ {
struct device_node *np; struct device_node *np;
const char *pathp; const char *pathp;
unsigned int l, allocl; int len;
pathp = fdt_get_name(blob, offset, &l); pathp = fdt_get_name(blob, offset, &len);
if (!pathp) { if (!pathp) {
*pnp = NULL; *pnp = NULL;
return false; return len;
} }
allocl = ++l; len++;
np = unflatten_dt_alloc(mem, sizeof(struct device_node) + allocl, np = unflatten_dt_alloc(mem, sizeof(struct device_node) + len,
__alignof__(struct device_node)); __alignof__(struct device_node));
if (!dryrun) { if (!dryrun) {
char *fn; char *fn;
of_node_init(np); of_node_init(np);
np->full_name = fn = ((char *)np) + sizeof(*np); np->full_name = fn = ((char *)np) + sizeof(*np);
memcpy(fn, pathp, l); memcpy(fn, pathp, len);
if (dad != NULL) { if (dad != NULL) {
np->parent = dad; np->parent = dad;
...@@ -295,6 +295,7 @@ static int unflatten_dt_nodes(const void *blob, ...@@ -295,6 +295,7 @@ static int unflatten_dt_nodes(const void *blob,
struct device_node *nps[FDT_MAX_DEPTH]; struct device_node *nps[FDT_MAX_DEPTH];
void *base = mem; void *base = mem;
bool dryrun = !base; bool dryrun = !base;
int ret;
if (nodepp) if (nodepp)
*nodepp = NULL; *nodepp = NULL;
...@@ -322,9 +323,10 @@ static int unflatten_dt_nodes(const void *blob, ...@@ -322,9 +323,10 @@ static int unflatten_dt_nodes(const void *blob,
!of_fdt_device_is_available(blob, offset)) !of_fdt_device_is_available(blob, offset))
continue; continue;
if (!populate_node(blob, offset, &mem, nps[depth], ret = populate_node(blob, offset, &mem, nps[depth],
&nps[depth+1], dryrun)) &nps[depth+1], dryrun);
return mem - base; if (ret < 0)
return ret;
if (!dryrun && nodepp && !*nodepp) if (!dryrun && nodepp && !*nodepp)
*nodepp = nps[depth+1]; *nodepp = nps[depth+1];
...@@ -372,6 +374,10 @@ void *__unflatten_device_tree(const void *blob, ...@@ -372,6 +374,10 @@ void *__unflatten_device_tree(const void *blob,
{ {
int size; int size;
void *mem; void *mem;
int ret;
if (mynodes)
*mynodes = NULL;
pr_debug(" -> unflatten_device_tree()\n"); pr_debug(" -> unflatten_device_tree()\n");
...@@ -392,7 +398,7 @@ void *__unflatten_device_tree(const void *blob, ...@@ -392,7 +398,7 @@ void *__unflatten_device_tree(const void *blob,
/* First pass, scan for size */ /* First pass, scan for size */
size = unflatten_dt_nodes(blob, NULL, dad, NULL); size = unflatten_dt_nodes(blob, NULL, dad, NULL);
if (size < 0) if (size <= 0)
return NULL; return NULL;
size = ALIGN(size, 4); size = ALIGN(size, 4);
...@@ -410,12 +416,16 @@ void *__unflatten_device_tree(const void *blob, ...@@ -410,12 +416,16 @@ void *__unflatten_device_tree(const void *blob,
pr_debug(" unflattening %p...\n", mem); pr_debug(" unflattening %p...\n", mem);
/* Second pass, do actual unflattening */ /* Second pass, do actual unflattening */
unflatten_dt_nodes(blob, mem, dad, mynodes); ret = unflatten_dt_nodes(blob, mem, dad, mynodes);
if (be32_to_cpup(mem + size) != 0xdeadbeef) if (be32_to_cpup(mem + size) != 0xdeadbeef)
pr_warn("End of tree marker overwritten: %08x\n", pr_warn("End of tree marker overwritten: %08x\n",
be32_to_cpup(mem + size)); be32_to_cpup(mem + size));
if (detached && mynodes) { if (ret <= 0)
return NULL;
if (detached && mynodes && *mynodes) {
of_node_set_flag(*mynodes, OF_DETACHED); of_node_set_flag(*mynodes, OF_DETACHED);
pr_debug("unflattened tree is detached\n"); pr_debug("unflattened tree is detached\n");
} }
......
...@@ -1017,7 +1017,7 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size, ...@@ -1017,7 +1017,7 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
const void *new_fdt; const void *new_fdt;
int ret; int ret;
u32 size; u32 size;
struct device_node *overlay_root; struct device_node *overlay_root = NULL;
*ovcs_id = 0; *ovcs_id = 0;
ret = 0; ret = 0;
......
...@@ -1408,7 +1408,7 @@ static void attach_node_and_children(struct device_node *np) ...@@ -1408,7 +1408,7 @@ static void attach_node_and_children(struct device_node *np)
static int __init unittest_data_add(void) static int __init unittest_data_add(void)
{ {
void *unittest_data; 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 * __dtb_testcases_begin[] and __dtb_testcases_end[] are magically
* created by cmd_dt_S_dtb in scripts/Makefile.lib * created by cmd_dt_S_dtb in scripts/Makefile.lib
...@@ -1417,10 +1417,10 @@ static int __init unittest_data_add(void) ...@@ -1417,10 +1417,10 @@ static int __init unittest_data_add(void)
extern uint8_t __dtb_testcases_end[]; extern uint8_t __dtb_testcases_end[];
const int size = __dtb_testcases_end - __dtb_testcases_begin; const int size = __dtb_testcases_end - __dtb_testcases_begin;
int rc; int rc;
void *ret;
if (!size) { if (!size) {
pr_warn("%s: No testcase data to attach; not running tests\n", pr_warn("%s: testcases is empty\n", __func__);
__func__);
return -ENODATA; return -ENODATA;
} }
...@@ -1429,9 +1429,14 @@ static int __init unittest_data_add(void) ...@@ -1429,9 +1429,14 @@ static int __init unittest_data_add(void)
if (!unittest_data) if (!unittest_data)
return -ENOMEM; return -ENOMEM;
of_fdt_unflatten_tree(unittest_data, NULL, &unittest_data_node); ret = of_fdt_unflatten_tree(unittest_data, NULL, &unittest_data_node);
if (!ret) {
pr_warn("%s: unflatten testcases tree failed\n", __func__);
kfree(unittest_data);
return -ENODATA;
}
if (!unittest_data_node) { if (!unittest_data_node) {
pr_warn("%s: No tree to attach; not running tests\n", __func__); pr_warn("%s: testcases tree is empty\n", __func__);
kfree(unittest_data); kfree(unittest_data);
return -ENODATA; return -ENODATA;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment