Skip to contentSkip to conversation

wilc1000: fix double free error in probe()

PATCH In review Dan Carpenter 3 participants 3 messages Dec 17, 2021 – Dec 20, 2021

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 wilc1000: fix double free error in probe()

Smatch complains that there is a double free in probe:

drivers/net/wireless/microchip/wilc1000/spi.c:186 wilc_bus_probe() error: double free of 'spi_priv' drivers/net/wireless/microchip/wilc1000/sdio.c:163 wilc_sdio_probe() error: double free of 'sdio_priv'

The problem is that wilc_netdev_cleanup() function frees "wilc->bus_data". That's confusing and a layering violation. Leave the frees in probe(), delete the free in wilc_netdev_cleanup(), and add some new frees to the remove() functions.

Fixes dc8b338f3bcd ("wilc1000: use goto labels on error path")
---
 drivers/net/wireless/microchip/wilc1000/netdev.c | 1 -
 drivers/net/wireless/microchip/wilc1000/sdio.c   | 2 ++
 drivers/net/wireless/microchip/wilc1000/spi.c    | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)
drivers/net/wireless/microchip/wilc1000/netdev.c +0 -1
index 03e3485d7e7f..643bddaae32a 100644
@@ -905,7 +905,6 @@ void wilc_netdev_cleanup(struct wilc *wilc)
 
 	wilc_wlan_cfg_deinit(wilc);
 	wlan_deinit_locks(wilc);
-	kfree(wilc->bus_data);
 	wiphy_unregister(wilc->wiphy);
 	wiphy_free(wilc->wiphy);
 }
drivers/net/wireless/microchip/wilc1000/sdio.c +2 -0
index 26ebf6664342..ec595dbd8959 100644
@@ -167,9 +167,11 @@ static int wilc_sdio_probe(struct sdio_func *func,
 static void wilc_sdio_remove(struct sdio_func *func)
 {
 	struct wilc *wilc = sdio_get_drvdata(func);
+	struct wilc_sdio *sdio_priv = wilc->bus_data;
 
 	clk_disable_unprepare(wilc->rtc_clk);
 	wilc_netdev_cleanup(wilc);
+	kfree(sdio_priv);
 }
 
 static int wilc_sdio_reset(struct wilc *wilc)
drivers/net/wireless/microchip/wilc1000/spi.c +2 -0
index e0871b89917d..5ace9e3a56fc 100644
@@ -190,9 +190,11 @@ static int wilc_bus_probe(struct spi_device *spi)
 static int wilc_bus_remove(struct spi_device *spi)
 {
 	struct wilc *wilc = spi_get_drvdata(spi);
+	struct wilc_spi *spi_priv = wilc->bus_data;
 
 	clk_disable_unprepare(wilc->rtc_clk);
 	wilc_netdev_cleanup(wilc);
+	kfree(spi_priv);
 
 	return 0;
 }
Signature · 1 line
2.20.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 7 lines · Dan Carpenter

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Smatch complains that there is a double free in probe:

drivers/net/wireless/microchip/wilc1000/spi.c:186 wilc_bus_probe() error: double free of 'spi_priv' drivers/net/wireless/microchip/wilc1000/sdio.c:163 wilc_sdio_probe() error: double free of 'sdio_priv'

Quoted 7 lines

The problem is that wilc_netdev_cleanup() function frees "wilc->bus_data". That's confusing and a layering violation. Leave the frees in probe(), delete the free in wilc_netdev_cleanup(), and add some new frees to the remove() functions.

Fixes dc8b338f3bcd ("wilc1000: use goto labels on error path")
Quoted 53 lines
---
 drivers/net/wireless/microchip/wilc1000/netdev.c | 1 -
 drivers/net/wireless/microchip/wilc1000/sdio.c   | 2 ++
 drivers/net/wireless/microchip/wilc1000/spi.c    | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)
drivers/net/wireless/microchip/wilc1000/netdev.c +0 -1
index 03e3485d7e7f..643bddaae32a 100644
@@ -905,7 +905,6 @@ void wilc_netdev_cleanup(struct wilc *wilc)
 
        wilc_wlan_cfg_deinit(wilc);
        wlan_deinit_locks(wilc);
-       kfree(wilc->bus_data);
        wiphy_unregister(wilc->wiphy);
        wiphy_free(wilc->wiphy);
 }
drivers/net/wireless/microchip/wilc1000/sdio.c +2 -0
index 26ebf6664342..ec595dbd8959 100644
@@ -167,9 +167,11 @@ static int wilc_sdio_probe(struct sdio_func *func,
 static void wilc_sdio_remove(struct sdio_func *func)
 {
        struct wilc *wilc = sdio_get_drvdata(func);
+       struct wilc_sdio *sdio_priv = wilc->bus_data;
 
        clk_disable_unprepare(wilc->rtc_clk);
        wilc_netdev_cleanup(wilc);
+       kfree(sdio_priv);
 }
 
 static int wilc_sdio_reset(struct wilc *wilc)
drivers/net/wireless/microchip/wilc1000/spi.c +2 -1
index e0871b89917d..5ace9e3a56fc 100644
@@ -190,9 +190,11 @@ static int wilc_bus_probe(struct spi_device *spi)
 static int wilc_bus_remove(struct spi_device *spi)
 {
        struct wilc *wilc = spi_get_drvdata(spi);
+       struct wilc_spi *spi_priv = wilc->bus_data;
 
        clk_disable_unprepare(wilc->rtc_clk);
        wilc_netdev_cleanup(wilc);
+       kfree(spi_priv);
 
        return 0;
 }
--

2.20.1

#2 lore raw
In reply to Dan Carpenter
Quoted 5 lines · Dan Carpenter

Smatch complains that there is a double free in probe:

drivers/net/wireless/microchip/wilc1000/spi.c:186 wilc_bus_probe() error: double free of 'spi_priv' drivers/net/wireless/microchip/wilc1000/sdio.c:163 wilc_sdio_probe() error: double free of 'sdio_priv'

Quoted 8 lines

The problem is that wilc_netdev_cleanup() function frees "wilc->bus_data". That's confusing and a layering violation. Leave the frees in probe(), delete the free in wilc_netdev_cleanup(), and add some new frees to the remove() functions.

Fixes dc8b338f3bcd ("wilc1000: use goto labels on error path")

Patch applied to wireless-drivers-next.git, thanks.

4894edacfa93 wilc1000: fix double free error in probe()

Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/patch/20211217150311.GC16611@kili/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches