linux-kernel - Re: [PATCH v2] clk: Avoid DT fetch in possible_parent_show if clk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGb2v66rvs_wM-vUrMiKHGCmVi2hKXsiBhmr8ATwfXnXv24trQ@mail.gmail.com>
Date: Sun, 8 Feb 2026 21:57:46 +0800
From: Chen-Yu Tsai <wens@...e.org>
To: Yao Zi <me@...ao.cc>
Cc: Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, linux-clk@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Drew Fustini <fustini@...nel.org>
Subject: Re: [PATCH v2] clk: Avoid DT fetch in possible_parent_show if clk_hw
 is provided

On Thu, Feb 5, 2026 at 11:42 PM Yao Zi <me@...ao.cc> wrote:
>
> When showing a parent for which clk_core_get_parent_by_index fails, we
> may try using the parent's global name or the local name. If this fails
> either, the parent clock's clock-output-names is fetched through
> DT-index.
>
> struct clk_hw pointer takes precedence with DT-index when registering
> clocks, thus most drivers only zero the index member of struct
> clk_parent_data when providing the parent through struct clk_hw pointer.
> If the pointer cannot resovle to a clock, clk_core_get_parent_by_index

                        ^ typo here.

> will fail as well, in which case possible_parent_show will fetch the
> parent's clock-output-names property, treat the unintended, zeroed index
> as valid, and yield a misleading name if the clock controller does come
> with a clocks property.
>
> Let's add an extra check against the struct clk_hw pointer, and only
> perform the DT-index-based fetch if it isn't provided.
>
> Fixes: 2d156b78ce8f ("clk: Fix debugfs clk_possible_parents for clks without parent string names")
> Signed-off-by: Yao Zi <me@...ao.cc>
> Tested-by: Drew Fustini <fustini@...nel.org>
> ---
>  drivers/clk/clk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> This was found when fixing the wrong parent description of
> clk-th1520-ap.c[1]. Without the patch,
>
>         # cat /sys/kernel/debug/clk/c910/clk_possible_parents
>         osc_24m cpu-pll1
>
> The first parent should be c910-i0, provided by an unresolvable struct
> clk_hw pointer. osc_24m is the first (and only) parent specified in
> devicetree for the clock controller. With the patch,
>
>         # cat /sys/kernel/debug/clk/c910/clk_possible_parents
>         (missing) cpu-pll1
>
> [1]: https://lore.kernel.org/linux-riscv/20250705052028.24611-1-ziyao@disroot.org/
>
> Changed from v1:
> - Collect tags
> - Switch to my new mail address
> - Link to v1: https://lore.kernel.org/all/20250705095816.29480-2-ziyao@disroot.org/
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 85d2f2481acf..a9d1aea59689 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3588,7 +3588,7 @@ static void possible_parent_show(struct seq_file *s, struct clk_core *core,
>         } else if (core->parents[i].fw_name) {
>                 seq_printf(s, "<%s>(fw)", core->parents[i].fw_name);
>         } else {
> -               if (core->parents[i].index >= 0)
> +               if (!core->parents[i].hw && core->parents[i].index >= 0)
>                         name = of_clk_get_parent_name(core->of_node, core->parents[i].index);
>                 if (!name)
>                         name = "(missing)";

I think this is slightly wrong. It should never fall back to looking
to the other sources if parents[i].hw is a valid pointer, since that
takes precedence.

I think it should look more like:

parent = clk_core_get_parent_by_index(core, i);
if (parent) {
        seq_puts(s, parent->name);
} else if (!core->parents[i].hw) {
        if (core->parents[i].name) {
                seq_puts(s, core->parents[i].name);
        } else if (core->parents[i].fw_name) {
                seq_printf(s, "<%s>(fw)", core->parents[i].fw_name);
        } else {
                if (core->parents[i].index >= 0)
                        name = of_clk_get_parent_name(core->of_node,
core->parents[i].index);
                if (!name)
                        name = "(missing)";
                seq_puts(s, name);
        }
}

Anyway, thank you for fixing this. I wrote the original code and this was
one of the corner cases that we didn't have on the sunxi platform.


ChenYu