Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1451953
| From | Alan Stern <stern@rowland.harvard.edu> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] usb: hub: Fix unbalanced reference count and memory leak |
| Date | 2016-07-28 16:20 +0200 |
| Message-ID | <rZUkh-7mZ-11@gated-at.bofh.it> (permalink) |
| References | <rZFuW-5sq-43@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Wed, 27 Jul 2016, Viresh Kumar wrote:
> If the hub gets disconnected while the core is still activating it, this
> can result in leaking memory of few USB structures.
>
> This will happen if we have done a kref_get() from hub_activate() and
> scheduled a delayed work item for HUB_INIT2/3. Now if hub_disconnect()
> gets called before the delayed work expires, then we will cancel the
> work from hub_quiesce(), but wouldn't do a kref_put(). And so the
> unbalance.
>
> kmemleak reports this as (with the commit e50293ef9775 backported to
> 3.10 kernel with other changes, though the same is true for mainline as
> well):
...
> Fix this by putting the reference in hub_quiesce() if we canceled a
> pending work.
>
> CC: <stable@vger.kernel.org> #4.4+
> Fixes: e50293ef9775 ("USB: fix invalid memory access in hub_activate()")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Greg,
>
> This is tested over 3.10 with backported patches only, sorry didn't had
> a mainline setup to test this out. :(
Arg. This is exactly the sort of thing I should have foreseen when
writing the earlier commit.
> drivers/usb/core/hub.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index bee13517676f..3173693fa8e3 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1315,7 +1315,8 @@ static void hub_quiesce(struct usb_hub *hub, enum hub_quiescing_type type)
> struct usb_device *hdev = hub->hdev;
> int i;
>
> - cancel_delayed_work_sync(&hub->init_work);
> + if (cancel_delayed_work_sync(&hub->init_work))
> + kref_put(&hub->kref, hub_release);
>
> /* hub_wq and related activity won't re-trigger */
> hub->quiescing = 1;
Another possibility is to remove the cancel_delayed_work_sync call
entirely. Either way, you can add
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Alan Stern
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH] usb: hub: Fix unbalanced reference count and memory leak Viresh Kumar <viresh.kumar@linaro.org> - 2016-07-28 00:30 +0200
Re: [PATCH] usb: hub: Fix unbalanced reference count and memory leak Alan Stern <stern@rowland.harvard.edu> - 2016-07-28 16:20 +0200
Re: [PATCH] usb: hub: Fix unbalanced reference count and memory leak Viresh Kumar <viresh.kumar@linaro.org> - 2016-07-28 16:20 +0200
csiph-web