* Re: [PATCH 1/5] USB: serial: fix unthrottle races [not found] ` <20190425160540.10036-2-johan@kernel.org> @ 2019-05-13 10:43 ` Johan Hovold 2019-05-13 10:56 ` Greg Kroah-Hartman 0 siblings, 1 reply; 7+ messages in thread From: Johan Hovold @ 2019-05-13 10:43 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-usb, Johan Hovold, Alan Stern, Oliver Neukum, stable 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@kernel.org> 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/5] USB: serial: fix unthrottle races 2019-05-13 10:43 ` [PATCH 1/5] USB: serial: fix unthrottle races Johan Hovold @ 2019-05-13 10:56 ` Greg Kroah-Hartman 2019-05-13 11:46 ` Johan Hovold 0 siblings, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2019-05-13 10:56 UTC (permalink / raw) To: Johan Hovold; +Cc: linux-usb, Alan Stern, Oliver Neukum, stable 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@kernel.org> > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/5] USB: serial: fix unthrottle races 2019-05-13 10:56 ` Greg Kroah-Hartman @ 2019-05-13 11:46 ` Johan Hovold 2019-05-13 12:51 ` Greg Kroah-Hartman 0 siblings, 1 reply; 7+ messages in thread From: Johan Hovold @ 2019-05-13 11:46 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Johan Hovold, linux-usb, Alan Stern, Oliver Neukum, stable On Mon, May 13, 2019 at 12:56:06PM +0200, Greg Kroah-Hartman wrote: > 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(). > > > Fixes: d83b405383c9 ("USB: serial: add support for multiple read urbs") > > > Signed-off-by: Johan Hovold <johan@kernel.org> > > > > 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. The issue has been there since v3.3 so I guess you could queue it for all stable trees. Johan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/5] USB: serial: fix unthrottle races 2019-05-13 11:46 ` Johan Hovold @ 2019-05-13 12:51 ` Greg Kroah-Hartman 2019-05-13 12:59 ` Johan Hovold 0 siblings, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2019-05-13 12:51 UTC (permalink / raw) To: Johan Hovold; +Cc: linux-usb, Alan Stern, Oliver Neukum, stable On Mon, May 13, 2019 at 01:46:01PM +0200, Johan Hovold wrote: > On Mon, May 13, 2019 at 12:56:06PM +0200, Greg Kroah-Hartman wrote: > > 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(). > > > > > Fixes: d83b405383c9 ("USB: serial: add support for multiple read urbs") > > > > Signed-off-by: Johan Hovold <johan@kernel.org> > > > > > > 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. The issue has been there since v3.3 so I guess you could queue > it for all stable trees. Doesn't apply cleanly for 4.4.y or 3.18.y, so if it's really worth adding there (and I kind of doubt it), I would need a backport :) thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/5] USB: serial: fix unthrottle races 2019-05-13 12:51 ` Greg Kroah-Hartman @ 2019-05-13 12:59 ` Johan Hovold 2019-05-14 12:53 ` Sasha Levin 0 siblings, 1 reply; 7+ messages in thread From: Johan Hovold @ 2019-05-13 12:59 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Johan Hovold, linux-usb, Alan Stern, Oliver Neukum, stable On Mon, May 13, 2019 at 02:51:31PM +0200, Greg Kroah-Hartman wrote: > On Mon, May 13, 2019 at 01:46:01PM +0200, Johan Hovold wrote: > > Thanks. The issue has been there since v3.3 so I guess you could queue > > it for all stable trees. > > Doesn't apply cleanly for 4.4.y or 3.18.y, so if it's really worth > adding there (and I kind of doubt it), I would need a backport :) Ah ok, just wasn't sure why you chose 4.9+. Johan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/5] USB: serial: fix unthrottle races 2019-05-13 12:59 ` Johan Hovold @ 2019-05-14 12:53 ` Sasha Levin 2019-05-14 12:57 ` Johan Hovold 0 siblings, 1 reply; 7+ messages in thread From: Sasha Levin @ 2019-05-14 12:53 UTC (permalink / raw) To: Johan Hovold Cc: Greg Kroah-Hartman, linux-usb, Alan Stern, Oliver Neukum, stable On Mon, May 13, 2019 at 02:59:09PM +0200, Johan Hovold wrote: >On Mon, May 13, 2019 at 02:51:31PM +0200, Greg Kroah-Hartman wrote: >> On Mon, May 13, 2019 at 01:46:01PM +0200, Johan Hovold wrote: > >> > Thanks. The issue has been there since v3.3 so I guess you could queue >> > it for all stable trees. >> >> Doesn't apply cleanly for 4.4.y or 3.18.y, so if it's really worth >> adding there (and I kind of doubt it), I would need a backport :) > >Ah ok, just wasn't sure why you chose 4.9+. On 4.4 and 3.18 it just had a contextual conflict because of 3161da970d38c ("USB: serial: use variable for status"), I can queue both 3161da970d38c and 3f5edd58d040b for 4.4 and 3.18. -- Thanks, Sasha ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/5] USB: serial: fix unthrottle races 2019-05-14 12:53 ` Sasha Levin @ 2019-05-14 12:57 ` Johan Hovold 0 siblings, 0 replies; 7+ messages in thread From: Johan Hovold @ 2019-05-14 12:57 UTC (permalink / raw) To: Sasha Levin Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, Alan Stern, Oliver Neukum, stable On Tue, May 14, 2019 at 08:53:53AM -0400, Sasha Levin wrote: > On Mon, May 13, 2019 at 02:59:09PM +0200, Johan Hovold wrote: > >On Mon, May 13, 2019 at 02:51:31PM +0200, Greg Kroah-Hartman wrote: > >> On Mon, May 13, 2019 at 01:46:01PM +0200, Johan Hovold wrote: > > > >> > Thanks. The issue has been there since v3.3 so I guess you could queue > >> > it for all stable trees. > >> > >> Doesn't apply cleanly for 4.4.y or 3.18.y, so if it's really worth > >> adding there (and I kind of doubt it), I would need a backport :) > > > >Ah ok, just wasn't sure why you chose 4.9+. > > On 4.4 and 3.18 it just had a contextual conflict because of > 3161da970d38c ("USB: serial: use variable for status"), I can queue both > 3161da970d38c and 3f5edd58d040b for 4.4 and 3.18. Sounds good, thanks! Johan ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-05-14 12:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20190425160540.10036-1-johan@kernel.org>
[not found] ` <20190425160540.10036-2-johan@kernel.org>
2019-05-13 10:43 ` [PATCH 1/5] USB: serial: fix unthrottle races Johan Hovold
2019-05-13 10:56 ` Greg Kroah-Hartman
2019-05-13 11:46 ` Johan Hovold
2019-05-13 12:51 ` Greg Kroah-Hartman
2019-05-13 12:59 ` Johan Hovold
2019-05-14 12:53 ` Sasha Levin
2019-05-14 12:57 ` Johan Hovold
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).