Skip to contentSkip to conversation

NFC: pn533: don't send USB data off of the stack

PATCH Superseded Greg Kroah-Hartman 1 participant 1 message Jun 1, 2018 USB

lore
Share & tools
lore

Greg Kroah-Hartman

author maintainer
#0 lore raw

PATCH v4 1/1 NFC: pn533: don't send USB data off of the stack

Quoted 41 lines · Mark Greer
Quoted 31 lines · Greg Kroah-Hartman
Quoted 24 lines · Johan Hovold
Quoted 15 lines · Greg Kroah-Hartman

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.

Quoted 7 lines

---
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

Quoted 3 lines

Your changes look correct now so feel free to add:

Thanks for the review.

Quoted 8 lines
Quoted 4 lines

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 :)

Quoted 8 lines

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
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html