Messages in this thread | | | From | Kees Cook <> | Date | Fri, 15 Jun 2018 09:47:16 -0700 | Subject | Re: [PATCH v2] staging: rtl8192u: add error handling for usb_alloc_urb |
| |
On Fri, Jun 15, 2018 at 9:33 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Sat, Jun 16, 2018 at 12:25:23AM +0800, Zhouyang Jia wrote: >> When usb_alloc_urb fails, the lack of error-handling code may >> cause unexpected results. >> >> This patch adds error-handling code after calling usb_alloc_urb. >> >> Signed-off-by: Zhouyang Jia <jiazhouyang09@gmail.com> >> --- >> v1->v2: >> - Fix memory leak. >> --- >> drivers/staging/rtl8192u/r8192U_core.c | 18 +++++++++++++++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c >> index 7a0dbc0..6afab4e 100644 >> --- a/drivers/staging/rtl8192u/r8192U_core.c >> +++ b/drivers/staging/rtl8192u/r8192U_core.c >> @@ -1648,13 +1648,17 @@ static short rtl8192_usb_initendpoints(struct net_device *dev) >> #ifndef JACKSON_NEW_RX >> for (i = 0; i < (MAX_RX_URB + 1); i++) { >> priv->rx_urb[i] = usb_alloc_urb(0, GFP_KERNEL); >> - if (!priv->rx_urb[i]) >> + if (!priv->rx_urb[i]) { >> + kfree(priv->rx_urb);
You're freeing rx_urb, which holds all the pointers to allocated memory. You'll need to free each item of the array before freeing the array itself:
for (i = 0; i < (MAX_RX_URB + 1); i++) kfree(priv->rx_urb[i]); kfree(priv->rx_urb);
I think you need some kind of helper to do this, and you can call into it from your error paths...
-Kees
> {sigh} > > No, you are still leaking memory on all of these changes that you just > made :( > > greg k-h
-- Kees Cook Pixel Security
| |