diff options
author | Frank Rowand <frowand.list@gmail.com> | 2023-02-13 12:56:59 -0600 |
---|---|---|
committer | Rob Herring <robh@kernel.org> | 2023-02-20 15:36:51 -0600 |
commit | 74df14cd301a1433947077e79ce2c610654a32e7 (patch) | |
tree | 4bfd35c904f7bd848d7925c516b3ae05acee9e2e /drivers/of | |
parent | f381b31a80bc47102f5a3f3001d8e45c328eb548 (diff) |
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 <frowand.list@gmail.com>
Link: https://lore.kernel.org/r/20230213185702.395776-5-frowand.list@gmail.com
[robh: Fix !CONFIG_OF_DYNAMIC build]
Signed-off-by: Rob Herring <robh@kernel.org>
Diffstat (limited to 'drivers/of')
-rw-r--r-- | drivers/of/dynamic.c | 14 | ||||
-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 |
4 files changed, 170 insertions, 3 deletions
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: <<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(); |