From: Peter Chen <hzpeterchen@gmail.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>,
Fengguang Wu <fengguang.wu@intel.com>,
USB list <linux-usb@vger.kernel.org>
Subject: USB: UDC core: fix double-free in usb_add_gadget_udc_release
Date: Thu, 4 Jan 2018 09:27:25 +0800 [thread overview]
Message-ID: <20180104012725.GA31267@b29397-desktop> (raw)
On Wed, Jan 03, 2018 at 12:51:51PM -0500, Alan Stern wrote:
> The error-handling pathways in usb_add_gadget_udc_release() are messed
> up. Aside from the uninformative statement labels, they can deallocate
> the udc structure after calling put_device(), which is a double-free.
> This was observed by KASAN in automatic testing.
>
> This patch cleans up the routine. It preserves the requirement that
> when any failure occurs, we call put_device(&gadget->dev).
>
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> CC: <stable@vger.kernel.org>
>
> ---
>
> Not having received any feedback on this patch for a while, I have
> decided to submit it. The problem it fixes is quite obviously a real
> one, when you look at the existing code.
>
> Alan Stern
>
>
> [as1856]
>
>
> drivers/usb/gadget/udc/core.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> Index: usb-4.x/drivers/usb/gadget/udc/core.c
> ===================================================================
> --- usb-4.x.orig/drivers/usb/gadget/udc/core.c
> +++ usb-4.x/drivers/usb/gadget/udc/core.c
> @@ -1147,11 +1147,7 @@ int usb_add_gadget_udc_release(struct de
>
> udc = kzalloc(sizeof(*udc), GFP_KERNEL);
> if (!udc)
> - goto err1;
> -
> - ret = device_add(&gadget->dev);
> - if (ret)
> - goto err2;
> + goto err_put_gadget;
>
> device_initialize(&udc->dev);
> udc->dev.release = usb_udc_release;
> @@ -1160,7 +1156,11 @@ int usb_add_gadget_udc_release(struct de
> udc->dev.parent = parent;
> ret = dev_set_name(&udc->dev, "%s", kobject_name(&parent->kobj));
> if (ret)
> - goto err3;
> + goto err_put_udc;
> +
> + ret = device_add(&gadget->dev);
> + if (ret)
> + goto err_put_udc;
>
> udc->gadget = gadget;
> gadget->udc = udc;
> @@ -1170,7 +1170,7 @@ int usb_add_gadget_udc_release(struct de
>
> ret = device_add(&udc->dev);
> if (ret)
> - goto err4;
> + goto err_unlist_udc;
>
> usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
> udc->vbus = true;
> @@ -1178,27 +1178,25 @@ int usb_add_gadget_udc_release(struct de
> /* pick up one of pending gadget drivers */
> ret = check_pending_gadget_drivers(udc);
> if (ret)
> - goto err5;
> + goto err_del_udc;
>
> mutex_unlock(&udc_lock);
>
> return 0;
>
> -err5:
> + err_del_udc:
> device_del(&udc->dev);
>
> -err4:
> + err_unlist_udc:
> list_del(&udc->list);
> mutex_unlock(&udc_lock);
>
> -err3:
> - put_device(&udc->dev);
> device_del(&gadget->dev);
>
> -err2:
> - kfree(udc);
> + err_put_udc:
> + put_device(&udc->dev);
>
> -err1:
> + err_put_gadget:
> put_device(&gadget->dev);
> return ret;
> }
>
Reviewed-by: Peter Chen <peter.chen@nxp.com>
next reply other threads:[~2018-01-04 1:27 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-04 1:27 Peter Chen [this message]
-- strict thread matches above, loose matches on Subject: below --
2018-01-03 17:51 USB: UDC core: fix double-free in usb_add_gadget_udc_release Alan Stern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180104012725.GA31267@b29397-desktop \
--to=hzpeterchen@gmail.com \
--cc=felipe.balbi@linux.intel.com \
--cc=fengguang.wu@intel.com \
--cc=linux-usb@vger.kernel.org \
--cc=stern@rowland.harvard.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox