linux-kernel - [PATCH 4.19 059/116] spi: gpio: Dont leak SPI master in probe error path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210531130642.176191748@linuxfoundation.org>
Date:   Mon, 31 May 2021 15:13:55 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, Lukas Wunner <lukas@...ner.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        Navid Emamdoost <navid.emamdoost@...il.com>,
        Andrey Smirnov <andrew.smirnov@...il.com>,
        Mark Brown <broonie@...nel.org>
Subject: [PATCH 4.19 059/116] spi: gpio: Dont leak SPI master in probe error path

From: Lukas Wunner <lukas@...ner.de>

commit 7174dc655ef0578877b0b4598e69619d2be28b4d upstream.

If the call to devm_spi_register_master() fails on probe of the GPIO SPI
driver, the spi_master struct is erroneously not freed:

After allocating the spi_master, its reference count is 1.  The driver
unconditionally decrements the reference count on unbind using a devm
action.  Before calling devm_spi_register_master(), the driver
unconditionally increments the reference count because on success,
that function will decrement the reference count on unbind.  However on
failure, devm_spi_register_master() does *not* decrement the reference
count, so the spi_master is leaked.

The issue was introduced by commits 8b797490b4db ("spi: gpio: Make sure
spi_master_put() is called in every error path") and 79567c1a321e ("spi:
gpio: Use devm_spi_register_master()"), which sought to plug leaks
introduced by 9b00bc7b901f ("spi: spi-gpio: Rewrite to use GPIO
descriptors") but missed this remaining leak.

The situation was later aggravated by commit d3b0ffa1d75d ("spi: gpio:
prevent memory leak in spi_gpio_probe"), which introduced a
use-after-free because it releases a reference on the spi_master if
devm_add_action_or_reset() fails even though the function already
does that.

Fix by switching over to the new devm_spi_alloc_master() helper.

Fixes: 9b00bc7b901f ("spi: spi-gpio: Rewrite to use GPIO descriptors")
Signed-off-by: Lukas Wunner <lukas@...ner.de>
Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
Cc: <stable@...r.kernel.org> # v4.17+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@...r.kernel.org> # v5.1-: 8b797490b4db: spi: gpio: Make sure spi_master_put() is called in every error path
Cc: <stable@...r.kernel.org> # v5.1-: 45beec351998: spi: bitbang: Introduce spi_bitbang_init()
Cc: <stable@...r.kernel.org> # v5.1-: 79567c1a321e: spi: gpio: Use devm_spi_register_master()
Cc: <stable@...r.kernel.org> # v5.4-: d3b0ffa1d75d: spi: gpio: prevent memory leak in spi_gpio_probe
Cc: <stable@...r.kernel.org> # v4.17+
Cc: Navid Emamdoost <navid.emamdoost@...il.com>
Cc: Andrey Smirnov <andrew.smirnov@...il.com>
Link: https://lore.kernel.org/r/86eaed27431c3d709e3748eb76ceecbfc790dd37.1607286887.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@...nel.org>
[lukas: backport to v4.19.192]
Signed-off-by: Lukas Wunner <lukas@...ner.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
 drivers/spi/spi-gpio.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

--- a/drivers/spi/spi-gpio.c
+++ b/drivers/spi/spi-gpio.c
@@ -382,7 +382,7 @@ static int spi_gpio_probe(struct platfor
 		return -ENODEV;
 #endif

-	master = spi_alloc_master(&pdev->dev, sizeof(*spi_gpio));
+	master = devm_spi_alloc_master(&pdev->dev, sizeof(*spi_gpio));
 	if (!master)
 		return -ENOMEM;

@@ -438,11 +438,7 @@ static int spi_gpio_probe(struct platfor
 	}
 	spi_gpio->bitbang.setup_transfer = spi_bitbang_setup_transfer;

-	status = spi_bitbang_start(&spi_gpio->bitbang);
-	if (status)
-		spi_master_put(master);
-
-	return status;
+	return spi_bitbang_start(&spi_gpio->bitbang);
 }

 static int spi_gpio_remove(struct platform_device *pdev)