linux-kernel - Re: [PATCH v2] clk: scmi: Fix children encountered before parents case

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: <20250612-vivacious-bipedal-beetle-b04714@sudeepholla>
Date: Thu, 12 Jun 2025 10:35:53 +0100
From: Sudeep Holla <sudeep.holla@....com>
To: Sascha Hauer <s.hauer@...gutronix.de>
Cc: Cristian Marussi <cristian.marussi@....com>,
	Sudeep Holla <sudeep.holla@....com>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...nel.org>, Peng Fan <peng.fan@....com>,
	<arm-scmi@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-clk@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] clk: scmi: Fix children encountered before parents
 case

On Thu, Jun 12, 2025 at 10:43:32AM +0200, Sascha Hauer wrote:
> On Thu, Jun 12, 2025 at 09:29:16AM +0100, Sudeep Holla wrote:
> > On Thu, Jun 12, 2025 at 09:36:58AM +0200, Sascha Hauer wrote:
> > > When it comes to clocks with parents the SCMI clk driver assumes that
> > > parents are always initialized before their children which might not
> > > always be the case.
> > > 
> > > During initialization of the parent_data array we have:
> > > 
> > > 	sclk->parent_data[i].hw = hws[sclk->info->parents[i]];
> > > 
> > > hws[sclk->info->parents[i]] will not yet be initialized when children
> > > are encountered before their possible parents. Solve this by allocating
> > > all struct scmi_clk as an array first and populating all hws[] upfront.
> > > 
> > 
> > LGTM. I would like to add a note that we don't free individual scmi_clk
> > if for some reason it fails to setup. I can do that when I apply, just
> > checking if anyone has any objections. Please shout out if you have.
> 
> Feel free to add that note. I should have added this myself since it's
> not entirely obvious that the devm_kfree() has to be removed with this
> patch.
> 

I did that and realised only bit later that I usually route SCMI clk driver
changes via clk tree.

Please repost with

Reviewed-by: Sudeep Holla <sudeep.holla@....com>

I am sharing below the commit message update I did when I applied.

Regards,
Sudeep

-->8

clk: scmi: Handle case where child clocks are initialized before their parents

The SCMI clock driver currently assumes that parent clocks are always
initialized before their children. However, this assumption can fail if
a child clock is encountered before its parent during probe.

This leads to an issue during initialization of the parent_data array:

    sclk->parent_data[i].hw = hws[sclk->info->parents[i]];

If the parent clock's hardware structure has not been initialized yet,
this assignment results in invalid data.

To resolve this, allocate all struct scmi_clk instances as a contiguous
array at the beginning of the probe and populate the hws[] array upfront.
This ensures that any parent referenced later is already initialized,
regardless of the order in which clocks are processed.

Note that we can no longer free individual scmi_clk instances if
scmi_clk_ops_init() fails which shouldn't be a problem if the SCMI
platform has proper per-agent clock discovery.