From patchwork Wed Nov 11 19:07:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 11898349 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5DFDB921 for ; Wed, 11 Nov 2020 19:12:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4616B20797 for ; Wed, 11 Nov 2020 19:12:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726036AbgKKTMV (ORCPT ); Wed, 11 Nov 2020 14:12:21 -0500 Received: from mailout3.hostsharing.net ([176.9.242.54]:54639 "EHLO mailout3.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725949AbgKKTMU (ORCPT ); Wed, 11 Nov 2020 14:12:20 -0500 Received: from h08.hostsharing.net (h08.hostsharing.net [IPv6:2a01:37:1000::53df:5f1c:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.hostsharing.net", Issuer "COMODO RSA Domain Validation Secure Server CA" (not verified)) by mailout3.hostsharing.net (Postfix) with ESMTPS id 6A38B101E6B62; Wed, 11 Nov 2020 20:12:18 +0100 (CET) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 3656960E6357; Wed, 11 Nov 2020 20:12:18 +0100 (CET) X-Mailbox-Line: From 5e31a9a59fd1c0d0b795b2fe219f25e5ee855f9d Mon Sep 17 00:00:00 2001 Message-Id: <5e31a9a59fd1c0d0b795b2fe219f25e5ee855f9d.1605121038.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Wed, 11 Nov 2020 20:07:40 +0100 Subject: [PATCH 4/4] spi: bcm-qspi: Fix use-after-free on unbind To: Mark Brown , Florian Fainelli Cc: linux-spi@vger.kernel.org, Vladimir Oltean , Sascha Hauer , Kamal Dasu Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org bcm_qspi_remove() calls spi_unregister_master() even though bcm_qspi_probe() calls devm_spi_register_master(). The spi_master is therefore unregistered and freed twice on unbind. Moreover, since commit 0392727c261b ("spi: bcm-qspi: Handle clock probe deferral"), bcm_qspi_probe() leaks the spi_master allocation if the call to devm_clk_get_optional() fails. Fix by switching over to the new devm_spi_alloc_master() helper which keeps the private data accessible until the driver has unbound and also avoids the spi_master leak on probe. While at it, fix an ordering issue in bcm_qspi_remove() wherein spi_unregister_master() is called after uninitializing the hardware, disabling the clock and freeing an IRQ data structure. The correct order is to call spi_unregister_master() *before* those teardown steps because bus accesses may still be ongoing until that function returns. Fixes: fa236a7ef240 ("spi: bcm-qspi: Add Broadcom MSPI driver") Signed-off-by: Lukas Wunner Cc: # v4.9+: 123456789abc: spi: Introduce device-managed SPI controller allocation Cc: # v4.9+ Cc: Kamal Dasu Acked-by: Florian Fainelli Tested-by: Florian Fainelli --- drivers/spi/spi-bcm-qspi.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c index 14c9d0133bce..c028446c7460 100644 --- a/drivers/spi/spi-bcm-qspi.c +++ b/drivers/spi/spi-bcm-qspi.c @@ -1327,7 +1327,7 @@ int bcm_qspi_probe(struct platform_device *pdev, data = of_id->data; - master = spi_alloc_master(dev, sizeof(struct bcm_qspi)); + master = devm_spi_alloc_master(dev, sizeof(struct bcm_qspi)); if (!master) { dev_err(dev, "error allocating spi_master\n"); return -ENOMEM; @@ -1367,21 +1367,17 @@ int bcm_qspi_probe(struct platform_device *pdev, if (res) { qspi->base[MSPI] = devm_ioremap_resource(dev, res); - if (IS_ERR(qspi->base[MSPI])) { - ret = PTR_ERR(qspi->base[MSPI]); - goto qspi_resource_err; - } + if (IS_ERR(qspi->base[MSPI])) + return PTR_ERR(qspi->base[MSPI]); } else { - goto qspi_resource_err; + return 0; } res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "bspi"); if (res) { qspi->base[BSPI] = devm_ioremap_resource(dev, res); - if (IS_ERR(qspi->base[BSPI])) { - ret = PTR_ERR(qspi->base[BSPI]); - goto qspi_resource_err; - } + if (IS_ERR(qspi->base[BSPI])) + return PTR_ERR(qspi->base[BSPI]); qspi->bspi_mode = true; } else { qspi->bspi_mode = false; @@ -1392,18 +1388,14 @@ int bcm_qspi_probe(struct platform_device *pdev, res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cs_reg"); if (res) { qspi->base[CHIP_SELECT] = devm_ioremap_resource(dev, res); - if (IS_ERR(qspi->base[CHIP_SELECT])) { - ret = PTR_ERR(qspi->base[CHIP_SELECT]); - goto qspi_resource_err; - } + if (IS_ERR(qspi->base[CHIP_SELECT])) + return PTR_ERR(qspi->base[CHIP_SELECT]); } qspi->dev_ids = kcalloc(num_irqs, sizeof(struct bcm_qspi_dev_id), GFP_KERNEL); - if (!qspi->dev_ids) { - ret = -ENOMEM; - goto qspi_resource_err; - } + if (!qspi->dev_ids) + return -ENOMEM; for (val = 0; val < num_irqs; val++) { irq = -1; @@ -1484,7 +1476,7 @@ int bcm_qspi_probe(struct platform_device *pdev, qspi->xfer_mode.addrlen = -1; qspi->xfer_mode.hp = -1; - ret = devm_spi_register_master(&pdev->dev, master); + ret = spi_register_master(master); if (ret < 0) { dev_err(dev, "can't register master\n"); goto qspi_reg_err; @@ -1497,8 +1489,6 @@ int bcm_qspi_probe(struct platform_device *pdev, clk_disable_unprepare(qspi->clk); qspi_probe_err: kfree(qspi->dev_ids); -qspi_resource_err: - spi_master_put(master); return ret; } /* probe function to be called by SoC specific platform driver probe */ @@ -1508,10 +1498,10 @@ int bcm_qspi_remove(struct platform_device *pdev) { struct bcm_qspi *qspi = platform_get_drvdata(pdev); + spi_unregister_master(qspi->master); bcm_qspi_hw_uninit(qspi); clk_disable_unprepare(qspi->clk); kfree(qspi->dev_ids); - spi_unregister_master(qspi->master); return 0; }