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: Johan Hovold Message-Id: <20180521083825.GO30172@localhost> Date: Mon, 21 May 2018 10:38:25 +0200 To: Greg Kroah-Hartman Cc: Arend van Spriel , Carlos Manuel Santos , Samuel Ortiz , Stephen Hemminger , linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org List-ID: T24gU3VuLCBNYXkgMjAsIDIwMTggYXQgMDM6MTk6NDZQTSArMDIwMCwgR3JlZyBLcm9haC1IYXJ0 bWFuIHdyb3RlOgo+IEl0J3MgYW1hemluZyB0aGF0IHRoaXMgZHJpdmVyIGV2ZXIgd29ya2VkLCBi dXQgbm93IHRoYXQgeDg2IGRvZXNuJ3QKPiBhbGxvdyBVU0IgZGF0YSB0byBiZSBzZW50IG9mZiBv ZiB0aGUgc3RhY2ssIGl0IHJlYWxseSBkb2VzIG5vdCB3b3JrIGF0Cj4gYWxsLiAgRml4IHRoaXMg dXAgYnkgcHJvcGVybHkgYWxsb2NhdGluZyB0aGUgZGF0YSBmb3IgdGhlIHNtYWxsCj4gImNvbW1h bmRzIiB0aGF0IGdldCBzZW50IHRvIHRoZSBkZXZpY2Ugb2ZmIG9mIHRoZSBzdGFjay4KPiAKPiBX ZSBkbyB0aGlzIGZvciBvbmUgY29tbWFuZCBieSBoYXZpbmcgYSB3aG9sZSB1cmIganVzdCBmb3Ig YWNrIG1lc3NhZ2VzLAo+IGFzIHRoZXkgY2FuIGJlIHN1Ym1pdHRlZCBpbiBpbnRlcnJ1cHQgY29u dGV4dCwgc28gd2UgY2FuIG5vdCB1c2UKPiB1c2JfYnVsa19tc2coKS4gIEJ1dCB0aGUgcG93ZXJv biBjb21tYW5kIGNhbiBzbGVlcCAoYW5kIGRvZXMpLCBzbyB1c2UKPiB1c2JfYnVsa19tc2coKSBm b3IgdGhhdCB0cmFuc2Zlci4KPiAKPiBSZXBvcnRlZC1ieTogQ2FybG9zIE1hbnVlbCBTYW50b3Mg PGNtbXBzYW50b3NAZ21haWwuY29tPgo+IENjOiBTYW11ZWwgT3J0aXogPHNhbWVvQGxpbnV4Lmlu dGVsLmNvbT4KPiBDYzogU3RlcGhlbiBIZW1taW5nZXIgPHN0ZXBoZW5AbmV0d29ya3BsdW1iZXIu b3JnPgo+IENjOiBzdGFibGUgPHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmc+Cj4gU2lnbmVkLW9mZi1i eTogR3JlZyBLcm9haC1IYXJ0bWFuIDxncmVna2hAbGludXhmb3VuZGF0aW9uLm9yZz4KPiAtLS0K PiB2NDogZG9uJ3QgdXNlIHVyYiB0cmFuc2ZlciBidWZmZXIgZmxhZ3MgYXMgdGhlIG1lbW9yeSBp cyB0aWVkIHRvIHRoZSB1cmIKPiAgICAgKHRoYW5rcyB0byBKb2hhbikgIE5vdyB3ZSBoYXZlIGEg bmV3IHN0YXRpYyB1cmIsIGFuZCB3ZSB1c2UKPiAgICAgdXNiX2J1bGtfbXNnKCkgZm9yIHRoZSBv dGhlciBtZXNzYWdlLgo+IHYzOiBhY3R1YWxseSB1c2UgdGhlIGNvcnJlY3QgYnVmZmVyICh0aGFu a3MgdG8gQXJlbmQgdmFuIFNwcmllbCkKPiAgICAgdXNlIGttZW1kdXAgKHRoYW5rcyB0byBKb2hh bm5lcyBCZXJnIGFuZCBKdWxpYSBMYXdhbGwpCj4gdjI6IHNldCB0aGUgdXJiIGZsYWdzIGNvcnJl Y3RseQoKWW91ciBjaGFuZ2VzIGxvb2sgY29ycmVjdCBub3cgc28gZmVlbCBmcmVlIHRvIGFkZDoK ClJldmlld2VkLWJ5OiBKb2hhbiBIb3ZvbGQgPGpvaGFuQGtlcm5lbC5vcmc+CgpJdCBzZWVtcyB3 ZSBjb3VsZCBlbmQgdXAgcmV0dXJuaW5nIGFuIGVycm5vIGZyb20gcHJvYmUgd2l0aCBhY3RpdmUg dXJicwooaWYgcG41MzNfZmluYWxpemVfc2V0dXAoKSBmYWlscykgaW4gd2hpY2ggY2FzZSB0aGUg YWNrIGJ1ZmZlciB3b3VsZApsZWFrLiBCdXQgZnJlZWluZyB0aGUgdXJicyB3aGlsZSBhY3RpdmUg d291bGQgdGhlbiBiZSB0aGUgYmlnZ2VyCnByb2JsZW0sIGFuZCB0aGF0IHdhc24ndCBpbnRyb2R1 Y2VkIGJ5IHRoaXMgcGF0Y2guCgpKb2hhbgotLS0KVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxp c3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LXVzYiIgaW4KdGhlIGJvZHkgb2Yg YSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcKTW9yZSBtYWpvcmRvbW8gaW5m byBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-lf0-f67.google.com ([209.85.215.67]:34977 "EHLO mail-lf0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750923AbeEUIie (ORCPT ); Mon, 21 May 2018 04:38:34 -0400 Date: Mon, 21 May 2018 10:38:25 +0200 From: Johan Hovold To: Greg Kroah-Hartman Cc: 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: <20180521083825.GO30172@localhost> (sfid-20180521_103838_652695_1BA8EFF6) References: <20180520131946.GA7325@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20180520131946.GA7325@kroah.com> Sender: linux-wireless-owner@vger.kernel.org List-ID: 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 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. Johan