[PATCH] ARM: pxa: Fix a memory leak in pxa310_otg_exit()

[PATCH] ARM: pxa: Fix a memory leak in pxa310_otg_exit()

[Christophe JAILLET]

When otg_ulpi_create() is called, 2 kzalloc() are performed.
Only one is freed in pxa310_otg_exit().

The second kzalloc() has been introduced in the commit in the Fixes: tag.

Add the missing kfree().

Fixes: 298b083cf9dd ("usb: otg: ulpi: Start using struct usb_otg")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
This patch is NOT compile tested (I never cross compile even if I've been
tolled many times that it was easy...)

Another solution is to use devm_otg_ulpi_create() and further simplify
arch/arm/mach-pxa/pxa3xx-ulpi.c.

This would remove the only caller of otg_ulpi_create() and could also
simplify drivers/usb/phy/phy-ulpi.[ch]
---
 arch/arm/mach-pxa/pxa3xx-ulpi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-pxa/pxa3xx-ulpi.c b/arch/arm/mach-pxa/pxa3xx-ulpi.c
index c29a7f0fa1b0..82b4e2706f86 100644
--- a/arch/arm/mach-pxa/pxa3xx-ulpi.c
+++ b/arch/arm/mach-pxa/pxa3xx-ulpi.c
@@ -229,6 +229,7 @@ static int pxa310_otg_init(struct pxa3xx_u2d_platform_data *pdata)
 
 static void pxa310_otg_exit(void)
 {
+	kfree(u2d->otg->otg);
 	kfree(u2d->otg);
 }
 #else
-- 
2.34.1

Re: [PATCH] ARM: pxa: Fix a memory leak in pxa310_otg_exit()

[Dan Carpenter]

On Wed, Aug 03, 2022 at 08:00:44AM +0200, Christophe JAILLET wrote:
>  arch/arm/mach-pxa/pxa3xx-ulpi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-pxa/pxa3xx-ulpi.c b/arch/arm/mach-pxa/pxa3xx-ulpi.c
> index c29a7f0fa1b0..82b4e2706f86 100644
> --- a/arch/arm/mach-pxa/pxa3xx-ulpi.c
> +++ b/arch/arm/mach-pxa/pxa3xx-ulpi.c
> @@ -229,6 +229,7 @@ static int pxa310_otg_init(struct pxa3xx_u2d_platform_data *pdata)
>  
>  static void pxa310_otg_exit(void)
>  {
> +	kfree(u2d->otg->otg);
>  	kfree(u2d->otg);

It's unfortunate that we kept the "otg" name when changed u2d->otg to
a phy.  A lot of the stuff was renamed...  I don't have a cross compile
system set up either so I can't patch it but looks reasonably simple to
rename the remaining instance to phy?

diff --git a/arch/arm/mach-pxa/pxa3xx-ulpi.c b/arch/arm/mach-pxa/pxa3xx-ulpi.c
index c29a7f0fa1b0..a002fe1e96a2 100644
--- a/arch/arm/mach-pxa/pxa3xx-ulpi.c
+++ b/arch/arm/mach-pxa/pxa3xx-ulpi.c
@@ -30,7 +30,7 @@ struct pxa3xx_u2d_ulpi {
 	struct clk		*clk;
 	void __iomem		*mmio_base;
 
-	struct usb_phy		*otg;
+	struct usb_phy		*phy;
 	unsigned int		ulpi_mode;
 };
 
@@ -76,7 +76,7 @@ static int pxa310_ulpi_poll(void)
 	return -ETIMEDOUT;
 }
 
-static int pxa310_ulpi_read(struct usb_phy *otg, u32 reg)
+static int pxa310_ulpi_read(struct usb_phy *phy, u32 reg)
 {
 	int err;
 
@@ -95,7 +95,7 @@ static int pxa310_ulpi_read(struct usb_phy *otg, u32 reg)
 	return u2d_readl(U2DOTGUCR) & U2DOTGUCR_RDATA;
 }
 
-static int pxa310_ulpi_write(struct usb_phy *otg, u32 val, u32 reg)
+static int pxa310_ulpi_write(struct usb_phy *phy, u32 val, u32 reg)
 {
 	if (pxa310_ulpi_get_phymode() != SYNCH) {
 		pr_warn("%s: PHY is not in SYNCH mode!\n", __func__);
@@ -136,19 +136,19 @@ static int pxa310_start_otg_host_transcvr(struct usb_bus *host)
 
 	pxa310_otg_transceiver_rtsm();
 
-	err = usb_phy_init(u2d->otg);
+	err = usb_phy_init(u2d->phy);
 	if (err) {
 		pr_err("OTG transceiver init failed");
 		return err;
 	}
 
-	err = otg_set_vbus(u2d->otg->otg, 1);
+	err = otg_set_vbus(u2d->phy->otg, 1);
 	if (err) {
 		pr_err("OTG transceiver VBUS set failed");
 		return err;
 	}
 
-	err = otg_set_host(u2d->otg->otg, host);
+	err = otg_set_host(u2d->phy->otg, host);
 	if (err)
 		pr_err("OTG transceiver Host mode set failed");
 
@@ -186,9 +186,9 @@ static void pxa310_stop_otg_hc(void)
 {
 	pxa310_otg_transceiver_rtsm();
 
-	otg_set_host(u2d->otg->otg, NULL);
-	otg_set_vbus(u2d->otg->otg, 0);
-	usb_phy_shutdown(u2d->otg);
+	otg_set_host(u2d->phy->otg, NULL);
+	otg_set_vbus(u2d->phy->otg, 0);
+	usb_phy_shutdown(u2d->phy);
 }
 
 static void pxa310_u2d_setup_otg_hc(void)
@@ -218,18 +218,18 @@ static int pxa310_otg_init(struct pxa3xx_u2d_platform_data *pdata)
 
 	u2d->ulpi_mode = ulpi_mode;
 
-	u2d->otg = otg_ulpi_create(&pxa310_ulpi_access_ops, ulpi_mode);
-	if (!u2d->otg)
+	u2d->phy = otg_ulpi_create(&pxa310_ulpi_access_ops, ulpi_mode);
+	if (!u2d->phy)
 		return -ENOMEM;
 
-	u2d->otg->io_priv = u2d->mmio_base;
+	u2d->phy->io_priv = u2d->mmio_base;
 
 	return 0;
 }
 
 static void pxa310_otg_exit(void)
 {
-	kfree(u2d->otg);
+	kfree(u2d->phy);
 }
 #else
 static inline void pxa310_u2d_setup_otg_hc(void) {}