From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Johan Hovold <johan@kernel.org>
Cc: gregkh@linuxfoundation.org, stern@rowland.harvard.edu,
linux-usb@vger.kernel.org, "# v5 . 3" <stable@vger.kernel.org>
Subject: Re: [RFT PATCH] xhci: Fix use-after-free regression in xhci clear hub TT implementation
Date: Mon, 14 Oct 2019 16:18:49 +0300 [thread overview]
Message-ID: <7e88dd63-9cd6-1149-10a0-960e944ef31f@linux.intel.com> (raw)
In-Reply-To: <20191014101611.GN13531@localhost>
On 14.10.2019 13.16, Johan Hovold wrote:
> On Fri, Oct 11, 2019 at 03:58:42PM +0300, Mathias Nyman wrote:
>> commit ef513be0a905 ("usb: xhci: Add Clear_TT_Buffer") schedules work
>> to clear TT buffer, but causes a use-after-free regression at the same time
>>
>> Make sure hub_tt_work finishes before endpoint is disabled, otherwise
>> the work will dereference already freed endpoint and device related
>> pointers.
>>
>> This was triggered when usb core failed to read the configuration
>> descriptor of a FS/LS device during enumeration.
>> xhci driver queued clear_tt_work while usb core freed and reallocated
>> a new device for the next enumeration attempt.
>>
>> EHCI driver implents ehci_endpoint_disable() that makes sure
>> clear_tt_work has finished before it returns, but xhci lacks this support.
>> usb core will call hcd->driver->endpoint_disable() callback before
>> disabling endpoints, so we want this in xhci as well.
>>
>> The added xhci_endpoint_disable() is based on ehci_endpoint_disable()
>>
>
> I used essentially the same reproducer as you did for debugging this
> after I first hit it with an actually stalled control endpoint, and this
> patch works also with my fault-injection hack.
>
> I've reviewed the code and it looks good to me except for one mostly
> theoretical issue. You need to check ep->hc_priv while holding the
> xhci->lock in xhci_clear_tt_buffer_complete() or you could end up having
> xhci_endpoint_disable() reschedule indefinitely while waiting for
> EP_CLEARING_TT to be cleared on a sufficiently weakly ordered
> system.
Good point, I'll change that
>
> Since cfbb8a84c2d2 ("xhci: Fix NULL pointer dereference in
> xhci_clear_tt_buffer_complete()") isn't needed anymore and is slightly
> misleading, I suggest amending the patch with the following:
>
I'll add those changes and your tags to the patch
Thanks
Mathias
prev parent reply other threads:[~2019-10-14 13:16 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-04 11:59 [PATCH 0/8] xhci fixes for usb-linus Mathias Nyman
2019-10-04 11:59 ` [PATCH 1/8] xhci: Fix false warning message about wrong bounce buffer write length Mathias Nyman
2019-10-04 11:59 ` [PATCH 2/8] xhci: Prevent device initiated U1/U2 link pm if exit latency is too long Mathias Nyman
2019-10-04 11:59 ` [PATCH 3/8] xhci: Check all endpoints for LPM timeout Mathias Nyman
2019-10-04 11:59 ` [PATCH 4/8] xhci: Fix USB 3.1 capability detection on early xHCI 1.1 spec based hosts Mathias Nyman
2019-10-04 11:59 ` [PATCH 5/8] usb: xhci: wait for CNR controller not ready bit in xhci resume Mathias Nyman
2019-10-04 11:59 ` [PATCH 6/8] xhci: Prevent deadlock when xhci adapter breaks during init Mathias Nyman
2019-10-04 11:59 ` [PATCH 7/8] xhci: Increase STS_SAVE timeout in xhci_suspend() Mathias Nyman
[not found] ` <20191006120750.5334F2087E@mail.kernel.org>
2019-10-07 18:51 ` Kai-Heng Feng
2019-10-04 11:59 ` [PATCH 8/8] xhci: Fix NULL pointer dereference in xhci_clear_tt_buffer_complete() Mathias Nyman
2019-10-07 14:02 ` Johan Hovold
2019-10-08 8:15 ` Mathias Nyman
2019-10-11 12:47 ` Mathias Nyman
2019-10-11 12:58 ` [RFT PATCH] xhci: Fix use-after-free regression in xhci clear hub TT implementation Mathias Nyman
2019-10-14 10:16 ` Johan Hovold
2019-10-14 13:18 ` Mathias Nyman [this message]
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=7e88dd63-9cd6-1149-10a0-960e944ef31f@linux.intel.com \
--to=mathias.nyman@linux.intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=johan@kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=stern@rowland.harvard.edu \
/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.