diff options
Diffstat (limited to 'drivers/of')
| -rw-r--r-- | drivers/of/Kconfig | 14 | ||||
| -rw-r--r-- | drivers/of/base.c | 3 | ||||
| -rw-r--r-- | drivers/of/dynamic.c | 31 | ||||
| -rw-r--r-- | drivers/of/irq.c | 12 | ||||
| -rw-r--r-- | drivers/of/kobj.c | 2 | ||||
| -rw-r--r-- | drivers/of/of_reserved_mem.c | 10 | ||||
| -rw-r--r-- | drivers/of/overlay.c | 2 | ||||
| -rw-r--r-- | drivers/of/platform.c | 7 | ||||
| -rw-r--r-- | drivers/of/property.c | 4 | ||||
| -rw-r--r-- | drivers/of/unittest-data/testcases_common.dtsi | 1 | ||||
| -rw-r--r-- | drivers/of/unittest-data/tests-lifecycle.dtsi | 8 | ||||
| -rw-r--r-- | drivers/of/unittest.c | 150 |
12 files changed, 224 insertions, 20 deletions
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index 80b5fd44ab1c..644386833a7b 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -23,7 +23,19 @@ config OF_UNITTEST that are executed once at boot time, and the results dumped to the console. - If unsure, say N here, but this option is safe to enable. + This option should only be enabled for a development kernel. The tests + will taint the kernel with TAINT_TEST. The tests will cause ERROR and + WARNING messages to print on the console. The tests will cause stack + traces to print on the console. It is possible that the tests will + leave the devicetree in a corrupted state. + + The unittest output will be verbose. Copy the output to a file + via capturing the console output or via the dmesg command. Process + this file with scripts/dtc/of_unittest_expect to reduce the + verbosity, test whether expected output is present, and to + summarize the results. + + If unsure, say N here. This option is not safe to enable. config OF_ALL_DTBS bool "Build all Device Tree Blobs" diff --git a/drivers/of/base.c b/drivers/of/base.c index d5a5c35eba72..ac6fde53342f 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1884,8 +1884,7 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np, { ap->np = np; ap->id = id; - strncpy(ap->stem, stem, stem_len); - ap->stem[stem_len] = 0; + strscpy(ap->stem, stem, stem_len + 1); list_add_tail(&ap->link, &aliases_lookup); pr_debug("adding DT alias:%s: stem=%s id=%i node=%pOF\n", ap->alias, ap->stem, ap->id, np); diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index cd3821a6444f..07d93753b12f 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -329,10 +329,30 @@ void of_node_release(struct kobject *kobj) { struct device_node *node = kobj_to_device_node(kobj); + /* + * can not use '"%pOF", node' in pr_err() calls from this function + * because an of_node_get(node) when refcount is already zero + * will result in an error and a stack dump + */ + /* We should never be releasing nodes that haven't been detached. */ if (!of_node_check_flag(node, OF_DETACHED)) { - pr_err("ERROR: Bad of_node_put() on %pOF\n", node); - dump_stack(); + + pr_err("ERROR: %s() detected bad of_node_put() on %pOF/%s\n", + __func__, node->parent, node->full_name); + + /* + * of unittests will test this path. Do not print the stack + * trace when the error is caused by unittest so that we do + * not display what a normal developer might reasonably + * consider a real bug. + */ + if (!IS_ENABLED(CONFIG_OF_UNITTEST) || + strcmp(node->parent->full_name, "testcase-data")) { + dump_stack(); + pr_err("ERROR: next of_node_put() on this node will result in a kobject warning 'refcount_t: underflow; use-after-free.'\n"); + } + return; } if (!of_node_check_flag(node, OF_DYNAMIC)) @@ -357,6 +377,10 @@ void of_node_release(struct kobject *kobj) __func__, node); } + if (node->child) + pr_err("ERROR: %s() unexpected children for %pOF/%s\n", + __func__, node->parent, node->full_name); + property_list_free(node->properties); property_list_free(node->deadprops); fwnode_links_purge(of_fwnode_handle(node)); @@ -419,7 +443,8 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags) * another node. The node data are dynamically allocated and all the node * flags have the OF_DYNAMIC & OF_DETACHED bits set. * - * Return: The newly allocated node or NULL on out of memory error. + * Return: The newly allocated node or NULL on out of memory error. Use + * of_node_put() on it when done to free the memory allocated for it. */ struct device_node *__of_node_dup(const struct device_node *np, const char *full_name) diff --git a/drivers/of/irq.c b/drivers/of/irq.c index e9bf5236ed89..174900072c18 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -438,10 +438,16 @@ int of_irq_get(struct device_node *dev, int index) return rc; domain = irq_find_host(oirq.np); - if (!domain) - return -EPROBE_DEFER; + if (!domain) { + rc = -EPROBE_DEFER; + goto out; + } - return irq_create_of_mapping(&oirq); + rc = irq_create_of_mapping(&oirq); +out: + of_node_put(oirq.np); + + return rc; } EXPORT_SYMBOL_GPL(of_irq_get); diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c index 7d3853a5a09a..3dbce1e6f184 100644 --- a/drivers/of/kobj.c +++ b/drivers/of/kobj.c @@ -24,7 +24,7 @@ static void of_node_release(struct kobject *kobj) } #endif /* CONFIG_OF_DYNAMIC */ -struct kobj_type of_node_ktype = { +const struct kobj_type of_node_ktype = { .release = of_node_release, }; diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index f90975e00446..948efa9f99e3 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -285,6 +285,16 @@ void __init fdt_init_reserved_mem(void) else memblock_phys_free(rmem->base, rmem->size); + } else { + phys_addr_t end = rmem->base + rmem->size - 1; + bool reusable = + (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL; + + pr_info("%pa..%pa (%lu KiB) %s %s %s\n", + &rmem->base, &end, (unsigned long)(rmem->size / SZ_1K), + nomap ? "nomap" : "map", + reusable ? "reusable" : "non-reusable", + rmem->name ? rmem->name : "unknown"); } } } diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index ed4e6c144a68..2e01960f1aeb 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -1121,7 +1121,7 @@ static int node_overlaps_later_cs(struct overlay_changeset *remove_ovcs, * The topmost check is done by exploiting this property. For each * affected device node in the log list we check if this overlay is * the one closest to the tail. If another overlay has affected this - * device node and is closest to the tail, then removal is not permited. + * device node and is closest to the tail, then removal is not permitted. */ static int overlay_removal_is_ok(struct overlay_changeset *remove_ovcs) { diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b3878a98d27f..b2bd2e783445 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -222,7 +222,6 @@ static struct amba_device *of_amba_device_create(struct device_node *node, struct device *parent) { struct amba_device *dev; - const void *prop; int ret; pr_debug("Creating amba device %pOF\n", node); @@ -250,9 +249,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node, of_device_make_bus_id(&dev->dev); /* Allow the HW Peripheral ID to be overridden */ - prop = of_get_property(node, "arm,primecell-periphid", NULL); - if (prop) - dev->periphid = of_read_ulong(prop, 1); + of_property_read_u32(node, "arm,primecell-periphid", &dev->periphid); ret = of_address_to_resource(node, 0, &dev->res); if (ret) { @@ -529,7 +526,7 @@ static int __init of_platform_default_populate_init(void) int ret; /* Check if we have a MacOS display without a node spec */ - if (of_get_property(of_chosen, "linux,bootx-noscreen", NULL)) { + if (of_property_present(of_chosen, "linux,bootx-noscreen")) { /* * The old code tried to work out which node was the MacOS * display based on the address. I'm dropping that since the diff --git a/drivers/of/property.c b/drivers/of/property.c index fb210e921ca9..ddc75cd50825 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1072,7 +1072,7 @@ static struct device_node *of_get_compat_node(struct device_node *np) np = NULL; } - if (of_find_property(np, "compatible", NULL)) + if (of_property_present(np, "compatible")) break; np = of_get_next_parent(np); @@ -1300,7 +1300,7 @@ static struct device_node *parse_gpio_compat(struct device_node *np, * Ignore node with gpio-hog property since its gpios are all provided * by its parent. */ - if (of_find_property(np, "gpio-hog", NULL)) + if (of_property_read_bool(np, "gpio-hog")) return NULL; if (of_parse_phandle_with_args(np, prop_name, "#gpio-cells", index, diff --git a/drivers/of/unittest-data/testcases_common.dtsi b/drivers/of/unittest-data/testcases_common.dtsi index 19292bbb4cbb..e7887f2301c1 100644 --- a/drivers/of/unittest-data/testcases_common.dtsi +++ b/drivers/of/unittest-data/testcases_common.dtsi @@ -17,3 +17,4 @@ #include "tests-address.dtsi" #include "tests-platform.dtsi" #include "tests-overlay.dtsi" +#include "tests-lifecycle.dtsi" diff --git a/drivers/of/unittest-data/tests-lifecycle.dtsi b/drivers/of/unittest-data/tests-lifecycle.dtsi new file mode 100644 index 000000000000..28509a8783a7 --- /dev/null +++ b/drivers/of/unittest-data/tests-lifecycle.dtsi @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 + +/ { + testcase-data { + refcount-node { + }; + }; +}; diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index bc0f1e50a4be..b5a7a31d8bd2 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -54,8 +54,9 @@ static struct unittest_results { * Print the expected message only if the current loglevel will allow * the actual message to print. * - * Do not use EXPECT_BEGIN() or EXPECT_END() for messages generated by - * pr_debug(). + * Do not use EXPECT_BEGIN(), EXPECT_END(), EXPECT_NOT_BEGIN(), or + * EXPECT_NOT_END() to report messages expected to be reported or not + * reported by pr_debug(). */ #define EXPECT_BEGIN(level, fmt, ...) \ printk(level pr_fmt("EXPECT \\ : ") fmt, ##__VA_ARGS__) @@ -63,6 +64,12 @@ static struct unittest_results { #define EXPECT_END(level, fmt, ...) \ printk(level pr_fmt("EXPECT / : ") fmt, ##__VA_ARGS__) +#define EXPECT_NOT_BEGIN(level, fmt, ...) \ + printk(level pr_fmt("EXPECT_NOT \\ : ") fmt, ##__VA_ARGS__) + +#define EXPECT_NOT_END(level, fmt, ...) \ + printk(level pr_fmt("EXPECT_NOT / : ") fmt, ##__VA_ARGS__) + static void __init of_unittest_find_node_by_name(void) { struct device_node *np; @@ -1488,6 +1495,7 @@ static int __init unittest_data_add(void) struct device_node *next = np->sibling; np->parent = of_root; + /* this will clear OF_DETACHED in np and children */ attach_node_and_children(np); np = next; } @@ -2998,6 +3006,143 @@ out: static inline void __init of_unittest_overlay(void) { } #endif +static void __init of_unittest_lifecycle(void) +{ +#ifdef CONFIG_OF_DYNAMIC + unsigned int refcount; + int found_refcount_one = 0; + int put_count = 0; + struct device_node *np; + struct device_node *prev_sibling, *next_sibling; + const char *refcount_path = "/testcase-data/refcount-node"; + const char *refcount_parent_path = "/testcase-data"; + + /* + * Node lifecycle tests, non-dynamic node: + * + * - Decrementing refcount to zero via of_node_put() should cause the + * attempt to free the node memory by of_node_release() to fail + * because the node is not a dynamic node. + * + * - Decrementing refcount past zero should result in additional + * errors reported. + */ + + np = of_find_node_by_path(refcount_path); + unittest(np, "find refcount_path \"%s\"\n", refcount_path); + if (np == NULL) + goto out_skip_tests; + + while (!found_refcount_one) { + + if (put_count++ > 10) { + unittest(0, "guardrail to avoid infinite loop\n"); + goto out_skip_tests; + } + + refcount = kref_read(&np->kobj.kref); + if (refcount == 1) + found_refcount_one = 1; + else + of_node_put(np); + } + + EXPECT_BEGIN(KERN_INFO, "OF: ERROR: of_node_release() detected bad of_node_put() on /testcase-data/refcount-node"); + + /* + * refcount is now one, decrementing to zero will result in a call to + * of_node_release() to free the node's memory, which should result + * in an error + */ + unittest(1, "/testcase-data/refcount-node is one"); + of_node_put(np); + + EXPECT_END(KERN_INFO, "OF: ERROR: of_node_release() detected bad of_node_put() on /testcase-data/refcount-node"); + + + /* + * expect stack trace for subsequent of_node_put(): + * __refcount_sub_and_test() calls: + * refcount_warn_saturate(r, REFCOUNT_SUB_UAF) + * + * Not capturing entire WARN_ONCE() trace with EXPECT_*(), just + * the first three lines, and the last line. + */ + EXPECT_BEGIN(KERN_INFO, "------------[ cut here ]------------"); + EXPECT_BEGIN(KERN_INFO, "WARNING: <<all>>"); + EXPECT_BEGIN(KERN_INFO, "refcount_t: underflow; use-after-free."); + EXPECT_BEGIN(KERN_INFO, "---[ end trace <<int>> ]---"); + + /* refcount is now zero, this should fail */ + unittest(1, "/testcase-data/refcount-node is zero"); + of_node_put(np); + + EXPECT_END(KERN_INFO, "---[ end trace <<int>> ]---"); + EXPECT_END(KERN_INFO, "refcount_t: underflow; use-after-free."); + EXPECT_END(KERN_INFO, "WARNING: <<all>>"); + EXPECT_END(KERN_INFO, "------------[ cut here ]------------"); + + /* + * Q. do we expect to get yet another warning? + * A. no, the WARNING is from WARN_ONCE() + */ + EXPECT_NOT_BEGIN(KERN_INFO, "------------[ cut here ]------------"); + EXPECT_NOT_BEGIN(KERN_INFO, "WARNING: <<all>>"); + EXPECT_NOT_BEGIN(KERN_INFO, "refcount_t: underflow; use-after-free."); + EXPECT_NOT_BEGIN(KERN_INFO, "---[ end trace <<int>> ]---"); + + unittest(1, "/testcase-data/refcount-node is zero, second time"); + of_node_put(np); + + EXPECT_NOT_END(KERN_INFO, "---[ end trace <<int>> ]---"); + EXPECT_NOT_END(KERN_INFO, "refcount_t: underflow; use-after-free."); + EXPECT_NOT_END(KERN_INFO, "WARNING: <<all>>"); + EXPECT_NOT_END(KERN_INFO, "------------[ cut here ]------------"); + + /* + * refcount of zero will trigger stack traces from any further + * attempt to of_node_get() node "refcount-node". One example of + * this is where of_unittest_check_node_linkage() will recursively + * scan the tree, with 'for_each_child_of_node()' doing an + * of_node_get() of the children of a node. + * + * Prevent the stack trace by removing node "refcount-node" from + * its parent's child list. + * + * WARNING: EVIL, EVIL, EVIL: + * + * Directly manipulate the child list of node /testcase-data to + * remove child refcount-node. This is ignoring all proper methods + * of removing a child and will leak a small amount of memory. + */ + + np = of_find_node_by_path(refcount_parent_path); + unittest(np, "find refcount_parent_path \"%s\"\n", refcount_parent_path); + unittest(np, "ERROR: devicetree live tree left in a 'bad state' if test fail\n"); + if (np == NULL) + return; + + prev_sibling = np->child; + next_sibling = prev_sibling->sibling; + if (!strcmp(prev_sibling->full_name, "refcount-node")) { + np->child = next_sibling; + next_sibling = next_sibling->sibling; + } + while (next_sibling) { + if (!strcmp(next_sibling->full_name, "refcount-node")) + prev_sibling->sibling = next_sibling->sibling; + prev_sibling = next_sibling; + next_sibling = next_sibling->sibling; + } + of_node_put(np); + + return; + +out_skip_tests: +#endif + unittest(0, "One or more lifecycle tests skipped\n"); +} + #ifdef CONFIG_OF_OVERLAY /* @@ -3502,6 +3647,7 @@ static int __init of_unittest(void) of_unittest_match_node(); of_unittest_platform_populate(); of_unittest_overlay(); + of_unittest_lifecycle(); /* Double check linkage after removing testcase data */ of_unittest_check_tree_linkage(); |