From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Fri, 25 May 2018 00:52:58 +0200 Subject: [U-Boot] [PATCH v2] gadget: f_thor: Fix memory leaks of usb request and its buffer In-Reply-To: <1527128900-7801-1-git-send-email-sw0312.kim@samsung.com> References: <1527060345-23134-1-git-send-email-sw0312.kim@samsung.com> <1527128900-7801-1-git-send-email-sw0312.kim@samsung.com> Message-ID: <20180525005258.13094518@jawa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Seung-Woo, > There are memory leaks of usb request and its buffer for ep0, > in_ep, and out ep. Fix memory leaks of usb request and its buffer. > > Signed-off-by: Seung-Woo Kim > --- > Change from v1 > - remove allocation of out_ep request instead of allocating and > freeing > - fix use error path instead of duplicated error handling code > --- > drivers/usb/gadget/f_thor.c | 45 > ++++++++++++++++++++++++++++--------------- 1 file changed, 29 > insertions(+), 16 deletions(-) > > diff --git a/drivers/usb/gadget/f_thor.c b/drivers/usb/gadget/f_thor.c > index c8eda05..02d6844 100644 > --- a/drivers/usb/gadget/f_thor.c > +++ b/drivers/usb/gadget/f_thor.c > @@ -752,6 +752,13 @@ int thor_handle(void) > return 0; > } > > +static void free_ep_req(struct usb_ep *ep, struct usb_request *req) > +{ > + if (req->buf) > + free(req->buf); > + usb_ep_free_request(ep, req); > +} > + > static int thor_func_bind(struct usb_configuration *c, struct > usb_function *f) { > struct usb_gadget *gadget = c->cdev->gadget; > @@ -860,21 +867,18 @@ static int thor_func_bind(struct > usb_configuration *c, struct usb_function *f) return 0; > > fail: > + if (dev->req) > + free_ep_req(gadget->ep0, dev->req); > free(dev); > return status; > } > > -static void free_ep_req(struct usb_ep *ep, struct usb_request *req) > -{ > - free(req->buf); > - usb_ep_free_request(ep, req); > -} > - > static void thor_unbind(struct usb_configuration *c, struct > usb_function *f) { > struct f_thor *f_thor = func_to_thor(f); > struct thor_dev *dev = f_thor->dev; > > + free_ep_req(dev->gadget->ep0, dev->req); Till this change - no issues. > free(dev); > memset(thor_func, 0, sizeof(*thor_func)); > thor_func = NULL; > @@ -895,8 +899,6 @@ static void thor_func_disable(struct usb_function > *f) } > > if (dev->out_ep->driver_data) { > - free(dev->out_req->buf); > - dev->out_req->buf = NULL; I think that this setting (to NULL) was needed to be able to ctrl+C from thor command and then run it again (as some code checks if buf is NULL). > usb_ep_free_request(dev->out_ep, dev->out_req); > usb_ep_disable(dev->out_ep); > dev->out_ep->driver_data = NULL; > @@ -924,14 +926,13 @@ static int thor_eps_setup(struct usb_function > *f) > result = usb_ep_enable(ep, d); > if (result) > - goto exit; > + goto err; > > ep->driver_data = cdev; /* claim */ > req = thor_start_ep(ep); > if (!req) { > - usb_ep_disable(ep); > result = -EIO; > - goto exit; > + goto err_disable_in_ep; > } > > dev->in_req = req; > @@ -941,22 +942,34 @@ static int thor_eps_setup(struct usb_function > *f) > result = usb_ep_enable(ep, d); > if (result) > - goto exit; > + goto err_free_in_req; > > ep->driver_data = cdev; /* claim */ > - req = thor_start_ep(ep); > + req = usb_ep_alloc_request(ep, 0); Is this safe to replace thor_start_ep() - which tunes the ep params - with generic function? ( I do see the req->complete = thor_rx_tx_complete below ). If the thor_start_ep can be replaced with generic code, then maybe we can remove it? > if (!req) { > - usb_ep_disable(ep); > result = -EIO; > - goto exit; > + goto err_disable_out_ep; > } > > + req->complete = thor_rx_tx_complete; > dev->out_req = req; > /* ACM control EP */ > ep = dev->int_ep; > ep->driver_data = cdev; /* claim */ > > - exit: > + return 0; > + > + err_disable_out_ep: > + usb_ep_disable(dev->out_ep); > + > + err_free_in_req: > + free_ep_req(dev->in_ep, dev->in_req); > + dev->in_req = NULL; > + > + err_disable_in_ep: > + usb_ep_disable(dev->in_ep); > + > + err: > return result; > } > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 488 bytes Desc: OpenPGP digital signature URL: