From 8e5d0c68f23ab139e472f93ebbcfda9545e9953b Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Tue, 20 Dec 2022 23:20:32 +0100 Subject: of: overlay: Fix trivial typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Permitted is spelled with two t. Signed-off-by: Ricardo Ribalda Reviewed-by: Philippe Mathieu-Daudé Link: https://lore.kernel.org/r/20221220-permited-v1-3-52ea9857fa61@chromium.org Signed-off-by: Rob Herring --- drivers/of/overlay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') 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) { -- cgit v1.2.3 From a98bf9df1c332a2c447083e1a2ca9578cd9f0721 Mon Sep 17 00:00:00 2001 From: Xu Panda Date: Fri, 23 Dec 2022 10:39:12 +0800 Subject: of: base: use strscpy() to instead of strncpy() The implementation of strscpy() is more robust and safer. That's now the recommended way to copy NUL terminated strings. Signed-off-by: Xu Panda Signed-off-by: Yang Yang Link: https://lore.kernel.org/r/202212231039128402297@zte.com.cn Signed-off-by: Rob Herring --- drivers/of/base.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers') 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); -- cgit v1.2.3 From eb2b4ecf7299ee39ee9cecc4d3f2633aeb10cefb Mon Sep 17 00:00:00 2001 From: Clément Léger Date: Tue, 17 Jan 2023 15:49:29 +0100 Subject: of/irq: add missing of_node_put() for interrupt parent node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After calling of_irq_parse_one(), the node provided in the of_phandle_args has a refcount increment by one. Add missing of_node_put in of_irq_get() to decrement the refcount once used. Signed-off-by: Clément Léger Link: https://lore.kernel.org/r/20230117144929.423089-1-clement.leger@bootlin.com Signed-off-by: Rob Herring --- drivers/of/irq.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'drivers') 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); -- cgit v1.2.3 From 20f6d4f2a474fc2dc502358dcee3c9b18304ec1e Mon Sep 17 00:00:00 2001 From: Thomas Weißschuh Date: Sat, 4 Feb 2023 05:47:03 +0000 Subject: of: make of_node_ktype constant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit ee6d3dd4ed48 ("driver core: make kobj_type constant.") the driver core allows the usage of const struct kobj_type. Take advantage of this to constify the structure definition to prevent modification at runtime. Signed-off-by: Thomas Weißschuh Link: https://lore.kernel.org/r/20230204-kobj_type-of-v1-1-5910c8ecb7a3@weissschuh.net Signed-off-by: Rob Herring --- drivers/of/kobj.c | 2 +- include/linux/of.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') 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/include/linux/of.h b/include/linux/of.h index 8b9f94386dc3..8bb348666709 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -101,7 +101,7 @@ struct of_reconfig_data { }; /* initialize a node */ -extern struct kobj_type of_node_ktype; +extern const struct kobj_type of_node_ktype; extern const struct fwnode_operations of_fwnode_ops; static inline void of_node_init(struct device_node *node) { -- cgit v1.2.3 From aeb9267eb6b1df992e39467a620da8fdf434df54 Mon Sep 17 00:00:00 2001 From: Martin Liu Date: Fri, 10 Feb 2023 00:09:55 +0800 Subject: of: reserved-mem: print out reserved-mem details during boot It's important to know reserved-mem information in mobile world since reserved memory via device tree keeps increased in platform (e.g., 45% in our platform). Therefore, it's crucial to know the reserved memory sizes breakdown for the memory accounting. This patch prints out reserved memory details during boot to make them visible. Below is an example output: [ 0.000000] OF: reserved mem: 0x00000009f9400000..0x00000009fb3fffff ( 32768 KB ) map reusable test1 [ 0.000000] OF: reserved mem: 0x00000000ffdf0000..0x00000000ffffffff ( 2112 KB ) map non-reusable test2 [ 0.000000] OF: reserved mem: 0x0000000091000000..0x00000000912fffff ( 3072 KB ) nomap non-reusable test3 Signed-off-by: Martin Liu Link: https://lore.kernel.org/r/20230209160954.1471909-1-liumartin@google.com Signed-off-by: Rob Herring --- drivers/of/of_reserved_mem.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'drivers') diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 65f3b02a0e4e..00e75064ca19 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -284,6 +284,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 KB ) %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"); } } } -- cgit v1.2.3 From 6ee7afbabcee4d54024c46f8fc74314c69a04613 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Thu, 16 Feb 2023 09:37:25 +0100 Subject: of: reserved_mem: Use proper binary prefix The printed reserved memory information uses the non-standard "K" prefix, while all other printed values use proper binary prefixes. Fix this by using "Ki" instead. While at it, drop the superfluous spaces inside the parentheses, to reduce printed line length. Fixes: aeb9267eb6b1df99 ("of: reserved-mem: print out reserved-mem details during boot") Signed-off-by: Geert Uytterhoeven Link: https://lore.kernel.org/r/20230216083725.1244817-1-geert+renesas@glider.be Signed-off-by: Rob Herring --- drivers/of/of_reserved_mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 00e75064ca19..9f7127297f4d 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -289,7 +289,7 @@ void __init fdt_init_reserved_mem(void) bool reusable = (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL; - pr_info("%pa..%pa ( %lu KB ) %s %s %s\n", + 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", -- cgit v1.2.3 From 2f0cb4753dd2b5fe71d04407213e2ef253dcdd32 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Thu, 9 Feb 2023 17:20:43 -0600 Subject: of: Use of_property_present() helper Use of_property_present() instead of of_get_property/of_find_property() in places where we just need to test presence of a property. Reviewed-by: Frank Rowand Tested-by: Frank Rowand Link: https://lore.kernel.org/all/20230215215547.691573-2-robh@kernel.org/ Signed-off-by: Rob Herring --- drivers/of/platform.c | 2 +- drivers/of/property.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 81c8c227ab6b..f7d796b54039 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -528,7 +528,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 134cfc980b70..ff71d2ac26cb 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1086,7 +1086,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); -- cgit v1.2.3 From 16b0c7cad99e96b002e26f016f28150cc7c52514 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Wed, 8 Feb 2023 17:13:23 -0600 Subject: of: Use preferred of_property_read_* functions Replace instances of of_get_property/of_find_property() with appropriate typed of_property_read_*() functions. Link: https://lore.kernel.org/all/20230215215502.690716-1-robh@kernel.org/ Signed-off-by: Rob Herring --- drivers/of/platform.c | 5 +---- drivers/of/property.c | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/of/platform.c b/drivers/of/platform.c index f7d796b54039..9f015ad73288 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) { diff --git a/drivers/of/property.c b/drivers/of/property.c index ff71d2ac26cb..a229536f7564 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1358,7 +1358,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, -- cgit v1.2.3 From f381b31a80bc47102f5a3f3001d8e45c328eb548 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Mon, 13 Feb 2023 12:56:58 -0600 Subject: of: update kconfig unittest help Add more information about the impact the of unittests have on the live devicetree and why the tests should only be enabled for developer kernels. Add information about processing the test output such that the results are more complete and comprehendable. Signed-off-by: Frank Rowand Link: https://lore.kernel.org/r/20230213185702.395776-4-frowand.list@gmail.com Signed-off-by: Rob Herring --- drivers/of/Kconfig | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'drivers') 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" -- cgit v1.2.3 From 74df14cd301a1433947077e79ce2c610654a32e7 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Mon, 13 Feb 2023 12:56:59 -0600 Subject: of: unittest: add node lifecycle tests Add tests to exercise the actions that occur when the reference count of devicetree nodes decrement to zero and beyond. Decrementing to zero triggers freeing memory allocated for the node. This commit will expose a pr_err() issue in of_node_release(), resulting in some kernal warnings and stack traces. When scripts/dtc/of_unittest_expect processes the console messages, it will also report related problems for EXPECT messages due to the pr_err() issue: ** missing EXPECT begin : 5 Signed-off-by: Frank Rowand Link: https://lore.kernel.org/r/20230213185702.395776-5-frowand.list@gmail.com [robh: Fix !CONFIG_OF_DYNAMIC build] Signed-off-by: Rob Herring --- drivers/of/dynamic.c | 14 ++- drivers/of/unittest-data/testcases_common.dtsi | 1 + drivers/of/unittest-data/tests-lifecycle.dtsi | 8 ++ drivers/of/unittest.c | 150 ++++++++++++++++++++++++- 4 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 drivers/of/unittest-data/tests-lifecycle.dtsi (limited to 'drivers') diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index cd3821a6444f..becb80f762c8 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -332,7 +332,19 @@ void of_node_release(struct kobject *kobj) /* 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(); + + /* + * 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 kboject warning 'refcount_t: underflow; use-after-free.'\n"); + } + return; } if (!of_node_check_flag(node, OF_DYNAMIC)) 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: <>"); + EXPECT_BEGIN(KERN_INFO, "refcount_t: underflow; use-after-free."); + EXPECT_BEGIN(KERN_INFO, "---[ end trace <> ]---"); + + /* 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 <> ]---"); + EXPECT_END(KERN_INFO, "refcount_t: underflow; use-after-free."); + EXPECT_END(KERN_INFO, "WARNING: <>"); + 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: <>"); + EXPECT_NOT_BEGIN(KERN_INFO, "refcount_t: underflow; use-after-free."); + EXPECT_NOT_BEGIN(KERN_INFO, "---[ end trace <> ]---"); + + unittest(1, "/testcase-data/refcount-node is zero, second time"); + of_node_put(np); + + EXPECT_NOT_END(KERN_INFO, "---[ end trace <> ]---"); + EXPECT_NOT_END(KERN_INFO, "refcount_t: underflow; use-after-free."); + EXPECT_NOT_END(KERN_INFO, "WARNING: <>"); + 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(); -- cgit v1.2.3 From 23522dd7033ad8dcd818f75469907a8fdac8e8a4 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Mon, 13 Feb 2023 12:57:00 -0600 Subject: of: do not use "%pOF" printk format on node with refcount of zero of_node_release() can not use the "%pOF" printk format to report the node name of a node when the node reference count is zero. This is because the formatter device_node_string() calls fwnode_full_name_string() which indirectly calls of_node_get(). Calling of_node_get() on the node with a zero reference count results in a WARNING and stack trace. When the reference count has been decremented to zero, this function is in the subsequent call path which frees memory related to the node. This commit resolves the unittest EXPECT errors that were created in the previous commmit. Signed-off-by: Frank Rowand Link: https://lore.kernel.org/r/20230213185702.395776-6-frowand.list@gmail.com Signed-off-by: Rob Herring --- drivers/of/dynamic.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index becb80f762c8..dbcbc41f3465 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -329,9 +329,17 @@ 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); + + 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 -- cgit v1.2.3 From ec0b7e24d566a843e3cfba21a6471170fdb0f810 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Mon, 13 Feb 2023 12:57:01 -0600 Subject: of: add consistency check to of_node_release() Add an additional consistency check to of_node_release(), which is called when the reference count of a devicetree node is decremented to zero. The node's children should have been deleted before the node is deleted so check that no children exist. Signed-off-by: Frank Rowand Link: https://lore.kernel.org/r/20230213185702.395776-7-frowand.list@gmail.com Signed-off-by: Rob Herring --- drivers/of/dynamic.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers') diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index dbcbc41f3465..657a65006056 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -377,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)); -- cgit v1.2.3 From d9194e009efef9613f48022f3c885191c48120f9 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Mon, 13 Feb 2023 12:57:02 -0600 Subject: of: dynamic: add lifecycle docbook info to node creation functions The existing docbook comments for the functions related to creating a devicetree node do not explain the reference count of a newly created node, how decrementing the reference count to zero will free the associated memory, and the caller's responsibility to call of_node_put() on the node. Explain what happens when the reference count is decremented to zero. Signed-off-by: Frank Rowand Link: https://lore.kernel.org/r/20230213185702.395776-8-frowand.list@gmail.com Signed-off-by: Rob Herring --- drivers/of/dynamic.c | 3 ++- include/linux/of.h | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 657a65006056..12aa99018969 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -443,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/include/linux/of.h b/include/linux/of.h index 37afd04f36eb..1cb659e682f5 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -100,6 +100,17 @@ struct of_reconfig_data { struct property *old_prop; }; +/** + * of_node_init - initialize a devicetree node + * @node: Pointer to device node that has been created by kzalloc() + * @phandle_name: Name of property holding a phandle value + * + * On return the device_node refcount is set to one. Use of_node_put() + * on @node when done to free the memory allocated for it. If the node + * is NOT a dynamic node the memory will not be freed. The decision of + * whether to free the memory will be done by node->release(), which is + * of_node_release(). + */ /* initialize a node */ extern const struct kobj_type of_node_ktype; extern const struct fwnode_operations of_fwnode_ops; -- cgit v1.2.3 From b4858dc61647e5439cbba79022bfc87328c1fb84 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Mon, 20 Feb 2023 14:44:22 +0000 Subject: of: dynamic: Fix spelling mistake "kojbect" -> "kobject" There is a spelling mistake in a pr_err message. Fix it. Signed-off-by: Colin Ian King Reviewed-by: Frank Rowand Link: https://lore.kernel.org/r/20230220144422.873356-1-colin.i.king@gmail.com Signed-off-by: Rob Herring --- drivers/of/dynamic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 12aa99018969..07d93753b12f 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -350,7 +350,7 @@ void of_node_release(struct kobject *kobj) 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 kboject warning 'refcount_t: underflow; use-after-free.'\n"); + pr_err("ERROR: next of_node_put() on this node will result in a kobject warning 'refcount_t: underflow; use-after-free.'\n"); } return; -- cgit v1.2.3