| Message ID | 20221222024630.1812735-1-hhhuuu@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] usb: xhci: Check endpoint is valid before dereferencing it | expand |
On Thu, Dec 22, 2022 at 02:46:30AM +0000, Jimmy Hu wrote: > When the host controller is not responding, all URBs queued to all > endpoints need to be killed. This can cause a kernel panic if we > dereference an invalid endpoint. > > Fix this by using xhci_get_virt_ep() helper to find the endpoint and > checking if the endpoint is valid before dereferencing it. > > [233311.853271] xhci-hcd xhci-hcd.1.auto: xHCI host controller not responding, assume dead > [233311.853393] Unable to handle kernel NULL pointer dereference at virtual address 00000000000000e8 > > [233311.853964] pc : xhci_hc_died+0x10c/0x270 > [233311.853971] lr : xhci_hc_died+0x1ac/0x270 > > [233311.854077] Call trace: > [233311.854085] xhci_hc_died+0x10c/0x270 > [233311.854093] xhci_stop_endpoint_command_watchdog+0x100/0x1a4 > [233311.854105] call_timer_fn+0x50/0x2d4 > [233311.854112] expire_timers+0xac/0x2e4 > [233311.854118] run_timer_softirq+0x300/0xabc > [233311.854127] __do_softirq+0x148/0x528 > [233311.854135] irq_exit+0x194/0x1a8 > [233311.854143] __handle_domain_irq+0x164/0x1d0 > [233311.854149] gic_handle_irq.22273+0x10c/0x188 > [233311.854156] el1_irq+0xfc/0x1a8 > [233311.854175] lpm_cpuidle_enter+0x25c/0x418 [msm_pm] > [233311.854185] cpuidle_enter_state+0x1f0/0x764 > [233311.854194] do_idle+0x594/0x6ac > [233311.854201] cpu_startup_entry+0x7c/0x80 > [233311.854209] secondary_start_kernel+0x170/0x198 > > Signed-off-by: Jimmy Hu <hhhuuu@google.com> > --- > drivers/usb/host/xhci-ring.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) What commit id does this fix? Should it also be cc: stable kernels? If so, how far back? thanks, greg k-h
A: http://en.wikipedia.org/wiki/Top_post Q: Were do I find info about this thing called top-posting? A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? A: No. Q: Should I include quotations after my reply? http://daringfireball.net/2007/07/on_top On Thu, Dec 22, 2022 at 02:39:16PM +0800, Jimmy Hu wrote: > The xhci_kill_endpoint_urbs() function was added in commit 50e8725e7c42 - > 2014-03-04 xhci: Refactor command watchdog and fix split string. [Sarah > Sharp <sarah.a.sharp@linux.intel.com>] > And then, it is called by xhci_hc_died() function added in d9f11ba9f107 - > 2017-04-08 xhci: Rework how we handle unresponsive or hoptlug removed hosts > [Mathias Nyman <mathias.nyman@linux.intel.com>] Ok, so which one do you think should be used? > Sorry. I don't know if it should also be cc: stable kernels. I use scripts/ > get_maintainer.pl to get the list. > ========= > $ scripts/get_maintainer.pl > 0001-usb-xhci-Check-endpoint-is-valid-before-dereferencin.patch > Mathias Nyman <mathias.nyman@intel.com> (supporter:USB XHCI DRIVER) > Greg Kroah-Hartman <gregkh@linuxfoundation.org> (supporter:USB SUBSYSTEM) > linux-usb@vger.kernel.org (open list:USB XHCI DRIVER) > linux-kernel@vger.kernel.org (open list) > ========= that tool has nothing to do with copying for stable kernels as the perl script does not know if this is a bugfix or not, right? Please read https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. thanks, greg k-h
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index ad81e9a508b1..4a514c227db7 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1170,7 +1170,10 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, struct xhci_virt_ep *ep; struct xhci_ring *ring; - ep = &xhci->devs[slot_id]->eps[ep_index]; + ep = xhci_get_virt_ep(xhci, slot_id, ep_index); + if (!ep) + return; + if ((ep->ep_state & EP_HAS_STREAMS) || (ep->ep_state & EP_GETTING_NO_STREAMS)) { int stream_id;
When the host controller is not responding, all URBs queued to all endpoints need to be killed. This can cause a kernel panic if we dereference an invalid endpoint. Fix this by using xhci_get_virt_ep() helper to find the endpoint and checking if the endpoint is valid before dereferencing it. [233311.853271] xhci-hcd xhci-hcd.1.auto: xHCI host controller not responding, assume dead [233311.853393] Unable to handle kernel NULL pointer dereference at virtual address 00000000000000e8 [233311.853964] pc : xhci_hc_died+0x10c/0x270 [233311.853971] lr : xhci_hc_died+0x1ac/0x270 [233311.854077] Call trace: [233311.854085] xhci_hc_died+0x10c/0x270 [233311.854093] xhci_stop_endpoint_command_watchdog+0x100/0x1a4 [233311.854105] call_timer_fn+0x50/0x2d4 [233311.854112] expire_timers+0xac/0x2e4 [233311.854118] run_timer_softirq+0x300/0xabc [233311.854127] __do_softirq+0x148/0x528 [233311.854135] irq_exit+0x194/0x1a8 [233311.854143] __handle_domain_irq+0x164/0x1d0 [233311.854149] gic_handle_irq.22273+0x10c/0x188 [233311.854156] el1_irq+0xfc/0x1a8 [233311.854175] lpm_cpuidle_enter+0x25c/0x418 [msm_pm] [233311.854185] cpuidle_enter_state+0x1f0/0x764 [233311.854194] do_idle+0x594/0x6ac [233311.854201] cpu_startup_entry+0x7c/0x80 [233311.854209] secondary_start_kernel+0x170/0x198 Signed-off-by: Jimmy Hu <hhhuuu@google.com> --- drivers/usb/host/xhci-ring.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)