| Message ID | 20211217150311.GC16611@kili (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4894edacfa93d7046bec4fc61fc402ac6a2ac9e8 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wilc1000: fix double free error in probe() | expand |
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 >
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 --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; }
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(-)