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. Thanks, Johan