Any chance this could be backported to R70 as well?

I was told that there will be no more ChromeOS builds for R70.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/4be1b892ee2087b474654f9af605bc8572df41b2

commit 4be1b892ee2087b474654f9af605bc8572df41b2
Author: Mathias Nyman <mathias.nyman@linux.intel.com>
Date: Tue Nov 27 04:31:19 2018

UPSTREAM: xhci: Fix USB3 NULL pointer dereference at logical disconnect.

Hub driver will try to disable a USB3 device twice at logical disconnect,
racing with xhci_free_dev() callback from the first port disable.

This can be triggered with "udisksctl power-off --block-device <disk>"
or by writing "1" to the "remove" sysfs file for a USB3 device
in 4.17-rc4.

USB3 devices don't have a similar disabled link state as USB2 devices,
and use a U3 suspended link state instead. In this state the port
is still enabled and connected.

hub_port_connect() first disconnects the device, then later it notices
that device is still enabled (due to U3 states) it will try to disable
the port again (set to U3).

The xhci_free_dev() called during device disable is async, so checking
for existing xhci->devs[i] when setting link state to U3 the second time
was successful, even if device was being freed.

The regression was caused by, and whole thing revealed by,
Commit 44a182b9d177 ("xhci: Fix use-after-free in xhci_free_virt_device")
which sets xhci->devs[i]->udev to NULL before xhci_virt_dev() returned.
and causes a NULL pointer dereference the second time we try to set U3.

Fix this by checking xhci->devs[i]->udev exists before setting link state.

The original patch went to stable so this fix needs to be applied there as
well.

Fixes: 44a182b9d177 ("xhci: Fix use-after-free in xhci_free_virt_device")
Cc: <stable@vger.kernel.org>
Reported-by: Jordan Glover <Golden_Miller83@protonmail.ch>
Tested-by: Jordan Glover <Golden_Miller83@protonmail.ch>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 2278446e2b7cd33ad894b32e7eb63afc7db6c86e)

BUG= chromium:908455 
TEST=udisksctl power-off --block-device <disk>

Change-Id: Ief142b666aadf014d00a8bb3e92a4d71030bc924
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1351289
Reviewed-by: Dariusz Marcinkiewicz <darekm@google.com>
Reviewed-by: Felix Ekblom <felixe@chromium.org>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>

[modify] https://crrev.com/4be1b892ee2087b474654f9af605bc8572df41b2/drivers/usb/host/xhci-hub.c

This bug requires manual review: We don't branch M72 until 2018-11-29.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This bug requires manual review: We are only 6 days from stable.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Hi, I don't see comments regarding testing / verification. Given pending stable I'd like to capture those details

#10: What kind of testing information do you require ? For my education, and since we had this several times now, is there some common document describing those testing requirements that I can rely on in the future ?

Comments from Felix Ekblom (felixe@) about the patch as applied here:

"As pointed out by +Stefan Adolfsson, https://patchwork.kernel.org/patch/10394653/ might be interesting for the USB XHCI  issue #3  above. Since applying that patch locally I have not seen the USB crash (though not enough time has passed to say anything conclusive)."

Based on code inspection, my conclusion is 1) that the patch does indeed fix the observed problem, and that 2) the patch, even if it does not completely fix the problem, adds zero additional risk. If that is insufficient to accept the patch into M-71, I would suggest to let it rest in mainline for an extended period of time and verify with the following query on crash.corp.google.com if it is still seen and where.

"product_name='ChromeOS' AND EXISTS (SELECT 1 FROM UNNEST(productdata) WHERE Key='exec_name' AND STRPOS(Value, 'kernel') > 0) AND stable_signature like '%xhci_find_slot_id_by_port%'"

Unfortunately, Teemo devices do not currently show up with this query, so the result may be less than perfect, but maybe that will change in the future as the affected devices become more common in the field.

I'd like to see that the changes have been tested on M72, etc. and that the change had the desired effect without creating other problems.  Ideally on more than one board.

Submitting a CL with no testing detail makes it impossible to approve.

Thanks for the update

Approving merge to M71 Chrome OS.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/b3236cae9e59881db1d87a5ce658f1a28b1cb278

commit b3236cae9e59881db1d87a5ce658f1a28b1cb278
Author: Mathias Nyman <mathias.nyman@linux.intel.com>
Date: Wed Nov 28 23:33:38 2018

UPSTREAM: xhci: Fix USB3 NULL pointer dereference at logical disconnect.

Hub driver will try to disable a USB3 device twice at logical disconnect,
racing with xhci_free_dev() callback from the first port disable.

This can be triggered with "udisksctl power-off --block-device <disk>"
or by writing "1" to the "remove" sysfs file for a USB3 device
in 4.17-rc4.

USB3 devices don't have a similar disabled link state as USB2 devices,
and use a U3 suspended link state instead. In this state the port
is still enabled and connected.

hub_port_connect() first disconnects the device, then later it notices
that device is still enabled (due to U3 states) it will try to disable
the port again (set to U3).

The xhci_free_dev() called during device disable is async, so checking
for existing xhci->devs[i] when setting link state to U3 the second time
was successful, even if device was being freed.

The regression was caused by, and whole thing revealed by,
Commit 44a182b9d177 ("xhci: Fix use-after-free in xhci_free_virt_device")
which sets xhci->devs[i]->udev to NULL before xhci_virt_dev() returned.
and causes a NULL pointer dereference the second time we try to set U3.

Fix this by checking xhci->devs[i]->udev exists before setting link state.

The original patch went to stable so this fix needs to be applied there as
well.

Fixes: 44a182b9d177 ("xhci: Fix use-after-free in xhci_free_virt_device")
Cc: <stable@vger.kernel.org>
Reported-by: Jordan Glover <Golden_Miller83@protonmail.ch>
Tested-by: Jordan Glover <Golden_Miller83@protonmail.ch>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 2278446e2b7cd33ad894b32e7eb63afc7db6c86e)

BUG= chromium:908455 
TEST=udisksctl power-off --block-device <disk>

Change-Id: Ief142b666aadf014d00a8bb3e92a4d71030bc924
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1351289
Reviewed-by: Dariusz Marcinkiewicz <darekm@google.com>
Reviewed-by: Felix Ekblom <felixe@chromium.org>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
(cherry picked from commit 4be1b892ee2087b474654f9af605bc8572df41b2)
Reviewed-on: https://chromium-review.googlesource.com/c/1352555

[modify] https://crrev.com/b3236cae9e59881db1d87a5ce658f1a28b1cb278/drivers/usb/host/xhci-hub.c

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot