2022-12-22 03:08:24

by Jimmy Hu

[permalink] [raw]
Subject: [PATCH 1/1] usb: xhci: Check endpoint is valid before dereferencing it

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 <[email protected]>
---
drivers/usb/host/xhci-ring.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

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;
--
2.38.0.rc1.362.ged0d419d3c-goog


2022-12-22 06:40:12

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/1] usb: xhci: Check endpoint is valid before dereferencing it

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 <[email protected]>
> ---
> 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

2022-12-22 07:20:31

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/1] usb: xhci: Check endpoint is valid before dereferencing it

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 <[email protected]>]
> 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 <[email protected]>]

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 <[email protected]> (supporter:USB XHCI DRIVER)
> Greg Kroah-Hartman <[email protected]> (supporter:USB SUBSYSTEM)
> [email protected] (open list:USB XHCI DRIVER)
> [email protected] (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