Re: [PATCH] staging: mt29f_spinand: fix memory leak while programming pages
On Thu, Aug 02, 2018 at 11:42:30AM +0800, Jheng-Jhong Wu wrote: > Dear Dan, > > I know what you wrote, but before the spinand device is removed and > freed memory automatically, programming pages may do many many times. > Assume we erase and rewrite a large part of the flash, then > spinand_program_page() might exhaust memory if memory is not large > enough. > In fact, OOM indeed occured when I tested programming multi-pages by > mtd_debug tool. > If OOM was not caused by devm_kzalloc() in spinand_program_page(), > what may exhaust memory? > Ok. That makes sense. I didn't look at it in context. You should say that sort of thing in your changelog. Looking at it now, the devm_ model isn't right for this function so we should change it to use normal kzalloc(). We should fix all the error paths as well. It looks like if this function starts returning errors, we are probably toasted, but it's still good practice to avoid slow leaks. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: mt29f_spinand: fix memory leak while programming pages
Dear Dan, I know what you wrote, but before the spinand device is removed and freed memory automatically, programming pages may do many many times. Assume we erase and rewrite a large part of the flash, then spinand_program_page() might exhaust memory if memory is not large enough. In fact, OOM indeed occured when I tested programming multi-pages by mtd_debug tool. If OOM was not caused by devm_kzalloc() in spinand_program_page(), what may exhaust memory? Best Regards, ─ Jheng-Jhong Wu (Victor Wu) E-mail: goodwater...@gmail.com ─ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: mt29f_spinand: fix memory leak while programming pages
devm_ resources are freed automatically when the device is removed. The name devm_ stands for "device" and "managed". regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: mt29f_spinand: fix memory leak while programming pages
In spinand_program_page(), it uses devm_kzalloc() to allocate memory to wbuf dynamically if internal ECC is on, but it doesn't free memory allocated to wbuf at the end of this function. This leads to a memory leak issue when internal ECC is on. Signed-off-by: Jheng-Jhong Wu --- drivers/staging/mt29f_spinand/mt29f_spinand.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c index e389009..cf51ca8 100644 --- a/drivers/staging/mt29f_spinand/mt29f_spinand.c +++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c @@ -553,6 +553,8 @@ static int spinand_program_page(struct spi_device *spi_nand, } enable_hw_ecc = 0; } + + devm_kfree(&spi_nand->dev, wbuf); #endif return 0; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
