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();