From patchwork Thu May 11 16:25:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 92734 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp4515866vqo; Thu, 11 May 2023 09:40:00 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7Lc0FdUDdkzMYmjG/9BnfNvOAj/ttCMpsG2MXWYfTRuF76sL1FKDqm4gaePblcn8jMYeKw X-Received: by 2002:a05:6a20:4314:b0:fa:4fc6:79b2 with SMTP id h20-20020a056a20431400b000fa4fc679b2mr26062547pzk.11.1683823200350; Thu, 11 May 2023 09:40:00 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683823200; cv=none; d=google.com; s=arc-20160816; b=KVPv/+BkXxQ254vHQ3cBMOSUoqyDp0pXJ/XDJ2vAdFkM4lsSjZTB5W4h8sE6Ftm41o c2lrrpIfd5EC6K/+qsHbMpK83HT7BQfMAZEWZSn33H8jAPEJ6PsUnGOeEdwCej5j5eM9 LrSBt6yVIZPV/nSc0p5h9HPxJGQOYfMeYfNiRC3ukVM+TNzdm0vdrA4Nyd8PrbOzR7M+ JMguWijYllvuB4PJCKZWNxrEETuXRGNk8V4qbykbC0umxHwIDeww6Y0dP7aEDml4Nbbi ObaYTRKRXZe1pWTe1G6odyu0nye5xJTjBJh5EkBDkvNfPaqhNdt+IyAiwhIamV6B6ZNG C0sQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=hMQTOddVJSa22SR6zcvBJmFXpgRZVXNpCl3DB1GYgq4=; b=s4HZ53jhS/hnnWksHLEvKB0d4MJGlUbgjcimxefvbmqWg+HiwrAE6ZoUYNhOSVLANJ OlUnNw/4x3duiGqBLVj9EC0+3XaXu1uLir0pPswKoZWXm3WISLsAjSjC/2FziOuHcSQV D9mJdiGms42DrB8w9UhIZ4otuceT3vbZQR56Dh/cj3jTMefajXbE0LKNS25CUPFvWDTu nB52rXC5G/wmZOALwP6al6vytT1geEjOZIRjBVUAbj+1jm2Cut2uNGYvIySKScvsYcdG XhWo5LVpB32M2UnBROHyy/CHMXV5m+AzOL2kfWduouE5NepOT0hAtFa0A+oPD2ofpKUf sbQg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=PmisfVof; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id n20-20020a637214000000b005131739af72si7109976pgc.755.2023.05.11.09.39.44; Thu, 11 May 2023 09:40:00 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=PmisfVof; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238327AbjEKQ1S (ORCPT + 99 others); Thu, 11 May 2023 12:27:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41730 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238130AbjEKQ1Q (ORCPT ); Thu, 11 May 2023 12:27:16 -0400 Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AAFA7DB for ; Thu, 11 May 2023 09:27:14 -0700 (PDT) Received: by mail-pf1-x436.google.com with SMTP id d2e1a72fcca58-64115eef620so59116970b3a.1 for ; Thu, 11 May 2023 09:27:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1683822434; x=1686414434; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=hMQTOddVJSa22SR6zcvBJmFXpgRZVXNpCl3DB1GYgq4=; b=PmisfVof65a4l9eAYcp8O+qzOBcGyWPCT4YUzP5gsLa3PQizXpgoIH+Sl0L2ly0Bvk Da1HdA2Z4zb7UsmM3Y8bBO8MaQpBSb+bgRmbew3sn8MgzDgcuNScF5MyME+mQ0GFhtsm ER5LtWKgDF+zGPeqEYWm1ufwRBywXWdJoU08w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683822434; x=1686414434; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=hMQTOddVJSa22SR6zcvBJmFXpgRZVXNpCl3DB1GYgq4=; b=e6QjI279g7NW4XkEb74jSI5nPxrnXRtqhIz5TEsm/mWWtzpfAyjzb8y2FA5L+poQo7 VE2pzZc6ggHsO58NeTeH5rtfcJHwIzSF4yh+Kg5k1SMVR6BKTtW8MbYuXbZQwkpwfMj9 BDnht1WyE0XlCaReOErWkKhA/xyxjLTap8/vKrMGg4qDkpHuuUO2Z46/9xk1c4TUXn+1 aFbwXG5Oi0p8EQVTHDgL3UjpXC1RIIUGWJ1IgonXzWtSdUdK5NBKKjI9mwJmrqo1sXHp T7fJtLegD6YsH+fno1+tFeQLO092HPNGe87Du/zCjE5GdJhG9VduKzQzCnXJcac/Ob3E zm8g== X-Gm-Message-State: AC+VfDxTNfiNvxrjdHYCWOYXnWgQtynAsIsveemsb77xIEYoJOSmSpvs 1vbBuNUugWdgxG5R794pTqE/nQ== X-Received: by 2002:a05:6a20:4311:b0:104:2d89:8f89 with SMTP id h17-20020a056a20431100b001042d898f89mr1138807pzk.23.1683822434073; Thu, 11 May 2023 09:27:14 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:9d:2:9915:1a0e:d2b7:87ef]) by smtp.gmail.com with ESMTPSA id g15-20020a63f40f000000b0053031f7a367sm5217008pgi.85.2023.05.11.09.27.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 May 2023 09:27:13 -0700 (PDT) From: Douglas Anderson To: Mark Brown , Jaroslav Kysela , Takashi Iwai , Matthias Brugger Cc: Douglas Anderson , AngeloGioacchino Del Regno , Chunxu Li , Jiaxin Yu , Liam Girdwood , alsa-devel@alsa-project.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org Subject: [PATCH] ASoC: mediatek: mt8186: Fix use-after-free in driver remove path Date: Thu, 11 May 2023 09:25:12 -0700 Message-ID: <20230511092437.1.I31cceffc8c45bb1af16eb613e197b3df92cdc19e@changeid> X-Mailer: git-send-email 2.40.1.606.ga4b1b128d6-goog MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765616596251282972?= X-GMAIL-MSGID: =?utf-8?q?1765616596251282972?= 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: 55b423d5623c ("ASoC: mediatek: mt8186: support audio clock control in platform driver") Signed-off-by: Douglas Anderson --- sound/soc/mediatek/mt8186/mt8186-afe-clk.c | 6 --- sound/soc/mediatek/mt8186/mt8186-afe-clk.h | 1 - sound/soc/mediatek/mt8186/mt8186-afe-pcm.c | 4 -- sound/soc/mediatek/mt8186/mt8186-audsys-clk.c | 46 ++++++++++--------- sound/soc/mediatek/mt8186/mt8186-audsys-clk.h | 1 - 5 files changed, 24 insertions(+), 34 deletions(-) diff --git a/sound/soc/mediatek/mt8186/mt8186-afe-clk.c b/sound/soc/mediatek/mt8186/mt8186-afe-clk.c index a6b4f29049bb..539e3a023bc4 100644 --- a/sound/soc/mediatek/mt8186/mt8186-afe-clk.c +++ b/sound/soc/mediatek/mt8186/mt8186-afe-clk.c @@ -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); -} diff --git a/sound/soc/mediatek/mt8186/mt8186-afe-clk.h b/sound/soc/mediatek/mt8186/mt8186-afe-clk.h index d5988717d8f2..a9d59e506d9a 100644 --- a/sound/soc/mediatek/mt8186/mt8186-afe-clk.h +++ b/sound/soc/mediatek/mt8186/mt8186-afe-clk.h @@ -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); diff --git a/sound/soc/mediatek/mt8186/mt8186-afe-pcm.c b/sound/soc/mediatek/mt8186/mt8186-afe-pcm.c index 41172a82103e..a868a04ed4e7 100644 --- a/sound/soc/mediatek/mt8186/mt8186-afe-pcm.c +++ b/sound/soc/mediatek/mt8186/mt8186-afe-pcm.c @@ -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; diff --git a/sound/soc/mediatek/mt8186/mt8186-audsys-clk.c b/sound/soc/mediatek/mt8186/mt8186-audsys-clk.c index 578969ca91c8..5666be6b1bd2 100644 --- a/sound/soc/mediatek/mt8186/mt8186-audsys-clk.c +++ b/sound/soc/mediatek/mt8186/mt8186-audsys-clk.c @@ -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); - } -} diff --git a/sound/soc/mediatek/mt8186/mt8186-audsys-clk.h b/sound/soc/mediatek/mt8186/mt8186-audsys-clk.h index b8d6a06e11e8..897a2914dc19 100644 --- a/sound/soc/mediatek/mt8186/mt8186-audsys-clk.h +++ b/sound/soc/mediatek/mt8186/mt8186-audsys-clk.h @@ -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