From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7A64CEE4996 for ; Mon, 21 Aug 2023 03:36:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=GHaXLb9Tyv5ZkoeO0hR1ApIB04/3uVkhlSjMwdJNg5o=; b=3nYjawQQLjIi1k +2zSq8w38H41j+tgqdDvKlYVSJ/sEYAIhLrtWgqc/TSdu7Y0y9kH+CIMvws1pY9wx6xemYJOUJPQ1 yoA457BXsCeetx4qHjX/R4O+aJr7PrR6bSg0Fw0oaIvWL4TwcvqI7nRoHZJ7ZvCyZGEDG5uRDHMcf VtsrBpdGfo2tLP4YuiukEZQxMFyLSH2oYhXIF6D8Jw2awNHE9A+U1EnGqC2s2VbOjeLVKEmkkg2u1 L0Q4TFf228erMnbAG3olzKnvxgb3eh/d6w8PHs0s4Jh1AibZskcp1jQXW1bJ3rKyes0W5wR4JX/5x sp3VIfaBnhkOAo5OUtIQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qXvhd-00Czp0-08; Mon, 21 Aug 2023 03:35:41 +0000 Received: from mail-pl1-x62f.google.com ([2607:f8b0:4864:20::62f]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qXvhZ-00CzoA-24 for linux-arm-kernel@lists.infradead.org; Mon, 21 Aug 2023 03:35:38 +0000 Received: by mail-pl1-x62f.google.com with SMTP id d9443c01a7336-1bc3d94d40fso22339975ad.3 for ; Sun, 20 Aug 2023 20:35:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1692588936; x=1693193736; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=00sTKj6r29idEJKO1tqRjR2HLLSE0fyMr4hXBqiW47I=; b=EoyOxijThar9EAUzeD2tdAYhkNhVBKLl5DvbKipB5sV6cz6R9mEcIg+4Z8Z9okgZ2k YeZ2+ayHnz4AcanYdoMscZoQpZ7Te1XBrw3p9RyKA66C2vIxkEcpKYICCoALmRS3VlTM iRe2QuQTxYpQJTh5Hz17W91X7F5RoYaXw1JaQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692588936; x=1693193736; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=00sTKj6r29idEJKO1tqRjR2HLLSE0fyMr4hXBqiW47I=; b=JTBmttZaH1xfCMEbo83NXZ5zOFQC/ZJgXHwJwsdoW/dnmhU9/LGAlmqBNZ6ueHRUwJ 0p6IYlyHcXstskG9KVujSbwR86xFp3Sg0H/CAK0GLlL9E/e7JsDO/AkK2E4Y/TOgFIue RgjCZBStPETbplh9PwJAtUOknC9v19qo/zWMsUxiX2n4O1a3dwPc3KWHVAo2ylplEEzk qB3yLLgyW2CcFBxNUnmViwjHDnnBjA85PmFyNqoyK/AuR8HDNQn3vt3sb5UbCCOaDO3l 28P7cwzFNEqQhm2u43i5tgv26MM310Gw9xaV/UMrnhGou+e7+jAARGWL7yLbQDlIsEW1 /YDA== X-Gm-Message-State: AOJu0Yz+47Sqhi4tmsLG5Qxy9o9OD4HwfNOldgX2oz8LFTWmsAijdCag fljLjsQSNud4KwCITiPGA5j5fw== X-Google-Smtp-Source: AGHT+IG5aGqs5D00DDjj7OJwcEOzDcmv6vQD+bo07/gyCLmBibr72UHzCi3wAuI/jaEHXPiLMS1QHg== X-Received: by 2002:a17:903:2305:b0:1bf:cdc:f402 with SMTP id d5-20020a170903230500b001bf0cdcf402mr7301478plh.48.1692588935784; Sun, 20 Aug 2023 20:35:35 -0700 (PDT) Received: from google.com ([2401:fa00:1:10:d88c:8ac2:1586:6240]) by smtp.gmail.com with ESMTPSA id iz22-20020a170902ef9600b001b9da42cd7dsm5881881plb.279.2023.08.20.20.35.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 20 Aug 2023 20:35:35 -0700 (PDT) Date: Mon, 21 Aug 2023 11:35:32 +0800 From: Chen-Yu Tsai To: Yu-Che Cheng , Stephen Boyd Cc: James Lo , Stephen Boyd , Matthias Brugger , Fei Shao , AngeloGioacchino Del Regno , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org Subject: Re: [PATCH] spmi: mediatek: Fix UAF on device remove Message-ID: <20230821033532.GA21555@google.com> References: <20230717173934.1.If004a6e055a189c7f2d0724fa814422c26789839@changeid> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230717173934.1.If004a6e055a189c7f2d0724fa814422c26789839@changeid> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230820_203537_677986_34B75F81 X-CRM114-Status: GOOD ( 25.44 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Jul 17, 2023 at 05:39:35PM +0800, Yu-Che Cheng wrote: > The pmif driver data that contains the clocks is allocated along with > spmi_controller. > On device remove, spmi_controller will be freed first, and then devres > , including the clocks, will be cleanup. > This leads to UAF because putting the clocks will access the clocks in > the pmif driver data, which is already freed along with spmi_controller. > > This can be reproduced by enabling DEBUG_TEST_DRIVER_REMOVE and > building the kernel with KASAN. > > Fix the UAF issue by using unmanaged clk_bulk_get() and putting the > clocks before freeing spmi_controller. > > Reported-by: Fei Shao > Signed-off-by: Yu-Che Cheng Reviewed-by: Chen-Yu Tsai Stephen, could you pick this up? > --- > > drivers/spmi/spmi-mtk-pmif.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/spmi/spmi-mtk-pmif.c b/drivers/spmi/spmi-mtk-pmif.c > index b3c991e1ea40..74b73f9bc222 100644 > --- a/drivers/spmi/spmi-mtk-pmif.c > +++ b/drivers/spmi/spmi-mtk-pmif.c > @@ -465,7 +465,7 @@ static int mtk_spmi_probe(struct platform_device *pdev) > for (i = 0; i < arb->nclks; i++) > arb->clks[i].id = pmif_clock_names[i]; > > - err = devm_clk_bulk_get(&pdev->dev, arb->nclks, arb->clks); > + err = clk_bulk_get(&pdev->dev, arb->nclks, arb->clks); > if (err) { > dev_err(&pdev->dev, "Failed to get clocks: %d\n", err); > goto err_put_ctrl; > @@ -474,7 +474,7 @@ static int mtk_spmi_probe(struct platform_device *pdev) > err = clk_bulk_prepare_enable(arb->nclks, arb->clks); > if (err) { > dev_err(&pdev->dev, "Failed to enable clocks: %d\n", err); > - goto err_put_ctrl; > + goto err_put_clks; > } > > ctrl->cmd = pmif_arb_cmd; > @@ -498,6 +498,8 @@ static int mtk_spmi_probe(struct platform_device *pdev) > > err_domain_remove: > clk_bulk_disable_unprepare(arb->nclks, arb->clks); > +err_put_clks: > + clk_bulk_put(arb->nclks, arb->clks); > err_put_ctrl: > spmi_controller_put(ctrl); > return err; > @@ -509,6 +511,7 @@ static void mtk_spmi_remove(struct platform_device *pdev) > struct pmif *arb = spmi_controller_get_drvdata(ctrl); > > clk_bulk_disable_unprepare(arb->nclks, arb->clks); > + clk_bulk_put(arb->nclks, arb->clks); > spmi_controller_remove(ctrl); > spmi_controller_put(ctrl); Maybe we need devres versions of spmi_controller_alloc and spmi_controller_add to avoid this in the future? I'm sure drivers put all sorts of stuff in their private data. ChenYu > } > -- > 2.41.0.255.g8b1d071c50-goog > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel