[PATCH] xhci: Fix NULL pointer dereference at endpoint zero reset.

[PATCH] xhci: Fix NULL pointer dereference at endpoint zero reset.

[Mathias Nyman]

Usb core will reset the default control endpoint "ep0" before resetting
a device. if the endpoint has a valid pointer back to the usb device
then the xhci driver reset callback will try to clear the toggle for
the endpoint.

ep0 didn't use to have this pointer set as ep0 was always allocated
by default together with a xhci slot for the usb device. Other endpoints
got their usb device pointer set in xhci_add_endpoint()

This changed with commit ef513be0a905 ("usb: xhci: Add Clear_TT_Buffer")
which sets the pointer for any endpoint on a FS/LS device behind a
HS hub that halts, including ep0.

If xHC controller needs to be reset at resume, then all the xhci slots
will be lost. Slots will be reenabled and reallocated at device reset,
but unlike other endpoints the ep0 is reset before device reset, while
the xhci slot may still be invalid, causing NULL pointer dereference.

Fix it by checking that the endpoint has both a usb device pointer and
valid xhci slot before trying to clear the toggle.

This issue was not seen earlier as ep0 didn't use to have a valid usb
device pointer, and other endpoints were only reset after device reset
when xhci slots were properly reenabled.

Reported-by: Bob Gleitsmann <rjgleits@bellsouth.net>
Reported-by: Enric Balletbo Serra <eballetbo@gmail.com>
Fixes: ef513be0a905 ("usb: xhci: Add Clear_TT_Buffer")
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 248cd7a..03d1e55 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3089,8 +3089,18 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 		return;
 	udev = (struct usb_device *) host_ep->hcpriv;
 	vdev = xhci->devs[udev->slot_id];
+
+	/*
+	 * vdev may be lost due to xHC restore error and re-initialization
+	 * during S3/S4 resume. A new vdev will be allocated later by
+	 * xhci_discover_or_reset_device()
+	 */
+	if (!udev->slot_id || !vdev)
+		return;
 	ep_index = xhci_get_endpoint_index(&host_ep->desc);
 	ep = &vdev->eps[ep_index];
+	if (!ep)
+		return;
 
 	/* Bail out if toggle is already being cleared by a endpoint reset */
 	if (ep->ep_state & EP_HARD_CLEAR_TOGGLE) {
-- 
2.7.4

Re: [PATCH] xhci: Fix NULL pointer dereference at endpoint zero reset.

[Enric Balletbo Serra]

Hi Mathias,

Missatge de Mathias Nyman <mathias.nyman@linux.intel.com> del dia dv.,
2 d’ag. 2019 a les 16:59:
>
> Usb core will reset the default control endpoint "ep0" before resetting
> a device. if the endpoint has a valid pointer back to the usb device
> then the xhci driver reset callback will try to clear the toggle for
> the endpoint.
>
> ep0 didn't use to have this pointer set as ep0 was always allocated
> by default together with a xhci slot for the usb device. Other endpoints
> got their usb device pointer set in xhci_add_endpoint()
>
> This changed with commit ef513be0a905 ("usb: xhci: Add Clear_TT_Buffer")
> which sets the pointer for any endpoint on a FS/LS device behind a
> HS hub that halts, including ep0.
>
> If xHC controller needs to be reset at resume, then all the xhci slots
> will be lost. Slots will be reenabled and reallocated at device reset,
> but unlike other endpoints the ep0 is reset before device reset, while
> the xhci slot may still be invalid, causing NULL pointer dereference.
>
> Fix it by checking that the endpoint has both a usb device pointer and
> valid xhci slot before trying to clear the toggle.
>
> This issue was not seen earlier as ep0 didn't use to have a valid usb
> device pointer, and other endpoints were only reset after device reset
> when xhci slots were properly reenabled.
>
> Reported-by: Bob Gleitsmann <rjgleits@bellsouth.net>
> Reported-by: Enric Balletbo Serra <eballetbo@gmail.com>
> Fixes: ef513be0a905 ("usb: xhci: Add Clear_TT_Buffer")
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>

Thanks for spending time looking at this issue and for the clear
explanation. The patch fixes the issue for me, so

Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

> ---
>  drivers/usb/host/xhci.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 248cd7a..03d1e55 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -3089,8 +3089,18 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
>                 return;
>         udev = (struct usb_device *) host_ep->hcpriv;
>         vdev = xhci->devs[udev->slot_id];
> +
> +       /*
> +        * vdev may be lost due to xHC restore error and re-initialization
> +        * during S3/S4 resume. A new vdev will be allocated later by
> +        * xhci_discover_or_reset_device()
> +        */
> +       if (!udev->slot_id || !vdev)
> +               return;
>         ep_index = xhci_get_endpoint_index(&host_ep->desc);
>         ep = &vdev->eps[ep_index];
> +       if (!ep)
> +               return;
>
>         /* Bail out if toggle is already being cleared by a endpoint reset */
>         if (ep->ep_state & EP_HARD_CLEAR_TOGGLE) {
> --
> 2.7.4
>