| Message ID | 20221130101251.245399-1-yiyang13@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: fotg210-udc: fix potential memory leak in fotg210_udc_probe() | expand |
Hi Yi Yang, You can make your patch also simplify the code. Earlier, there's ret = -ENOMEM; /* initialize udc */ fotg210 = kzalloc(sizeof(struct fotg210_udc), GFP_KERNEL); if (fotg210 == NULL) goto err; and the code at the "err" label before your changes merely returns "ret". Instead of "goto err" return -ENOMEM can be used. This, in turn, eliminates the ret = -ENOMEM assignment. Furthermore, after applying your patch as-is, the said "goto err;" will result with kfree(NULL) which is safe, but unnecessary. If you include the above changes you can add my Reviewed-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> W dniu 30.11.2022 o 11:12, Yi Yang pisze: > In fotg210_udc_probe(), if devm_clk_get() or clk_prepare_enable() > fails, 'fotg210' will not be freed, which will lead to a memory leak. > Fix it by moving kfree() to a proper location. > > Fixes: 718a38d092ec ("fotg210-udc: Handle PCLK") > Signed-off-by: Yi Yang <yiyang13@huawei.com> > --- > drivers/usb/fotg210/fotg210-udc.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/fotg210/fotg210-udc.c b/drivers/usb/fotg210/fotg210-udc.c > index b9ea6c6d931c..a366d69c1818 100644 > --- a/drivers/usb/fotg210/fotg210-udc.c > +++ b/drivers/usb/fotg210/fotg210-udc.c > @@ -1178,7 +1178,7 @@ int fotg210_udc_probe(struct platform_device *pdev) > ret = clk_prepare_enable(fotg210->pclk); > if (ret) { > dev_err(dev, "failed to enable PCLK\n"); > - return ret; > + goto err; > } > } else if (PTR_ERR(fotg210->pclk) == -EPROBE_DEFER) { > /* > @@ -1302,8 +1302,7 @@ int fotg210_udc_probe(struct platform_device *pdev) > if (!IS_ERR(fotg210->pclk)) > clk_disable_unprepare(fotg210->pclk); > > - kfree(fotg210); > - > err: > + kfree(fotg210); > return ret; > }
On Wed, Nov 30, 2022 at 11:15 AM Yi Yang <yiyang13@huawei.com> wrote: > In fotg210_udc_probe(), if devm_clk_get() or clk_prepare_enable() > fails, 'fotg210' will not be freed, which will lead to a memory leak. > Fix it by moving kfree() to a proper location. > > Fixes: 718a38d092ec ("fotg210-udc: Handle PCLK") > Signed-off-by: Yi Yang <yiyang13@huawei.com> Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
diff --git a/drivers/usb/fotg210/fotg210-udc.c b/drivers/usb/fotg210/fotg210-udc.c index b9ea6c6d931c..a366d69c1818 100644 --- a/drivers/usb/fotg210/fotg210-udc.c +++ b/drivers/usb/fotg210/fotg210-udc.c @@ -1178,7 +1178,7 @@ int fotg210_udc_probe(struct platform_device *pdev) ret = clk_prepare_enable(fotg210->pclk); if (ret) { dev_err(dev, "failed to enable PCLK\n"); - return ret; + goto err; } } else if (PTR_ERR(fotg210->pclk) == -EPROBE_DEFER) { /* @@ -1302,8 +1302,7 @@ int fotg210_udc_probe(struct platform_device *pdev) if (!IS_ERR(fotg210->pclk)) clk_disable_unprepare(fotg210->pclk); - kfree(fotg210); - err: + kfree(fotg210); return ret; }
In fotg210_udc_probe(), if devm_clk_get() or clk_prepare_enable() fails, 'fotg210' will not be freed, which will lead to a memory leak. Fix it by moving kfree() to a proper location. Fixes: 718a38d092ec ("fotg210-udc: Handle PCLK") Signed-off-by: Yi Yang <yiyang13@huawei.com> --- drivers/usb/fotg210/fotg210-udc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)