On Mon, Sep 17, 2012 at 1:28 AM, Sebastian Andrzej Siewior
<bigeasy@xxxxxxxxxxxxx> wrote:
> In case something goes wrong here, don't leak memory / endpoints.
>
> Cc: Yadwinder Singh <yadi.brar01@xxxxxxxxx>
> Cc: Jaswinder Singh <jaswinder.singh@xxxxxxxxxx>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/f_uac2.c | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_uac2.c b/drivers/usb/gadget/f_uac2.c
> index d3c6cff..f02b8ec 100644
> --- a/drivers/usb/gadget/f_uac2.c
> +++ b/drivers/usb/gadget/f_uac2.c
> @@ -978,15 +978,19 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
> INTF_SET(agdev->as_in_alt, ret);
>
> agdev->out_ep = usb_ep_autoconfig(gadget, &fs_epout_desc);
> - if (!agdev->out_ep)
> + if (!agdev->out_ep) {
> dev_err(&uac2->pdev.dev,
> "%s:%d Error!\n", __func__, __LINE__);
> + goto err;
> + }
> agdev->out_ep->driver_data = agdev;
>
> agdev->in_ep = usb_ep_autoconfig(gadget, &fs_epin_desc);
> - if (!agdev->in_ep)
> + if (!agdev->in_ep) {
> dev_err(&uac2->pdev.dev,
> "%s:%d Error!\n", __func__, __LINE__);
> + goto err;
> + }
> agdev->in_ep->driver_data = agdev;
>
> hs_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress;
> @@ -1005,6 +1009,7 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
> prm->max_psize = 0;
> dev_err(&uac2->pdev.dev,
> "%s:%d Error!\n", __func__, __LINE__);
> + goto err;
> }
>
> prm = &agdev->uac2.p_prm;
> @@ -1014,9 +1019,23 @@ afunc_bind(struct usb_configuration *cfg, struct usb_function *fn)
> prm->max_psize = 0;
> dev_err(&uac2->pdev.dev,
> "%s:%d Error!\n", __func__, __LINE__);
> + goto err;
> }
>
> - return alsa_uac2_init(agdev);
> + ret = alsa_uac2_init(agdev);
> + if (ret)
> + goto err;
> + return 0;
> +err:
> + kfree(agdev->uac2.p_prm.rbuf);
> + kfree(agdev->uac2.c_prm.rbuf);
> + usb_free_descriptors(fn->hs_descriptors);
> + usb_free_descriptors(fn->descriptors);
> + if (agdev->in_ep)
> + agdev->in_ep->driver_data = NULL;
> + if (agdev->out_ep)
> + agdev->out_ep->driver_data = NULL;
> + return -EINVAL;
> }
>
Thanks for looking into this.
Though better would be to not refuse IN transfers if an OUT ep isn't
available and vice versa - the sound card might support only either
Playback or Capture, it doesn't have to always support both. The rbuf
should have been allocated only for ep that exists. So I think, while
this patch prevents potential null-pointer dereferencing it also weeds
out the "simplex" capability of the driver.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
