diff mbox series

wilc1000: fix double free error in probe()

Message ID 20211217150311.GC16611@kili
State New
Headers show
Series wilc1000: fix double free error in probe() | expand

Commit Message

Dan Carpenter Dec. 17, 2021, 3:03 p.m. UTC 
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")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 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(-)

Comments

Claudiu Beznea Dec. 20, 2021, 6:59 a.m. UTC | #1 
On 17.12.2021 17:03, Dan Carpenter wrote:
> 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'
> 
> 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")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>

> ---
>  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(-)
> 
> diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
> index 03e3485d7e7f..643bddaae32a 100644
> --- a/drivers/net/wireless/microchip/wilc1000/netdev.c
> +++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
> @@ -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);
>  }
> diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
> index 26ebf6664342..ec595dbd8959 100644
> --- a/drivers/net/wireless/microchip/wilc1000/sdio.c
> +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
> @@ -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)
> diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
> index e0871b89917d..5ace9e3a56fc 100644
> --- a/drivers/net/wireless/microchip/wilc1000/spi.c
> +++ b/drivers/net/wireless/microchip/wilc1000/spi.c
> @@ -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
>
Kalle Valo Dec. 20, 2021, 6:44 p.m. UTC | #2 
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> 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")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>

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

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

Patch

diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index 03e3485d7e7f..643bddaae32a 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -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);
 }
diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c b/drivers/net/wireless/microchip/wilc1000/sdio.c
index 26ebf6664342..ec595dbd8959 100644
--- a/drivers/net/wireless/microchip/wilc1000/sdio.c
+++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
@@ -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)
diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
index e0871b89917d..5ace9e3a56fc 100644
--- a/drivers/net/wireless/microchip/wilc1000/spi.c
+++ b/drivers/net/wireless/microchip/wilc1000/spi.c
@@ -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;
 }