Commit a93d2afd authored by Douglas Anderson's avatar Douglas Anderson Committed by Mark Brown

ASoC: mediatek: mt8186: Fix use-after-free in driver remove path

When devm runs function in the "remove" path for a device it runs them
in the reverse order. That means that if you have parts of your driver
that aren't using devm or are using "roll your own" devm w/
devm_add_action_or_reset() you need to keep that in mind.

The mt8186 audio driver didn't quite get this right. Specifically, in
mt8186_init_clock() it called mt8186_audsys_clk_register() and then
went on to call a bunch of other devm function. The caller of
mt8186_init_clock() used devm_add_action_or_reset() to call
mt8186_deinit_clock() but, because of the intervening devm functions,
the order was wrong.

Specifically at probe time, the order was:
1. mt8186_audsys_clk_register()
2. afe_priv->clk = devm_kcalloc(...)
3. afe_priv->clk[i] = devm_clk_get(...)

At remove time, the order (which should have been 3, 2, 1) was:
1. mt8186_audsys_clk_unregister()
3. Free all of afe_priv->clk[i]
2. Free afe_priv->clk

The above seemed to be causing a use-after-free. Luckily, it's easy to
fix this by simply using devm more correctly. Let's move the
devm_add_action_or_reset() to the right place. In addition to fixing
the use-after-free, code inspection shows that this fixes a leak
(missing call to mt8186_audsys_clk_unregister()) that would have
happened if any of the syscon_regmap_lookup_by_phandle() calls in
mt8186_init_clock() had failed.

Fixes: 55b423d5 ("ASoC: mediatek: mt8186: support audio clock control in platform driver")
Signed-off-by: Douglas Anderson <dianders@chromium.org
Link: https://lore.kernel.org/r/20230511092437.1.I31cceffc8c45bb1af16eb613e197b3df92cdc19e@changeid
Signed-off-by: Mark Brown <broonie@kernel.org
parent 4708449e
......@@ -644,9 +644,3 @@ int mt8186_init_clock(struct mtk_base_afe *afe)
return 0;
}
void mt8186_deinit_clock(void *priv)
{
struct mtk_base_afe *afe = priv;
mt8186_audsys_clk_unregister(afe);
}
......@@ -81,7 +81,6 @@ enum {
struct mtk_base_afe;
int mt8186_set_audio_int_bus_parent(struct mtk_base_afe *afe, int clk_id);
int mt8186_init_clock(struct mtk_base_afe *afe);
void mt8186_deinit_clock(void *priv);
int mt8186_afe_enable_cgs(struct mtk_base_afe *afe);
void mt8186_afe_disable_cgs(struct mtk_base_afe *afe);
int mt8186_afe_enable_clock(struct mtk_base_afe *afe);
......
......@@ -2848,10 +2848,6 @@ static int mt8186_afe_pcm_dev_probe(struct platform_device *pdev)
return ret;
}
ret = devm_add_action_or_reset(dev, mt8186_deinit_clock, (void *)afe);
if (ret)
return ret;
/* init memif */
afe->memif_32bit_supported = 0;
afe->memif_size = MT8186_MEMIF_NUM;
......
......@@ -84,6 +84,29 @@ static const struct afe_gate aud_clks[CLK_AUD_NR_CLK] = {
GATE_AUD2(CLK_AUD_ETDM_OUT1_BCLK, "aud_etdm_out1_bclk", "top_audio", 24),
};
static void mt8186_audsys_clk_unregister(void *data)
{
struct mtk_base_afe *afe = data;
struct mt8186_afe_private *afe_priv = afe->platform_priv;
struct clk *clk;
struct clk_lookup *cl;
int i;
if (!afe_priv)
return;
for (i = 0; i < CLK_AUD_NR_CLK; i++) {
cl = afe_priv->lookup[i];
if (!cl)
continue;
clk = cl->clk;
clk_unregister_gate(clk);
clkdev_drop(cl);
}
}
int mt8186_audsys_clk_register(struct mtk_base_afe *afe)
{
struct mt8186_afe_private *afe_priv = afe->platform_priv;
......@@ -124,27 +147,6 @@ int mt8186_audsys_clk_register(struct mtk_base_afe *afe)
afe_priv->lookup[i] = cl;
}
return 0;
return devm_add_action_or_reset(afe->dev, mt8186_audsys_clk_unregister, afe);
}
void mt8186_audsys_clk_unregister(struct mtk_base_afe *afe)
{
struct mt8186_afe_private *afe_priv = afe->platform_priv;
struct clk *clk;
struct clk_lookup *cl;
int i;
if (!afe_priv)
return;
for (i = 0; i < CLK_AUD_NR_CLK; i++) {
cl = afe_priv->lookup[i];
if (!cl)
continue;
clk = cl->clk;
clk_unregister_gate(clk);
clkdev_drop(cl);
}
}
......@@ -10,6 +10,5 @@
#define _MT8186_AUDSYS_CLK_H_
int mt8186_audsys_clk_register(struct mtk_base_afe *afe);
void mt8186_audsys_clk_unregister(struct mtk_base_afe *afe);
#endif
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment