* [PATCH] dmaengine: at_hdmac: fix module unloading
2018-11-27 16:06 [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate() Richard Genoud
@ 2018-11-27 16:06 ` Richard Genoud
2018-11-29 13:52 ` Ludovic Desroches
2018-11-29 14:21 ` Vinod Koul
2018-11-29 13:51 ` [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate() Ludovic Desroches
` (2 subsequent siblings)
3 siblings, 2 replies; 7+ messages in thread
From: Richard Genoud @ 2018-11-27 16:06 UTC (permalink / raw)
To: Ludovic Desroches, Dan Williams, Vinod Koul
Cc: Alexandre Belloni, Nicolas Ferre, Maxime Ripard, Mario Forner,
linux-arm-kernel, dmaengine, linux-kernel, linux-serial,
Richard Genoud, stable
of_dma_controller_free() was not called on module onloading.
This lead to a soft lockup:
watchdog: BUG: soft lockup - CPU#0 stuck for 23s!
Modules linked in: at_hdmac [last unloaded: at_hdmac]
when of_dma_request_slave_channel() tried to call ofdma->of_dma_xlate().
Cc: stable@vger.kernel.org
Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
drivers/dma/at_hdmac.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 1b7f0ca0d5cd..01d936c9fe89 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -2006,6 +2006,8 @@ static int at_dma_remove(struct platform_device *pdev)
struct resource *io;
at_dma_off(atdma);
+ if (pdev->dev.of_node)
+ of_dma_controller_free(pdev->dev.of_node);
dma_async_device_unregister(&atdma->dma_common);
dma_pool_destroy(atdma->memset_pool);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: at_hdmac: fix module unloading
2018-11-27 16:06 ` [PATCH] dmaengine: at_hdmac: fix module unloading Richard Genoud
@ 2018-11-29 13:52 ` Ludovic Desroches
2018-11-29 14:21 ` Vinod Koul
1 sibling, 0 replies; 7+ messages in thread
From: Ludovic Desroches @ 2018-11-29 13:52 UTC (permalink / raw)
To: Richard Genoud
Cc: Dan Williams, Vinod Koul, Alexandre Belloni, Nicolas Ferre,
Maxime Ripard, Mario Forner, linux-arm-kernel, dmaengine,
linux-kernel, linux-serial, stable
On Tue, Nov 27, 2018 at 05:06:35PM +0100, Richard Genoud wrote:
> of_dma_controller_free() was not called on module onloading.
s/onloading/unloading
> This lead to a soft lockup:
> watchdog: BUG: soft lockup - CPU#0 stuck for 23s!
> Modules linked in: at_hdmac [last unloaded: at_hdmac]
> when of_dma_request_slave_channel() tried to call ofdma->of_dma_xlate().
>
> Cc: stable@vger.kernel.org
> Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Thanks
Ludovic
> ---
> drivers/dma/at_hdmac.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 1b7f0ca0d5cd..01d936c9fe89 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -2006,6 +2006,8 @@ static int at_dma_remove(struct platform_device *pdev)
> struct resource *io;
>
> at_dma_off(atdma);
> + if (pdev->dev.of_node)
> + of_dma_controller_free(pdev->dev.of_node);
> dma_async_device_unregister(&atdma->dma_common);
>
> dma_pool_destroy(atdma->memset_pool);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: at_hdmac: fix module unloading
2018-11-27 16:06 ` [PATCH] dmaengine: at_hdmac: fix module unloading Richard Genoud
2018-11-29 13:52 ` Ludovic Desroches
@ 2018-11-29 14:21 ` Vinod Koul
1 sibling, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2018-11-29 14:21 UTC (permalink / raw)
To: Richard Genoud
Cc: Ludovic Desroches, Dan Williams, Alexandre Belloni,
Nicolas Ferre, Maxime Ripard, Mario Forner, linux-arm-kernel,
dmaengine, linux-kernel, linux-serial, stable
On 27-11-18, 17:06, Richard Genoud wrote:
> of_dma_controller_free() was not called on module onloading.
> This lead to a soft lockup:
> watchdog: BUG: soft lockup - CPU#0 stuck for 23s!
> Modules linked in: at_hdmac [last unloaded: at_hdmac]
> when of_dma_request_slave_channel() tried to call ofdma->of_dma_xlate().
Applied, thanks
--
~Vinod
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate()
2018-11-27 16:06 [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate() Richard Genoud
2018-11-27 16:06 ` [PATCH] dmaengine: at_hdmac: fix module unloading Richard Genoud
@ 2018-11-29 13:51 ` Ludovic Desroches
2018-11-29 14:20 ` Vinod Koul
2018-11-30 10:07 ` Mario Forner
3 siblings, 0 replies; 7+ messages in thread
From: Ludovic Desroches @ 2018-11-29 13:51 UTC (permalink / raw)
To: Richard Genoud
Cc: Dan Williams, Vinod Koul, Alexandre Belloni, Nicolas Ferre,
Maxime Ripard, Mario Forner, linux-arm-kernel, dmaengine,
linux-kernel, linux-serial, stable
On Tue, Nov 27, 2018 at 05:06:34PM +0100, Richard Genoud wrote:
> The leak was found when opening/closing a serial port a great number of
> time, increasing kmalloc-32 in slabinfo.
>
> Each time the port was opened, dma_request_slave_channel() was called.
> Then, in at_dma_xlate(), atslave was allocated with devm_kzalloc() and
> never freed. (Well, it was free at module unload, but that's not what we
> want).
> So, here, kzalloc is more suited for the job since it has to be freed in
> atc_free_chan_resources().
>
> Cc: stable@vger.kernel.org
> Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
> Reported-by: Mario Forner <m.forner@be4energy.com>
> Suggested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Thanks for handling this issue.
Ludovic
> ---
> drivers/dma/at_hdmac.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 7cbac6e8c113..1b7f0ca0d5cd 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -1641,6 +1641,12 @@ static void atc_free_chan_resources(struct dma_chan *chan)
> atchan->descs_allocated = 0;
> atchan->status = 0;
>
> + /*
> + * Free atslave allocated in at_dma_xlate()
> + */
> + kfree(chan->private);
> + chan->private = NULL;
> +
> dev_vdbg(chan2dev(chan), "free_chan_resources: done\n");
> }
>
> @@ -1675,7 +1681,7 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
>
> - atslave = devm_kzalloc(&dmac_pdev->dev, sizeof(*atslave), GFP_KERNEL);
> + atslave = kzalloc(sizeof(*atslave), GFP_KERNEL);
> if (!atslave)
> return NULL;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate()
2018-11-27 16:06 [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate() Richard Genoud
2018-11-27 16:06 ` [PATCH] dmaengine: at_hdmac: fix module unloading Richard Genoud
2018-11-29 13:51 ` [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate() Ludovic Desroches
@ 2018-11-29 14:20 ` Vinod Koul
2018-11-30 10:07 ` Mario Forner
3 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2018-11-29 14:20 UTC (permalink / raw)
To: Richard Genoud
Cc: Ludovic Desroches, Dan Williams, Alexandre Belloni,
Nicolas Ferre, Maxime Ripard, Mario Forner, linux-arm-kernel,
dmaengine, linux-kernel, linux-serial, stable
On 27-11-18, 17:06, Richard Genoud wrote:
> The leak was found when opening/closing a serial port a great number of
> time, increasing kmalloc-32 in slabinfo.
>
> Each time the port was opened, dma_request_slave_channel() was called.
> Then, in at_dma_xlate(), atslave was allocated with devm_kzalloc() and
> never freed. (Well, it was free at module unload, but that's not what we
> want).
> So, here, kzalloc is more suited for the job since it has to be freed in
> atc_free_chan_resources().
Applied, thanks
--
~Vinod
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate()
2018-11-27 16:06 [PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate() Richard Genoud
` (2 preceding siblings ...)
2018-11-29 14:20 ` Vinod Koul
@ 2018-11-30 10:07 ` Mario Forner
3 siblings, 0 replies; 7+ messages in thread
From: Mario Forner @ 2018-11-30 10:07 UTC (permalink / raw)
To: Richard Genoud
Cc: Ludovic Desroches, Dan Williams, Vinod Koul, Alexandre Belloni,
Nicolas Ferre, Maxime Ripard, linux-arm-kernel, dmaengine,
linux-kernel, linux-serial, stable
Am Dienstag, 27. November 2018, 17:06:34 CET schrieb Richard Genoud:
> The leak was found when opening/closing a serial port a great number of
> time, increasing kmalloc-32 in slabinfo.
>
> Each time the port was opened, dma_request_slave_channel() was called.
> Then, in at_dma_xlate(), atslave was allocated with devm_kzalloc() and
> never freed. (Well, it was free at module unload, but that's not what we
> want).
> So, here, kzalloc is more suited for the job since it has to be freed in
> atc_free_chan_resources().
>
> Cc: stable@vger.kernel.org
> Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
> Reported-by: Mario Forner <m.forner@be4energy.com>
> Suggested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
After testing I installed an updated kernel on a production machine, which
worked fine. The memory leak has been repaired successfully. There have been
no adverse side-effects.
Thank you for providing the patch.
> ---
> drivers/dma/at_hdmac.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 7cbac6e8c113..1b7f0ca0d5cd 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -1641,6 +1641,12 @@ static void atc_free_chan_resources(struct dma_chan *chan)
> atchan->descs_allocated = 0;
> atchan->status = 0;
>
> + /*
> + * Free atslave allocated in at_dma_xlate()
> + */
> + kfree(chan->private);
> + chan->private = NULL;
> +
> dev_vdbg(chan2dev(chan), "free_chan_resources: done\n");
> }
>
> @@ -1675,7 +1681,7 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
>
> - atslave = devm_kzalloc(&dmac_pdev->dev, sizeof(*atslave), GFP_KERNEL);
> + atslave = kzalloc(sizeof(*atslave), GFP_KERNEL);
> if (!atslave)
> return NULL;
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread