[PATCH] ISP1704 USB Charger: Fix use-after-free error in isp1704_charger_probe()

[PATCH] ISP1704 USB Charger: Fix use-after-free error in isp1704_charger_probe()

[Jesper Juhl]

In isp1704_charger_probe() at the 'fail0:' label we kfree(isp) and
then subsequently call isp1704_charger_set_power(isp, 0). That's a
problem since isp1704_charger_set_power() dereferences the pointer it
is passed as its first argument, which is 'isp', which we already
freed.

Fixed by simply swapping the order of the two calls so that we only
kfree() *after* the call to isp1704_charger_set_power().

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/power/isp1704_charger.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/power/isp1704_charger.c b/drivers/power/isp1704_charger.c
index 39eb50f..8a610da 100644
--- a/drivers/power/isp1704_charger.c
+++ b/drivers/power/isp1704_charger.c
@@ -476,11 +476,9 @@ fail2:
 fail1:
 	usb_put_transceiver(isp->phy);
 fail0:
-	kfree(isp);
-
 	dev_err(&pdev->dev, "failed to register isp1704 with error %d\n", ret);
-
 	isp1704_charger_set_power(isp, 0);
+	kfree(isp);
 	return ret;
 }
 
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

Re: [PATCH] ISP1704 USB Charger: Fix use-after-free error in isp1704_charger_probe()

[Marek Vasut]

Dear Jesper Juhl,

> In isp1704_charger_probe() at the 'fail0:' label we kfree(isp) and
> then subsequently call isp1704_charger_set_power(isp, 0). That's a
> problem since isp1704_charger_set_power() dereferences the pointer it
> is passed as its first argument, which is 'isp', which we already
> freed.
> 
> Fixed by simply swapping the order of the two calls so that we only
> kfree() *after* the call to isp1704_charger_set_power().
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  drivers/power/isp1704_charger.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/power/isp1704_charger.c
> b/drivers/power/isp1704_charger.c index 39eb50f..8a610da 100644
> --- a/drivers/power/isp1704_charger.c
> +++ b/drivers/power/isp1704_charger.c
> @@ -476,11 +476,9 @@ fail2:
>  fail1:
>  	usb_put_transceiver(isp->phy);
>  fail0:
> -	kfree(isp);
> -
>  	dev_err(&pdev->dev, "failed to register isp1704 with error %d\n", ret);
> -
>  	isp1704_charger_set_power(isp, 0);
> +	kfree(isp);

Use devm_kzalloc() and be done with all this goo?

>  	return ret;
>  }

Best regards,
Marek Vasut

Re: [PATCH] ISP1704 USB Charger: Fix use-after-free error in isp1704_charger_probe()

[Anton Vorontsov]

On Sun, Apr 22, 2012 at 10:34:59PM +0200, Marek Vasut wrote:
[...]
> > -
> >  	dev_err(&pdev->dev, "failed to register isp1704 with error %d\n", ret);
> > -
> >  	isp1704_charger_set_power(isp, 0);
> > +	kfree(isp);

Dan Carpenter sent a similar fix ~month ago, so it's in already.

> Use devm_kzalloc() and be done with all this goo?

Yeah, that's actually a good idea.

Thanks,

-- 
Anton Vorontsov
Email: cbouatmailru@gmail.com

Re: [PATCH] ISP1704 USB Charger: Fix use-after-free error in isp1704_charger_probe()

[Jesper Juhl]

Sorry about the late reply, I've been away from my mailbox for a while.

On Sat, 5 May 2012, Anton Vorontsov wrote:

> On Sun, Apr 22, 2012 at 10:34:59PM +0200, Marek Vasut wrote:
> [...]
> > > -
> > >  	dev_err(&pdev->dev, "failed to register isp1704 with error %d\n", ret);
> > > -
> > >  	isp1704_charger_set_power(isp, 0);
> > > +	kfree(isp);
> 
> Dan Carpenter sent a similar fix ~month ago, so it's in already.
> 
Ok :)

> > Use devm_kzalloc() and be done with all this goo?
> 
> Yeah, that's actually a good idea.
> 
No promises, but I may look into this in a few days.

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.