Re: [PATCH] of: unittest: fix node leak in phandle remapping test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] of: unittest: leak of p[] nodes in of_unittest_parse_phandle_with_args_map()
--

commit 76085e24075478d56a6949c4d7365c6d0d60c538
Author: Wentao Liang <vulab@iscas.ac.cn>

of: unittest: fix node leak in phandle remapping test

This commit releases the device_node reference for the consumer node and
adds cleanup for the provider node references when a provider lookup fails
in of_unittest_parse_phandle_with_args_map().

> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index e255f54f4d760..f240b1175211b 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -578,6 +578,9 @@ static void __init of_unittest_parse_phandle_with_args_map(void)
>  	for (i = 0; i < ARRAY_SIZE(p); ++i) {
>  		if (!p[i]) {
>  			pr_err("missing testcase data\n");
> +			while (i > 0)
> +				of_node_put(p[--i]);
> +			of_node_put(np);
>  			return;
>  		}
>  		prefs[i] = OF_KREF_READ(p[i]);

[Severity: Medium]
Does this error path still leak device_node references in p[i + 1] through
p[5]?

In drivers/of/unittest.c:of_unittest_parse_phandle_with_args_map(), all six
provider nodes are looked up before the validation loop begins:

	p[0] = of_find_node_by_path("/testcase-data/phandle-tests/provider0");
	p[1] = of_find_node_by_path("/testcase-data/phandle-tests/provider1");
	p[2] = of_find_node_by_path("/testcase-data/phandle-tests/provider2");
	p[3] = of_find_node_by_path("/testcase-data/phandle-tests/provider3");
	p[4] = of_find_node_by_path("/testcase-data/phandle-tests/provider4");
	p[5] = of_find_node_by_path("/testcase-data/phandle-tests/provider5");
	for (i = 0; i < ARRAY_SIZE(p); ++i) {
		if (!p[i]) {

If p[i] is NULL for an earlier entry (for example p[0]) while subsequent
lookups in p[i + 1 .. 5] succeeded, unwinding only from i down to 0 leaves
the non-NULL references in the remaining elements of p[] unreleased.

Since of_node_put() safely accepts NULL pointers, should this loop over the
entire p[] array (and update the commit message, which currently states that
the references were acquired by preceding loop iterations)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103104235.1373591-1-vulab@iscas.ac.cn?part=1