| Message ID | 773fc8425c3b8f5b0ca7c1d89f15b65831a85ca9.1705850155.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | clk: hisilicon: hi3559a: Fix an erroneous devm_kfree() | expand |
Quoting Christophe JAILLET (2024-01-21 07:16:24) > 'p_clk' is an array allocated just before the for loop for all clk that > need to be registered. > It is incremented at each loop iteration. > > If a clk_register() call fails, 'p_clk' may point to something different > from what should be freed. > > The best we can do, is to avoid this wrong release of memory. > > Fixes: 6c81966107dc ("clk: hisilicon: Add clock driver for hi3559A SoC") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- Applied to clk-next About doing the right thing, it seems OK to remove the free for now because the code continues on anyway.
diff --git a/drivers/clk/hisilicon/clk-hi3559a.c b/drivers/clk/hisilicon/clk-hi3559a.c index ff4ca0edce06..4623befafaec 100644 --- a/drivers/clk/hisilicon/clk-hi3559a.c +++ b/drivers/clk/hisilicon/clk-hi3559a.c @@ -491,7 +491,6 @@ static void hisi_clk_register_pll(struct hi3559av100_pll_clock *clks, clk = clk_register(NULL, &p_clk->hw); if (IS_ERR(clk)) { - devm_kfree(dev, p_clk); dev_err(dev, "%s: failed to register clock %s\n", __func__, clks[i].name); continue;
'p_clk' is an array allocated just before the for loop for all clk that need to be registered. It is incremented at each loop iteration. If a clk_register() call fails, 'p_clk' may point to something different from what should be freed. The best we can do, is to avoid this wrong release of memory. Fixes: 6c81966107dc ("clk: hisilicon: Add clock driver for hi3559A SoC") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- devm_kfree() is clearly wrong (IMHO :)), but just removing it is maybe not the best solution. Should hisi_clk_register_pll() return an error? Should some data->clk_data.clks[clks[i].id] be set to NULL? (hisi_clk_alloc() doesn't zero the allocated memory) Should hisi_clk_alloc() use kzalloc() when allocating clk_table? --- drivers/clk/hisilicon/clk-hi3559a.c | 1 - 1 file changed, 1 deletion(-)