[PATCH] spi: fix reference leak in spi_register_controller()

[PATCH] spi: fix reference leak in spi_register_controller()

[Ma Ke]

Once device_add() failed, we should call put_device() to decrement
reference count for cleanup. Or it could cause memory leak.

Found by code review.

Cc: stable@vger.kernel.org
Fixes: f9981d4f50b4 ("spi: spi_register_controller(): free bus id on error paths")
Signed-off-by: Ma Ke <make_ruc2021@163.com>
---
 drivers/spi/spi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ff1add2ecb91..6477c2f4ae82 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3339,8 +3339,10 @@ int spi_register_controller(struct spi_controller *ctlr)
 		ctlr->last_cs[idx] = SPI_INVALID_CS;
 
 	status = device_add(&ctlr->dev);
-	if (status < 0)
+	if (status < 0) {
+		put_device(&ctlr->dev);
 		goto free_bus_id;
+	}
 	dev_dbg(dev, "registered %s %s\n",
 			spi_controller_is_target(ctlr) ? "target" : "host",
 			dev_name(&ctlr->dev));
@@ -3356,6 +3358,7 @@ int spi_register_controller(struct spi_controller *ctlr)
 		status = spi_controller_initialize_queue(ctlr);
 		if (status) {
 			device_del(&ctlr->dev);
+			put_device(&ctlr->dev);
 			goto free_bus_id;
 		}
 	}
-- 
2.25.1

Re: [PATCH] spi: fix reference leak in spi_register_controller()

[Markus Elfring]

> Once device_add() failed, we should call put_device() to decrement
> reference count for cleanup. Or it could cause memory leak.

Is there a need to integrate the added function call better into
a goto chain?

Regards,
Markus

Re: [PATCH] spi: fix reference leak in spi_register_controller()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

On Tue, Dec 17, 2024 at 03:31:33PM +0800, Ma Ke wrote:
> Once device_add() failed, we should call put_device() to decrement
> reference count for cleanup. Or it could cause memory leak.
> 
> Found by code review.

This isn't clear to me, spi_register_controller() is analogous to
device_add() and __spi_alloc_controller() is analogous to (and wraps)
device_initialize() so I'd expect the _put() to be done by the caller
calling spi_controller_put() either directly or more usually via devm.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]