On Tue, Feb 11, 2020 at 09:46:54AM +0100, Michał Mirosław wrote:
> @@ -218,9 +218,9 @@ static int wfx_sdio_probe(struct sdio_func *func,
> return 0;
>
> err3:
> - wfx_free_common(bus->core);
> + wfx_sdio_irq_unsubscribe(bus);
> err2:
> - wfx_sdio_irq_unsubscribe(bus);
> + wfx_free_common(bus->core);
> err1:
> sdio_claim_host(func);
> sdio_disable_func(func);
> @@ -234,8 +234,8 @@ static void wfx_sdio_remove(struct sdio_func *func)
> struct wfx_sdio_priv *bus = sdio_get_drvdata(func);
>
> wfx_release(bus->core);
> - wfx_free_common(bus->core);
> wfx_sdio_irq_unsubscribe(bus);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This calls sdio_release_host(func);
> + wfx_free_common(bus->core);
> sdio_claim_host(func);
> sdio_disable_func(func);
> sdio_release_host(func);
^^^^^^^^^^^^^^^^^^^^^^^^
so is this a double free?
> diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
> index 3ba705477ca8..2b108a9fa5ae 100644
> --- a/drivers/staging/wfx/bus_spi.c
> +++ b/drivers/staging/wfx/bus_spi.c
> @@ -211,20 +211,22 @@ static int wfx_spi_probe(struct spi_device *func)
> udelay(2000);
> }
>
> - ret = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler,
> - IRQF_TRIGGER_RISING, "wfx", bus);
> - if (ret)
> - return ret;
> -
> INIT_WORK(&bus->request_rx, wfx_spi_request_rx);
> bus->core = wfx_init_common(&func->dev, &wfx_spi_pdata,
> &wfx_spi_hwbus_ops, bus);
> if (!bus->core)
> return -EIO;
>
> + ret = devm_request_irq(&func->dev, func->irq, wfx_spi_irq_handler,
> + IRQF_TRIGGER_RISING, "wfx", bus);
> + if (ret)
> + return ret;
Shouldn't this call wfx_free_common(bus->core); before returning?
> +
> ret = wfx_probe(bus->core);
> - if (ret)
> + if (ret) {
> + devm_free_irq(&func->dev, func->irq, bus);
We shouldn't have to free devm_ resource.
> wfx_free_common(bus->core);
> + }
>
> return ret;
> }
> @@ -234,11 +236,11 @@ static int wfx_spi_remove(struct spi_device *func)
> struct wfx_spi_priv *bus = spi_get_drvdata(func);
>
> wfx_release(bus->core);
> - wfx_free_common(bus->core);
> // A few IRQ will be sent during device release. Hopefully, no IRQ
> // should happen after wdev/wvif are released.
> devm_free_irq(&func->dev, func->irq, bus);
Is this devm_ free required?
> flush_work(&bus->request_rx);
> + wfx_free_common(bus->core);
> return 0;
> }
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
