Re: [PATCH 0/5] USB: fix tty unthrottle races

[Alan Stern <stern@rowland.harvard.edu>]

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