On Thu, 25 Apr 2019, Johan Hovold wrote: > This series fixes a couple of long-standing issues in USB serial and > cdc-acm which essentially share the same implementation. > > As noted by Oliver a few years back, read-urb completion can race with > unthrottle() running on another CPU and this can potentially lead to > memory corruption. This particular bug in cdc-acm was unfortunately > reintroduced a year later. > > There's also a second race due to missing memory barriers which could > theoretically lead to the port staying throttled until reopened on > weakly ordered systems. A second set of memory barriers should address > that. > > I would appreciate your keen eyes on this one to make sure I got the > barriers right. > > I noticed there's some on-going discussion about the atomic memory > barriers that Alan's involved in, and I'll try to catch up on his > data-race work as well. I'm still a little concerned about whether the > smp_mb__before_atomic() is sufficient to prevent the compiler from > messing things up without adding READ_ONCE(). I think your changes in patches 1 and 4 are correct. Regardless of the issues still undergoing discussion elsewhere, smp_mb__before_atomic() should cause both the compiler and the CPU to order every preceding operation (READ_ONCE or not) before the atomic op. Alan Stern > Note that none of these have stable tags as the issues have been there > for eight years or so without anyone noticing (besides Oliver). > > Still feels good to clean up your own mess. > > Note that the cdc-acm patches have so far only been compile tested. > > Johan > > > Johan Hovold (5): > USB: serial: fix unthrottle races > USB: serial: clean up throttle handling > USB: serial: generic: drop unnecessary goto > USB: cdc-acm: fix unthrottle races > USB: cdc-acm: clean up throttle handling > > drivers/usb/class/cdc-acm.c | 63 +++++++++++++++--------------- > drivers/usb/class/cdc-acm.h | 3 +- > drivers/usb/serial/generic.c | 76 +++++++++++++++++++----------------- > include/linux/usb/serial.h | 5 +-- > 4 files changed, 75 insertions(+), 72 deletions(-) > >
