From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: usb HC busted? From: Sudip Mukherjee Message-Id: <20180717151017.yk5d4glzwqcrxpqc@debian> Date: Tue, 17 Jul 2018 16:10:17 +0100 To: Alan Stern , Greg KH Cc: Mathias Nyman , Andy Shevchenko , Andy Shevchenko , Mathias Nyman , linux-usb@vger.kernel.org, lukaszx.szulc@intel.com, Christoph Hellwig , Marek Szyprowski , iommu@lists.linux-foundation.org List-ID: SGkgQWxhbiwgR3JlZywKCk9uIFR1ZSwgSnVsIDE3LCAyMDE4IGF0IDAzOjQ5OjE4UE0gKzAxMDAs IFN1ZGlwIE11a2hlcmplZSB3cm90ZToKPiBPbiBUdWUsIEp1bCAxNywgMjAxOCBhdCAwMzo0MDoy MlBNICswMTAwLCBTdWRpcCBNdWtoZXJqZWUgd3JvdGU6Cj4gPiBIaSBBbGFuLAo+ID4gCj4gPiBP biBUdWUsIEp1bCAxNywgMjAxOCBhdCAxMDoyODoxNEFNIC0wNDAwLCBBbGFuIFN0ZXJuIHdyb3Rl Ogo+ID4gPiBPbiBUdWUsIDE3IEp1bCAyMDE4LCBTdWRpcCBNdWtoZXJqZWUgd3JvdGU6Cj4gPiA+ IAo+ID4gPiA+IEkgZGlkIHNvbWUgbW9yZSBkZWJ1Z2dpbmcuIFRlc3RlZCB3aXRoIGEgS0FTQU4g ZW5hYmxlZCBrZXJuZWwgYW5kIHRoYXQKPiA+ID4gPiBzaG93cyB0aGUgcHJvYmxlbS4gVGhlIHJl cG9ydCBpcyBhdHRhY2hlZC4KPiA+ID4gPiAKPiA+ID4gPiBUbyBteSB1bmRlcnN0YW5kaW5nOgo+ ID4gPiA+IAo+ID4gPiA+IGJ0dXNiX3dvcmsoKSBpcyBjYWxsaW5nIHVzYl9zZXRfaW50ZXJmYWNl KCkgd2l0aCBhbHRlcm5hdGUgPSAwLiB3aGljaAo+ID4gPiA+IGFnYWluIGNhbGxzIHVzYl9oY2Rf YWxsb2NfYmFuZHdpZHRoKCkgYW5kIHRoYXQgZnJlZXMgdGhlIHJpbmdzIGJ5Cj4gPiA+ID4geGhj aV9mcmVlX2VuZHBvaW50X3JpbmcoKS4KPiA+ID4gCj4gPiA+IFRoYXQgZG9lc24ndCBzb3VuZCBs aWtlIHRoZSByaWdodCB0aGluZyB0byBkby4gIFRoZSByaW5ncyBzaG91bGRuJ3QgYmUgCj4gPiA+ IGZyZWVkIHVudGlsIHhoY2lfZW5kcG9pbnRfZGlzYWJsZSgpIGlzIGNhbGxlZC4gIAo+ID4gPiAK PiA+ID4gT24gdGhlIG90aGVyIGhhbmQsIHRoZXJlIGRvZXNuJ3QgYXBwZWFyIHRvIGJlIGFueSAK PiA+ID4geGhjaV9lbmRwb2ludF9kaXNhYmxlKCkgcm91dGluZSwgYWx0aG91Z2ggYSBjb21tZW50 IHJlZmVycyB0byBpdC4gIAo+ID4gPiBNYXliZSB0aGlzIGlzIHRoZSByZWFsIHByb2JsZW0/Cj4g PiAKPiA+IG9uZSBvZiB5b3VyIG9sZCBtYWlsIG1pZ2h0IGhlbHAgOikKPiA+IAo+ID4gaHR0cHM6 Ly93d3cuc3Bpbmljcy5uZXQvbGlzdHMvbGludXgtdXNiL21zZzk4MTIzLmh0bWwKPiAKPiBXcm90 ZSB0b28gc29vbi4KPiAKPiBJcyBpdCB0aGUgb25lIHlvdSBhcmUgbG9va2luZyBmb3IgLQo+IHVz Yl9kaXNhYmxlX2VuZHBvaW50KCkgaXMgaW4gZHJpdmVycy91c2IvY29yZS9tZXNzYWdlLmMKCkkg dGhpbmsgbm93IEkgdW5kZXJzdGFuZCB3aGF0IHRoZSBwcm9ibGVtIGlzLgp1c2Jfc2V0X2ludGVy ZmFjZSgpIGNhbGxzIHVzYl9kaXNhYmxlX2ludGVyZmFjZSgpIHdoaWNoIGFnYWluIGNhbGxzCnVz Yl9kaXNhYmxlX2VuZHBvaW50KCkuIFRoaXMgdXNiX2Rpc2FibGVfZW5kcG9pbnQoKSBnZXRzIHRo ZSBwb2ludGVyCnRvICdlcCcsIG1hcmtzIGl0IGFzIE5VTEwgYW5kIHNlbmRzIHRoZSBwb2ludGVy IHRvIHVzYl9oY2RfZmx1c2hfZW5kcG9pbnQoKS4KQWZ0ZXIgZmx1c2hpbmcgdGhlIGVuZHBvaW50 cyB1c2JfZGlzYWJsZV9lbmRwb2ludCgpIGNhbGxzCnVzYl9oY2RfZGlzYWJsZV9lbmRwb2ludCgp IHdoaWNoIHRyaWVzIHRvIGRvOgoJaWYgKGhjZC0+ZHJpdmVyLT5lbmRwb2ludF9kaXNhYmxlKQoJ CWhjZC0+ZHJpdmVyLT5lbmRwb2ludF9kaXNhYmxlKGhjZCwgZXApOwpidXQgdGhlcmUgaXMgbm8g ZW5kcG9pbnRfZGlzYWJsZSgpIGNhbGxiYWNrIGluIHhoY2ksIHNvIHRoZSBlbmRwb2ludCBpcwpu ZXZlciBtYXJrZWQgYXMgZGlzYWJsZWQuIFNvLCBuZXh0IHRpbWUgdXNiX2hjZF9mbHVzaF9lbmRw b2ludCgpIGlzCmNhbGxlZCBJIGdldCB0aGlzIGNvcnJ1cHRpb24uIApBbmQgdGhpcyBpcyBleGFj dGx5IHdoZXJlIEkgdXNlZCB0byBzZWUgdGhlIHByb2JsZW0gaGFwcGVuaW5nLgoKQW5kLCBteSBo YWNreSBwYXRjaCB3b3JrZWQgYXMgSSBwcmV2ZW50ZWQgaXQgZnJvbSBjYWxsaW5nCnVzYl9kaXNh YmxlX2ludGVyZmFjZSgpIGluIHRoaXMgcGFydGljdWxhciBjYXNlLgoKR3JlZyAtIGFuc3dlcmlu ZyB5b3VyIHF1ZXN0aW9uIGhlcmUuIE15IGhhY2t5IHBhdGNoIHdhcyBiYXNlZCBvbiB0aGUKZmFj dCB0aGF0IHVzYl9oY2RfYWxsb2NfYmFuZHdpZHRoKCkgaXMgY2FsbGluZyBoY2QtPmRyaXZlci0+ ZHJvcF9lbmRwb2ludCgpCmFuZCBoY2QtPmRyaXZlci0+YWRkX2VuZHBvaW50KCkgaWYgKGN1cl9h bHQgJiYgbmV3X2FsdCkuIFNvLCBJIHByZXZlbnRlZAp1c2JfZGlzYWJsZV9pbnRlcmZhY2UoKSB0 byBiZSBjYWxsZWQgZm9yIHRoYXQgc2FtZSBjb25kaXRpb24uIEFuZCB0aGF0CndvcmtlZCBhcyB0 aGUgY2FsbCB0byB1c2JfaGNkX2ZsdXNoX2VuZHBvaW50KCkgd2FzIG5vdCBleGVjdXRlZC4KSSBr bm93IGl0IGlzIG5vdCBjb3JyZWN0IGFuZCBJIG1pZ2h0IGJlIGhhdmluZyBtZW1vcnkgbGVha3Mg Zm9yIHRoaXMsIGJ1dApJIGhhdmUgdGhlIHN5c3RlbSB3b3JraW5nIHRpbGwgd2UgZ2V0IHRoZSBh Y3R1YWwgZml4LgotLS0KUmVnYXJkcwpTdWRpcAoKLS0KVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlz IGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LXVzYiIgaW4KdGhlIGJvZHkg b2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcKTW9yZSBtYWpvcmRvbW8g aW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sudip Mukherjee Subject: Re: usb HC busted? Date: Tue, 17 Jul 2018 16:10:17 +0100 Message-ID: <20180717151017.yk5d4glzwqcrxpqc@debian> References: <20180717114104.irgdb5rmg2qxclgp@debian> <20180717144022.4wabhdhysori3mvg@debian> <20180717144918.7ymzbhijxfcc3fhb@debian> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20180717144918.7ymzbhijxfcc3fhb@debian> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Alan Stern , Greg KH Cc: Mathias Nyman , Mathias Nyman , linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Christoph Hellwig , Andy Shevchenko , Andy Shevchenko , lukaszx.szulc-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org List-Id: iommu@lists.linux-foundation.org Hi Alan, Greg, On Tue, Jul 17, 2018 at 03:49:18PM +0100, Sudip Mukherjee wrote: > On Tue, Jul 17, 2018 at 03:40:22PM +0100, Sudip Mukherjee wrote: > > Hi Alan, > > > > On Tue, Jul 17, 2018 at 10:28:14AM -0400, Alan Stern wrote: > > > On Tue, 17 Jul 2018, Sudip Mukherjee wrote: > > > > > > > I did some more debugging. Tested with a KASAN enabled kernel and that > > > > shows the problem. The report is attached. > > > > > > > > To my understanding: > > > > > > > > btusb_work() is calling usb_set_interface() with alternate = 0. which > > > > again calls usb_hcd_alloc_bandwidth() and that frees the rings by > > > > xhci_free_endpoint_ring(). > > > > > > That doesn't sound like the right thing to do. The rings shouldn't be > > > freed until xhci_endpoint_disable() is called. > > > > > > On the other hand, there doesn't appear to be any > > > xhci_endpoint_disable() routine, although a comment refers to it. > > > Maybe this is the real problem? > > > > one of your old mail might help :) > > > > https://www.spinics.net/lists/linux-usb/msg98123.html > > Wrote too soon. > > Is it the one you are looking for - > usb_disable_endpoint() is in drivers/usb/core/message.c I think now I understand what the problem is. usb_set_interface() calls usb_disable_interface() which again calls usb_disable_endpoint(). This usb_disable_endpoint() gets the pointer to 'ep', marks it as NULL and sends the pointer to usb_hcd_flush_endpoint(). After flushing the endpoints usb_disable_endpoint() calls usb_hcd_disable_endpoint() which tries to do: if (hcd->driver->endpoint_disable) hcd->driver->endpoint_disable(hcd, ep); but there is no endpoint_disable() callback in xhci, so the endpoint is never marked as disabled. So, next time usb_hcd_flush_endpoint() is called I get this corruption. And this is exactly where I used to see the problem happening. And, my hacky patch worked as I prevented it from calling usb_disable_interface() in this particular case. Greg - answering your question here. My hacky patch was based on the fact that usb_hcd_alloc_bandwidth() is calling hcd->driver->drop_endpoint() and hcd->driver->add_endpoint() if (cur_alt && new_alt). So, I prevented usb_disable_interface() to be called for that same condition. And that worked as the call to usb_hcd_flush_endpoint() was not executed. I know it is not correct and I might be having memory leaks for this, but I have the system working till we get the actual fix. -- Regards Sudip