| 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
| ||
|
Message-ID: <ed953445-9c55-4658-af16-04eaa71d746e@tuxon.dev>
Date: Sat, 15 Feb 2025 15:55:51 +0200
From: Claudiu Beznea <claudiu.beznea@...on.dev>
To: Stephen Boyd <sboyd@...nel.org>,
Michael Turquette <mturquette@...libre.com>
Cc: linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Doug Anderson <dianders@...omium.org>
Subject: Re: [PATCH 9/9] clk: Overwrite clk_hw::init with NULL during
clk_register()
Hi, Stephen,
On 31.07.2019 22:35, Stephen Boyd wrote:
> We don't want clk provider drivers to use the init structure after clk
> registration time, but we leave a dangling reference to it by means of
> clk_hw::init. Let's overwrite the member with NULL during clk_register()
> so that this can't be used anymore after registration time.
>
> Cc: Bjorn Andersson <bjorn.andersson@...aro.org>
> Cc: Doug Anderson <dianders@...omium.org>
> Signed-off-by: Stephen Boyd <sboyd@...nel.org>
> ---
>
> Please ack so I can take this through clk tree
>
> drivers/clk/clk.c | 24 ++++++++++++++++--------
> include/linux/clk-provider.h | 3 ++-
> 2 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c0990703ce54..efac620264a2 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3484,9 +3484,9 @@ static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist)
> return 0;
> }
>
> -static int clk_core_populate_parent_map(struct clk_core *core)
> +static int clk_core_populate_parent_map(struct clk_core *core,
> + const struct clk_init_data *init)
> {
> - const struct clk_init_data *init = core->hw->init;
> u8 num_parents = init->num_parents;
> const char * const *parent_names = init->parent_names;
> const struct clk_hw **parent_hws = init->parent_hws;
> @@ -3566,6 +3566,14 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
> {
> int ret;
> struct clk_core *core;
> + const struct clk_init_data *init = hw->init;
> +
> + /*
> + * The init data is not supposed to be used outside of registration path.
> + * Set it to NULL so that provider drivers can't use it either and so that
> + * we catch use of hw->init early on in the core.
> + */
> + hw->init = NULL;
I found that this line impact the drivers/clk/clk-versaclock3.c driver when
doing unbind/bind. The driver provides statically initialized hw.init
object for registration, the hw.init pointers are set to NULL after clock
registrations then the next registration (when re-binding) fails.
In the driver probe the clock are registered like:
for (i = 0; i < ARRAY_SIZE(clk_pfd_mux); i++) {
clk_pfd_mux[i].regmap = regmap;
ret = devm_clk_hw_register(dev, &clk_pfd_mux[i].hw);
if (ret)
return dev_err_probe(dev, ret, "%s failed\n",
clk_pfd_mux[i].hw.init->name);
}
and entries in clk_pfd_mux[] are like:
static struct vc3_hw_data clk_pfd_mux[] = {
[VC3_PFD2_MUX] = {
.data = &(struct vc3_clk_data) {
.offs = VC3_PLL_OP_CTRL,
.bitmsk = BIT(VC3_PLL_OP_CTRL_PLL2_REFIN_SEL)
},
.hw.init = &(struct clk_init_data) {
.name = "pfd2_mux",
.ops = &vc3_pfd_mux_ops,
.parent_data = pfd_mux_parent_data,
.num_parents = 2,
.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT
}
},
// ...
};
I tried with the following diff, too:
diff --git a/drivers/clk/clk-versaclock3.c b/drivers/clk/clk-versaclock3.c
index 9fe27dace111..c1776c313e9b 100644
--- a/drivers/clk/clk-versaclock3.c
+++ b/drivers/clk/clk-versaclock3.c
@@ -1022,10 +1022,11 @@ static int vc3_probe(struct i2c_client *client)
/* Register pfd muxes */
for (i = 0; i < ARRAY_SIZE(clk_pfd_mux); i++) {
clk_pfd_mux[i].regmap = regmap;
+ pr_err("%s(): hw init=%08x\n", __func__,
clk_pfd_mux[i].hw.init);
ret = devm_clk_hw_register(dev, &clk_pfd_mux[i].hw);
if (ret)
return dev_err_probe(dev, ret, "%s failed\n",
- clk_pfd_mux[i].hw.init->name);
+ "test");
//clk_pfd_mux[i].hw.init->name);
}
/* Register pfd's */
@@ -1034,7 +1035,7 @@ static int vc3_probe(struct i2c_client *client)
ret = devm_clk_hw_register(dev, &clk_pfd[i].hw);
if (ret)
return dev_err_probe(dev, ret, "%s failed\n",
- clk_pfd[i].hw.init->name);
+ "test");
//clk_pfd[i].hw.init->name);
}
data = i2c_get_match_data(client);
@@ -1050,7 +1051,7 @@ static int vc3_probe(struct i2c_client *client)
ret = devm_clk_hw_register(dev, &clk_pll[i].hw);
if (ret)
return dev_err_probe(dev, ret, "%s failed\n",
- clk_pll[i].hw.init->name);
+ "test");
//clk_pll[i].hw.init->name);
}
/* Register divider muxes */
@@ -1059,7 +1060,7 @@ static int vc3_probe(struct i2c_client *client)
ret = devm_clk_hw_register(dev, &clk_div_mux[i].hw);
if (ret)
return dev_err_probe(dev, ret, "%s failed\n",
- clk_div_mux[i].hw.init->name);
+
"test");//clk_div_mux[i].hw.init->name);
}
/* Register dividers */
@@ -1068,7 +1069,7 @@ static int vc3_probe(struct i2c_client *client)
ret = devm_clk_hw_register(dev, &clk_div[i].hw);
if (ret)
return dev_err_probe(dev, ret, "%s failed\n",
- clk_div[i].hw.init->name);
+ "test");
//clk_div[i].hw.init->name);
}
/* Register clk muxes */
@@ -1082,7 +1083,7 @@ static int vc3_probe(struct i2c_client *client)
ret = devm_clk_hw_register(dev, &clk_mux[i].hw);
if (ret)
return dev_err_probe(dev, ret, "%s failed\n",
- clk_mux[i].hw.init->name);
+
"test");//clk_mux[i].hw.init->name);
}
/* Register clk outputs */
Can you please let me know if this is a wrong pattern? I noticed there are
other drivers following similar approach, e.g.:
- drivers/clk/clk-axm5516.c
- drivers/clk/qcom/camcc-sm6350.c
- drivers/clk/meson/c3-pll.c
Full console log with my trials can be found here:
https://p.fr33tux.org/9d976f
Thank you,
Claudiu
Powered by blists - more mailing lists