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: [v4] NFC: pn533: don't send USB data off of the stack From: Greg Kroah-Hartman Message-Id: <20180601080346.GC12809@kroah.com> Date: Fri, 1 Jun 2018 10:03:46 +0200 To: Mark Greer Cc: Johan Hovold , Arend van Spriel , Carlos Manuel Santos , Samuel Ortiz , Stephen Hemminger , linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org List-ID: T24gVGh1LCBNYXkgMzEsIDIwMTggYXQgMDk6MDY6MzJQTSAtMDcwMCwgTWFyayBHcmVlciB3cm90 ZToKPiBPbiBUaHUsIE1heSAzMSwgMjAxOCBhdCAxMjozODo0OFBNICswMjAwLCBHcmVnIEtyb2Fo LUhhcnRtYW4gd3JvdGU6Cj4gPiBPbiBNb24sIE1heSAyMSwgMjAxOCBhdCAxMDozODoyNUFNICsw MjAwLCBKb2hhbiBIb3ZvbGQgd3JvdGU6Cj4gPiA+IE9uIFN1biwgTWF5IDIwLCAyMDE4IGF0IDAz OjE5OjQ2UE0gKzAyMDAsIEdyZWcgS3JvYWgtSGFydG1hbiB3cm90ZToKPiA+ID4gPiBJdCdzIGFt YXppbmcgdGhhdCB0aGlzIGRyaXZlciBldmVyIHdvcmtlZCwgYnV0IG5vdyB0aGF0IHg4NiBkb2Vz bid0Cj4gPiA+ID4gYWxsb3cgVVNCIGRhdGEgdG8gYmUgc2VudCBvZmYgb2YgdGhlIHN0YWNrLCBp dCByZWFsbHkgZG9lcyBub3Qgd29yayBhdAo+ID4gPiA+IGFsbC4gIEZpeCB0aGlzIHVwIGJ5IHBy b3Blcmx5IGFsbG9jYXRpbmcgdGhlIGRhdGEgZm9yIHRoZSBzbWFsbAo+ID4gPiA+ICJjb21tYW5k cyIgdGhhdCBnZXQgc2VudCB0byB0aGUgZGV2aWNlIG9mZiBvZiB0aGUgc3RhY2suCj4gPiA+ID4g Cj4gPiA+ID4gV2UgZG8gdGhpcyBmb3Igb25lIGNvbW1hbmQgYnkgaGF2aW5nIGEgd2hvbGUgdXJi IGp1c3QgZm9yIGFjayBtZXNzYWdlcywKPiA+ID4gPiBhcyB0aGV5IGNhbiBiZSBzdWJtaXR0ZWQg aW4gaW50ZXJydXB0IGNvbnRleHQsIHNvIHdlIGNhbiBub3QgdXNlCj4gPiA+ID4gdXNiX2J1bGtf bXNnKCkuICBCdXQgdGhlIHBvd2Vyb24gY29tbWFuZCBjYW4gc2xlZXAgKGFuZCBkb2VzKSwgc28g dXNlCj4gPiA+ID4gdXNiX2J1bGtfbXNnKCkgZm9yIHRoYXQgdHJhbnNmZXIuCj4gPiA+ID4gCj4g PiA+ID4gUmVwb3J0ZWQtYnk6IENhcmxvcyBNYW51ZWwgU2FudG9zIDxjbW1wc2FudG9zQGdtYWls LmNvbT4KPiA+ID4gPiBDYzogU2FtdWVsIE9ydGl6IDxzYW1lb0BsaW51eC5pbnRlbC5jb20+Cj4g PiA+ID4gQ2M6IFN0ZXBoZW4gSGVtbWluZ2VyIDxzdGVwaGVuQG5ldHdvcmtwbHVtYmVyLm9yZz4K PiA+ID4gPiBDYzogc3RhYmxlIDxzdGFibGVAdmdlci5rZXJuZWwub3JnPgo+ID4gPiA+IFNpZ25l ZC1vZmYtYnk6IEdyZWcgS3JvYWgtSGFydG1hbiA8Z3JlZ2toQGxpbnV4Zm91bmRhdGlvbi5vcmc+ Cj4gPiA+ID4gLS0tCj4gPiA+ID4gdjQ6IGRvbid0IHVzZSB1cmIgdHJhbnNmZXIgYnVmZmVyIGZs YWdzIGFzIHRoZSBtZW1vcnkgaXMgdGllZCB0byB0aGUgdXJiCj4gPiA+ID4gICAgICh0aGFua3Mg dG8gSm9oYW4pICBOb3cgd2UgaGF2ZSBhIG5ldyBzdGF0aWMgdXJiLCBhbmQgd2UgdXNlCj4gPiA+ ID4gICAgIHVzYl9idWxrX21zZygpIGZvciB0aGUgb3RoZXIgbWVzc2FnZS4KPiA+ID4gPiB2Mzog YWN0dWFsbHkgdXNlIHRoZSBjb3JyZWN0IGJ1ZmZlciAodGhhbmtzIHRvIEFyZW5kIHZhbiBTcHJp ZWwpCj4gPiA+ID4gICAgIHVzZSBrbWVtZHVwICh0aGFua3MgdG8gSm9oYW5uZXMgQmVyZyBhbmQg SnVsaWEgTGF3YWxsKQo+ID4gPiA+IHYyOiBzZXQgdGhlIHVyYiBmbGFncyBjb3JyZWN0bHkKPiA+ ID4gCj4gPiA+IFlvdXIgY2hhbmdlcyBsb29rIGNvcnJlY3Qgbm93IHNvIGZlZWwgZnJlZSB0byBh ZGQ6Cj4gPiA+IAo+ID4gPiBSZXZpZXdlZC1ieTogSm9oYW4gSG92b2xkIDxqb2hhbkBrZXJuZWwu b3JnPgo+ID4gCj4gPiBUaGFua3MgZm9yIHRoZSByZXZpZXcuCj4gPiAKPiA+ID4gSXQgc2VlbXMg d2UgY291bGQgZW5kIHVwIHJldHVybmluZyBhbiBlcnJubyBmcm9tIHByb2JlIHdpdGggYWN0aXZl IHVyYnMKPiA+ID4gKGlmIHBuNTMzX2ZpbmFsaXplX3NldHVwKCkgZmFpbHMpIGluIHdoaWNoIGNh c2UgdGhlIGFjayBidWZmZXIgd291bGQKPiA+ID4gbGVhay4gQnV0IGZyZWVpbmcgdGhlIHVyYnMg d2hpbGUgYWN0aXZlIHdvdWxkIHRoZW4gYmUgdGhlIGJpZ2dlcgo+ID4gPiBwcm9ibGVtLCBhbmQg dGhhdCB3YXNuJ3QgaW50cm9kdWNlZCBieSB0aGlzIHBhdGNoLgo+ID4gCj4gPiBZZWFoLCB0aGlz IHdob2xlIHRoaW5nIGlzIGEgbWVzcyBhbmQgSSByZWFsbHkgZG9uJ3Qgd2FudCB0byBkbyBtdWNo IG1vcmUKPiA+IHRvIHRoZSBkcml2ZXIgd2l0aG91dCBoYXZpbmcgdGhlIGhhcmR3YXJlIHRvIHRl c3Qgd2l0aCBpdC4gIFNvIEknbGwganVzdAo+ID4gbGVhdmUgaXQgYXMtaXMgZm9yIG5vdyA6KQo+ IAo+IEhpIEdyZWcuCj4gCj4gSSBoYXZlIGEgcG41MzMgVVNCIHN0aWNrLCBhIHRyZjc5NzAtYmFz ZWQgYm9hcmQsIGFuZCBzb21lIG5mYyB0YWdzIHNvIEkKPiBjYW4gaGVscCB5b3UgdGVzdCBhcyBs b25nIGFzIHlvdSBkb24ndCBtaW5kIG11bHRpLWRheSB0dXJuLWFyb3VuZCB0aW1lcy4KPiBBbHRl cm5hdGl2ZWx5LCBJIGNhbiBzZXQgc29tZXRoaW5nIHVwIGFuZCBnaXZlIHlvdSByZW1vdGUgYWNj ZXNzLCBpZiB5b3UKPiBwcmVmZXIgdGhhdC4KPiAKPiBMZXQgbWUga25vdy4KCm11bHRpLWRheSB0 dXJuIGFyb3VuZHMgYXJlIGZpbmUuICBCdXQgZ2l2ZW4gdGhhdCB0aGlzIGRyaXZlciBoYXMgYmVl bgpicm9rZW4gZm9yIGEgdmVyeSBsb25nIHRpbWUgd2l0aCBubyBvbmUgbm90aWNpbmcsIEkgZG91 YnQgdGhhdCB0aGVyZSBhcmUKbWFueSwgaWYgYW55LCByZWFsIHVzZXJzIG91dCB0aGVyZSB0byBt YWtlIGl0IHdvcnRoIHdvcmtpbmcgb24gYXQgdGhlCm1vbWVudCB0byBmaXggdXAgbWlub3IgY29k ZSBxdWFsaXR5IGlzc3VlcyA6KAoKdGhhbmtzLAoKZ3JlZyBrLWgKLS0tClRvIHVuc3Vic2NyaWJl IGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51eC11c2IiIGlu CnRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnCk1vcmUg bWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8u aHRtbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail.kernel.org ([198.145.29.99]:41906 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751057AbeFAIEI (ORCPT ); Fri, 1 Jun 2018 04:04:08 -0400 Date: Fri, 1 Jun 2018 10:03:46 +0200 From: Greg Kroah-Hartman To: Mark Greer Cc: Johan Hovold , Arend van Spriel , Carlos Manuel Santos , Samuel Ortiz , Stephen Hemminger , linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org Subject: Re: [PATCH v4] NFC: pn533: don't send USB data off of the stack Message-ID: <20180601080346.GC12809@kroah.com> (sfid-20180601_100414_423390_7294A91A) References: <20180520131946.GA7325@kroah.com> <20180521083825.GO30172@localhost> <20180531103848.GB31952@kroah.com> <20180601040632.GA14834@animalcreek.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20180601040632.GA14834@animalcreek.com> Sender: linux-wireless-owner@vger.kernel.org List-ID: On Thu, May 31, 2018 at 09:06:32PM -0700, Mark Greer wrote: > On Thu, May 31, 2018 at 12:38:48PM +0200, Greg Kroah-Hartman wrote: > > On Mon, May 21, 2018 at 10:38:25AM +0200, Johan Hovold wrote: > > > On Sun, May 20, 2018 at 03:19:46PM +0200, Greg Kroah-Hartman wrote: > > > > It's amazing that this driver ever worked, but now that x86 doesn't > > > > allow USB data to be sent off of the stack, it really does not work at > > > > all. Fix this up by properly allocating the data for the small > > > > "commands" that get sent to the device off of the stack. > > > > > > > > We do this for one command by having a whole urb just for ack messages, > > > > as they can be submitted in interrupt context, so we can not use > > > > usb_bulk_msg(). But the poweron command can sleep (and does), so use > > > > usb_bulk_msg() for that transfer. > > > > > > > > Reported-by: Carlos Manuel Santos > > > > Cc: Samuel Ortiz > > > > Cc: Stephen Hemminger > > > > Cc: stable > > > > Signed-off-by: Greg Kroah-Hartman > > > > --- > > > > v4: don't use urb transfer buffer flags as the memory is tied to the urb > > > > (thanks to Johan) Now we have a new static urb, and we use > > > > usb_bulk_msg() for the other message. > > > > v3: actually use the correct buffer (thanks to Arend van Spriel) > > > > use kmemdup (thanks to Johannes Berg and Julia Lawall) > > > > v2: set the urb flags correctly > > > > > > Your changes look correct now so feel free to add: > > > > > > Reviewed-by: Johan Hovold > > > > Thanks for the review. > > > > > It seems we could end up returning an errno from probe with active urbs > > > (if pn533_finalize_setup() fails) in which case the ack buffer would > > > leak. But freeing the urbs while active would then be the bigger > > > problem, and that wasn't introduced by this patch. > > > > Yeah, this whole thing is a mess and I really don't want to do much more > > to the driver without having the hardware to test with it. So I'll just > > leave it as-is for now :) > > Hi Greg. > > I have a pn533 USB stick, a trf7970-based board, and some nfc tags so I > can help you test as long as you don't mind multi-day turn-around times. > Alternatively, I can set something up and give you remote access, if you > prefer that. > > Let me know. multi-day turn arounds are fine. But given that this driver has been broken for a very long time with no one noticing, I doubt that there are many, if any, real users out there to make it worth working on at the moment to fix up minor code quality issues :( thanks, greg k-h