Re: [PATCH v4 3/7] clk: tests: Add Kunit testing for of_clk_get_parent_name()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential use-after-free of device tree strings and clock names when returning clock parent names.
--

--- Patch [3]: [PATCH v4 3/7] clk: tests: Add Kunit testing for of_clk_get_parent_name() ---
commit 4b760cb4ff3b2b61c5f90468c3fd87c387504cd7
Author: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>

clk: tests: Add Kunit testing for of_clk_get_parent_name()

This commit adds a Kunit test to verify that of_clk_get_parent_name()
returns the correct clock name when looking up by index.

> diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
> index f47f81b7d72a4..4084559e157df 100644
> --- a/drivers/clk/clk_test.c
> +++ b/drivers/clk/clk_test.c

[ ... ]

> @@ -3637,9 +3637,23 @@ static void clk_parse_clkspec_with_incorrect_index_and_name(struct kunit *test)
>  	KUNIT_EXPECT_TRUE(test, IS_ERR(hw));
>  }
>  
> +/*
> + * Verify that of_clk_get_parent_name() returns the correct clock name when
> + * looking up by index through the consumer's clocks property.
> + */
> +static void of_clk_get_parent_name_gets_parent_name(struct kunit *test)
> +{
> +	struct clk_parse_clkspec_ctx *ctx = test->priv;
> +	const char *expected_name = "clk_parse_clkspec_1";
> +
> +	KUNIT_EXPECT_STREQ(test, expected_name,
> +			   of_clk_get_parent_name(ctx->cons_np, 0));

[Severity: High]
This isn't a bug introduced by this patch, but does of_clk_get_parent_name()
return a potentially dangling pointer here?

Looking at of_clk_get_parent_name() in drivers/clk/clk.c:

    clk_name = __clk_get_name(clk);
    clk_put(clk);
    ...
    of_node_put(clkspec.np);
    return clk_name;

The string pointer is extracted and the reference to its owning object (the
clock or the device node) is dropped before returning. If the node is from a
removed DT overlay or the clock provider is concurrently unregistered, and
this was the last reference, wouldn't the memory backing the string be freed
leaving the caller with a dangling pointer?

> +}
> +
>  static struct kunit_case clk_parse_clkspec_test_cases[] = {
>  	KUNIT_CASE(clk_parse_clkspec_with_correct_index_and_name),
>  	KUNIT_CASE(clk_parse_clkspec_with_incorrect_index_and_name),
> +	KUNIT_CASE(of_clk_get_parent_name_gets_parent_name),
>  	{}
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-schneider-v7-2-rc1-eip201-upstream-v4-0-751547e160e5@bootlin.com?part=3