On Mon, May 13, 2019 at 12:43:39PM +0200, Johan Hovold wrote: > On Thu, Apr 25, 2019 at 06:05:36PM +0200, Johan Hovold wrote: > > Fix two long-standing bugs which could potentially lead to memory > > corruption or leave the port throttled until it is reopened (on weakly > > ordered systems), respectively, when read-URB completion races with > > unthrottle(). > > > > First, the URB must not be marked as free before processing is complete > > to prevent it from being submitted by unthrottle() on another CPU. > > > > CPU 1 CPU 2 > > ================ ================ > > complete() unthrottle() > > process_urb(); > > smp_mb__before_atomic(); > > set_bit(i, free); if (test_and_clear_bit(i, free)) > > submit_urb(); > > > > Second, the URB must be marked as free before checking the throttled > > flag to prevent unthrottle() on another CPU from failing to observe that > > the URB needs to be submitted if complete() sees that the throttled flag > > is set. > > > > CPU 1 CPU 2 > > ================ ================ > > complete() unthrottle() > > set_bit(i, free); throttled = 0; > > smp_mb__after_atomic(); smp_mb(); > > if (throttled) if (test_and_clear_bit(i, free)) > > return; submit_urb(); > > > > Note that test_and_clear_bit() only implies barriers when the test is > > successful. To handle the case where the URB is still in use an explicit > > barrier needs to be added to unthrottle() for the second race condition. > > > > Fixes: d83b405383c9 ("USB: serial: add support for multiple read urbs") > > Signed-off-by: Johan Hovold <johan@xxxxxxxxxx> > > Greg, I noticed you added a stable tag to the corresponding cdc-acm fix > and think I should have added on one from the start to this one as well. > > Would you mind queuing this one up for stable? > > Upstream commit 3f5edd58d040bfa4b74fb89bc02f0bc6b9cd06ab. Sure, now queued up for 4.9+ thanks, greg k-h