ASoC: mediatek: mt8188: fix use-after-free in driver remove path

During mt8188_afe_init_clock(), mt8188_audsys_clk_register() was called
followed by several other devm functions. The caller of
mt8188_afe_init_clock() utilized devm_add_action_or_reset() to call
mt8188_afe_deinit_clock(). However, the order was incorrect, causing a
use-after-free issue during remove time.

At probe time, the order of calls was:
1. mt8188_audsys_clk_register
2. afe_priv->clk = devm_kcalloc
3. afe_priv->clk[i] = devm_clk_get

At remove time, the order of calls was:
1. mt8188_audsys_clk_unregister
3. free afe_priv->clk[i]
2. free afe_priv->clk

To resolve the problem, it's necessary to move devm_add_action_or_reset()
to the appropriate position so that the remove order can be 3->2->1.

Fixes: f6b026479b13 ("ASoC: mediatek: mt8188: support audio clock control")
Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Link: https://lore.kernel.org/r/20230601033318.10408-2-trevor.wu@mediatek.com
Signed-off-by: Mark Brown <broonie@kernel.org>

diff --git a/sound/soc/mediatek/mt8188/mt8188-afe-clk.c b/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
index 743d6a162cb9a871a196532e4b4af77fcd2039c4..0fb97517f82c6355546497d7a5bfa595395f1540 100644 (file)
--- a/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
+++ b/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
@@ -418,13 +418,6 @@ int mt8188_afe_init_clock(struct mtk_base_afe *afe)
        return 0;
 }
 
-void mt8188_afe_deinit_clock(void *priv)
-{
-       struct mtk_base_afe *afe = priv;
-
-       mt8188_audsys_clk_unregister(afe);
-}
-
 int mt8188_afe_enable_clk(struct mtk_base_afe *afe, struct clk *clk)
 {
        int ret;

diff --git a/sound/soc/mediatek/mt8188/mt8188-afe-clk.h b/sound/soc/mediatek/mt8188/mt8188-afe-clk.h
index 084fdfb1d877a42a8e2cb840a90c45eb1615b8fd..a4203a87a1e35138c4cbc279ae22f12fae53631a 100644 (file)
--- a/sound/soc/mediatek/mt8188/mt8188-afe-clk.h
+++ b/sound/soc/mediatek/mt8188/mt8188-afe-clk.h
@@ -100,7 +100,6 @@ int mt8188_afe_get_mclk_source_clk_id(int sel);
 int mt8188_afe_get_mclk_source_rate(struct mtk_base_afe *afe, int apll);
 int mt8188_afe_get_default_mclk_source_by_rate(int rate);
 int mt8188_afe_init_clock(struct mtk_base_afe *afe);
-void mt8188_afe_deinit_clock(void *priv);
 int mt8188_afe_enable_clk(struct mtk_base_afe *afe, struct clk *clk);
 void mt8188_afe_disable_clk(struct mtk_base_afe *afe, struct clk *clk);
 int mt8188_afe_set_clk_rate(struct mtk_base_afe *afe, struct clk *clk,

diff --git a/sound/soc/mediatek/mt8188/mt8188-afe-pcm.c b/sound/soc/mediatek/mt8188/mt8188-afe-pcm.c
index e5f9373bed562ecff59da0ae9da3c5163702c5ed..bcf7025886df4069d4e2841c100877feb40f21df 100644 (file)
--- a/sound/soc/mediatek/mt8188/mt8188-afe-pcm.c
+++ b/sound/soc/mediatek/mt8188/mt8188-afe-pcm.c
@@ -3185,10 +3185,6 @@ static int mt8188_afe_pcm_dev_probe(struct platform_device *pdev)
        if (ret)
                return dev_err_probe(dev, ret, "init clock error");
 
-       ret = devm_add_action_or_reset(dev, mt8188_afe_deinit_clock, (void *)afe);
-       if (ret)
-               return ret;
-
        spin_lock_init(&afe_priv->afe_ctrl_lock);
 
        mutex_init(&afe->irq_alloc_lock);

diff --git a/sound/soc/mediatek/mt8188/mt8188-audsys-clk.c b/sound/soc/mediatek/mt8188/mt8188-audsys-clk.c
index be1c53bf472987e7594d2668f81478da3c70c369..c796ad8b62eeaa929f24c09755f428116b105404 100644 (file)
--- a/sound/soc/mediatek/mt8188/mt8188-audsys-clk.c
+++ b/sound/soc/mediatek/mt8188/mt8188-audsys-clk.c
@@ -138,6 +138,29 @@ static const struct afe_gate aud_clks[CLK_AUD_NR_CLK] = {
        GATE_AUD6(CLK_AUD_GASRC11, "aud_gasrc11", "top_asm_h", 11),
 };
 
+static void mt8188_audsys_clk_unregister(void *data)
+{
+       struct mtk_base_afe *afe = data;
+       struct mt8188_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 mt8188_audsys_clk_register(struct mtk_base_afe *afe)
 {
        struct mt8188_afe_private *afe_priv = afe->platform_priv;
@@ -179,27 +202,5 @@ int mt8188_audsys_clk_register(struct mtk_base_afe *afe)
                afe_priv->lookup[i] = cl;
        }
 
-       return 0;
-}
-
-void mt8188_audsys_clk_unregister(struct mtk_base_afe *afe)
-{
-       struct mt8188_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);
-       }
+       return devm_add_action_or_reset(afe->dev, mt8188_audsys_clk_unregister, afe);
 }

diff --git a/sound/soc/mediatek/mt8188/mt8188-audsys-clk.h b/sound/soc/mediatek/mt8188/mt8188-audsys-clk.h
index 6c5f463ad7e4dd3808879c27ea3d934c450c5ad4..45b0948c4a06ee83a9d808b3e49e011180015f32 100644 (file)
--- a/sound/soc/mediatek/mt8188/mt8188-audsys-clk.h
+++ b/sound/soc/mediatek/mt8188/mt8188-audsys-clk.h
@@ -10,6 +10,5 @@
 #define _MT8188_AUDSYS_CLK_H_
 
 int mt8188_audsys_clk_register(struct mtk_base_afe *afe);
-void mt8188_audsys_clk_unregister(struct mtk_base_afe *afe);
 
 #endif