From: Jia-Ju Bai <baijiaju1990@gmail.com>
To: mathias.nyman@intel.com, Greg KH <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: [BUG] usb: xhci: Possible resource leaks when xhci_run() fails
Date: Tue, 14 May 2019 22:58:05 +0800 [thread overview]
Message-ID: <fd7610ec-5f14-7952-cd9a-e56adb4e1353@gmail.com> (raw)
xhci_pci_setup() is assigned to hc_driver.reset;
xhci_run() is assigned to hc_driver.start();
xhci_stop() is assigned to hc_driver.stop().
xhci_pci_setup() calls xhci_gen_setup, which calls xhci_init(). And
xhci_init() calls xhci_mem_init() to allocate resources.
xhci_stop() calls xhci_mem_cleanup(), to release the resources allocated
in xhci_mem_init() (also namely xhci_pci_setup()).
xhci_run() can fail, because xhci_try_enable_msi() or
xhci_alloc_command() in this function can fail.
In drivers/usb/core/hcd.c:
retval = hcd->driver->reset(hcd);
if (retval < 0) {
......
goto err_hcd_driver_setup;
}
......
retval = hcd->driver->start(hcd);
if (retval < 0) {
......
goto err_hcd_driver_start;
}
.......
hcd->driver->stop(hcd);
hcd->state = HC_STATE_HALT;
clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);
del_timer_sync(&hcd->rh_timer);
err_hcd_driver_start:
if (usb_hcd_is_primary_hcd(hcd) && hcd->irq > 0)
free_irq(irqnum, hcd);
err_request_irq:
err_hcd_driver_setup:
err_set_rh_speed:
usb_put_invalidate_rhdev(hcd);
err_allocate_root_hub:
usb_deregister_bus(&hcd->self);
err_register_bus:
hcd_buffer_destroy(hcd);
err_create_buf:
usb_phy_roothub_power_off(hcd->phy_roothub);
err_usb_phy_roothub_power_on:
usb_phy_roothub_exit(hcd->phy_roothub);
Thus, when hcd->driver->reset() succeeds and hcd->driver->start() fails,
hcd->driver->stop() is not called.
Namely, when xhci_pci_setup() successfully allocates resources, and
xhci_run() fails, xhci_stop() is not called to release the resources.
For this reason, resource leaks occur in this case.
I check the code of the ehci driver, uhci driver and ohci driver, and
find that they do not have such problem, because:
In the ehci driver, ehci_run() (namely hcd->driver->start()) never fails.
In the uhci driver, all the resources are allocated in uhci_start
(namely hcd->driver->start()), and no resources are allocated in
uhci_pci_init() (namely hcd->driver->reset()).
In the ohci driver, ohci_setup() (namely hcd->driver->reset()) also
allocates resources. But when ohci_start() (namely hcd->driver->start())
is going to fail, ohci_stop() is directly called to release the
resources allocated by ohci_setup().
Thus, there are two possible ways of fixing bugs:
1) Call xhci_stop() when xhci_run() is going to fail (like the ohci driver)
2) Move all resource-allocation operations into xhci_run() (like the
uhci driver).
I am not sure whether these ways are correct, so I only report bugs.
These bugs are found by a runtime fuzzing tool named FIZZER written by us.
Best wishes,
Jia-Ju Bai
next reply other threads:[~2019-05-14 14:58 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-14 14:58 Jia-Ju Bai [this message]
2019-05-14 16:55 ` [BUG] usb: xhci: Possible resource leaks when xhci_run() fails Greg KH
2019-05-15 1:18 ` Jia-Ju Bai
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=fd7610ec-5f14-7952-cd9a-e56adb4e1353@gmail.com \
--to=baijiaju1990@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.