Skip to contentSkip to conversation

FAILED: patch "[PATCH] spi: gpio: Don't leak SPI master in probe error path" failed to apply to 4.19-stable tree

PATCH [email protected] 2 participants 2 messages Dec 28, 2020 – May 29, 2021

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 7174dc655ef0578877b0b4598e69619d2be28b4d Mon Sep 17 00:00:00 2001
From: Lukas Wunner <[email protected]>
Date: Mon, 7 Dec 2020 09:17:09 +0100
Subject: [PATCH] spi: gpio: Don't leak SPI master in probe error path

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")
Cc <[email protected]> # v4.17+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc <[email protected]> # v5.1-: 8b797490b4db: spi: gpio: Make sure spi_master_put() is called in every error path
Cc <[email protected]> # v5.1-: 45beec351998: spi: bitbang: Introduce spi_bitbang_init()
Cc <[email protected]> # v5.1-: 79567c1a321e: spi: gpio: Use devm_spi_register_master()
Cc <[email protected]> # v5.4-: d3b0ffa1d75d: spi: gpio: prevent memory leak in spi_gpio_probe
Cc <[email protected]> # v4.17+
Cc Navid Emamdoost <[email protected]>
Cc Andrey Smirnov <[email protected]>
drivers/spi/spi-gpio.c +2 -13
index 7ceb0ba27b75..0584f4d2fde2 100644
@@ -350,11 +350,6 @@ static int spi_gpio_probe_pdata(struct platform_device *pdev,
 	return 0;
 }
 
-static void spi_gpio_put(void *data)
-{
-	spi_master_put(data);
-}
-
 static int spi_gpio_probe(struct platform_device *pdev)
 {
 	int				status;
@@ -363,16 +358,10 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	struct device			*dev = &pdev->dev;
 	struct spi_bitbang		*bb;
 
-	master = spi_alloc_master(dev, sizeof(*spi_gpio));
+	master = devm_spi_alloc_master(dev, sizeof(*spi_gpio));
 	if (!master)
 		return -ENOMEM;
 
-	status = devm_add_action_or_reset(&pdev->dev, spi_gpio_put, master);
-	if (status) {
-		spi_master_put(master);
-		return status;
-	}
-
 	if (pdev->dev.of_node)
 		status = spi_gpio_probe_dt(pdev, master);
 	else
@@ -432,7 +421,7 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	if (status)
 		return status;
 
-	return devm_spi_register_master(&pdev->dev, spi_master_get(master));
+	return devm_spi_register_master(&pdev->dev, master);
 }
 
 MODULE_ALIAS("platform:" DRIVER_NAME);
Reviewed-by Linus Walleij
#1 lore raw
Quoted 4 lines · On Mon, Dec 28, 2020 at 11:59:19AM +0100, [email protected] wrote:

The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Here's the backport of 7174dc655ef0 to v4.19.192:

-- >8 --
From: Lukas Wunner <[email protected]>
Date: Mon, 7 Dec 2020 09:17:09 +0100
Subject: [PATCH] spi: gpio: Don't leak SPI master in probe error path

commit 7174dc655ef0578877b0b4598e69619d2be28b4d upstream.

If the calls to devm_kcalloc() or spi_gpio_request() fail on probe of the GPIO SPI driver, the spi_master struct is erroneously not freed because the required calls to spi_master_put() are missing.

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

Fixes 9b00bc7b901f ("spi: spi-gpio: Rewrite to use GPIO descriptors")
Cc <[email protected]> # v4.17+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc <[email protected]> # v4.17+
Cc Navid Emamdoost <[email protected]>
Cc Andrey Smirnov <[email protected]>
[lukas: backport to v4.19.192]
---
 drivers/spi/spi-gpio.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
drivers/spi/spi-gpio.c +2 -6
index 77838d8fd9bb..341d2953d7fc 100644
@@ -382,7 +382,7 @@ static int spi_gpio_probe(struct platform_device *pdev)
 		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 platform_device *pdev)
 	}
 	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)
Signature · 1 line
2.31.1