From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [4/5] USB: cdc-acm: fix unthrottle races From: Oliver Neukum Message-Id: <1556532564.20085.10.camel@suse.com> Date: Mon, 29 Apr 2019 12:09:24 +0200 To: Johan Hovold , Greg Kroah-Hartman , Alan Stern Cc: linux-usb@vger.kernel.org List-ID: T24gRG8sIDIwMTktMDQtMjUgYXQgMTg6MDUgKzAyMDAsIEpvaGFuIEhvdm9sZCB3cm90ZToKPiBG aXggdHdvIGxvbmctc3RhbmRpbmcgYnVncyB3aGljaCBjb3VsZCBwb3RlbnRpYWxseSBsZWFkIHRv IG1lbW9yeQo+IGNvcnJ1cHRpb24gb3IgbGVhdmUgdGhlIHBvcnQgdGhyb3R0bGVkIHVudGlsIGl0 IGlzIHJlb3BlbmVkIChvbiB3ZWFrbHkKPiBvcmRlcmVkIHN5c3RlbXMpLCByZXNwZWN0aXZlbHks IHdoZW4gcmVhZC1VUkIgY29tcGxldGlvbiByYWNlcyB3aXRoCj4gdW50aHJvdHRsZSgpLgo+IAo+ IEZpcnN0LCB0aGUgVVJCIG11c3Qgbm90IGJlIG1hcmtlZCBhcyBmcmVlIGJlZm9yZSBwcm9jZXNz aW5nIGlzIGNvbXBsZXRlCj4gdG8gcHJldmVudCBpdCBmcm9tIGJlaW5nIHN1Ym1pdHRlZCBieSB1 bnRocm90dGxlKCkgb24gYW5vdGhlciBDUFUuCj4gCj4gICAgICAgICBDUFUgMSAgICAgICAgICAg ICAgICAgICAgICAgICAgIENQVSAyCj4gICAgICAgICA9PT09PT09PT09PT09PT09ICAgICAgICAg ICAgICAgID09PT09PT09PT09PT09PT0KPiAgICAgICAgIGNvbXBsZXRlKCkgICAgICAgICAgICAg ICAgICAgICAgdW50aHJvdHRsZSgpCj4gICAgICAgICAgIHByb2Nlc3NfdXJiKCk7Cj4gICAgICAg ICAgIHNtcF9tYl9fYmVmb3JlX2F0b21pYygpOwo+ICAgICAgICAgICBzZXRfYml0KGksIGZyZWUp OyAgICAgICAgICAgICAgIGlmICh0ZXN0X2FuZF9jbGVhcl9iaXQoaSwgZnJlZSkpCj4gICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzdWJtaXRfdXJiKCk7 Cj4gCj4gU2Vjb25kLCB0aGUgVVJCIG11c3QgYmUgbWFya2VkIGFzIGZyZWUgYmVmb3JlIGNoZWNr aW5nIHRoZSB0aHJvdHRsZWQKPiBmbGFnIHRvIHByZXZlbnQgdW50aHJvdHRsZSgpIG9uIGFub3Ro ZXIgQ1BVIGZyb20gZmFpbGluZyB0byBvYnNlcnZlIHRoYXQKPiB0aGUgVVJCIG5lZWRzIHRvIGJl IHN1Ym1pdHRlZCBpZiBjb21wbGV0ZSgpIHNlZXMgdGhhdCB0aGUgdGhyb3R0bGVkIGZsYWcKPiBp cyBzZXQuCj4gCj4gICAgICAgICBDUFUgMSAgICAgICAgICAgICAgICAgICAgICAgICAgIENQVSAy Cj4gICAgICAgICA9PT09PT09PT09PT09PT09ICAgICAgICAgICAgICAgID09PT09PT09PT09PT09 PT0KPiAgICAgICAgIGNvbXBsZXRlKCkgICAgICAgICAgICAgICAgICAgICAgdW50aHJvdHRsZSgp Cj4gICAgICAgICAgIHNldF9iaXQoaSwgZnJlZSk7ICAgICAgICAgICAgICAgdGhyb3R0bGVkID0g MDsKPiAgICAgICAgICAgc21wX21iX19hZnRlcl9hdG9taWMoKTsgICAgICAgICBzbXBfbWIoKTsK PiAgICAgICAgICAgaWYgKHRocm90dGxlZCkgICAgICAgICAgICAgICAgICBpZiAodGVzdF9hbmRf Y2xlYXJfYml0KGksIGZyZWUpKQo+ICAgICAgICAgICAgICAgICAgIHJldHVybjsgICAgICAgICAg ICAgICAgICAgICAgICAgc3VibWl0X3VyYigpOwo+IAo+IE5vdGUgdGhhdCB0ZXN0X2FuZF9jbGVh cl9iaXQoKSBvbmx5IGltcGxpZXMgYmFycmllcnMgd2hlbiB0aGUgdGVzdCBpcwo+IHN1Y2Nlc3Nm dWwuIFRvIGhhbmRsZSB0aGUgY2FzZSB3aGVyZSB0aGUgVVJCIGlzIHN0aWxsIGluIHVzZSBhbiBl eHBsaWNpdAo+IGJhcnJpZXIgbmVlZHMgdG8gYmUgYWRkZWQgdG8gdW50aHJvdHRsZSgpIGZvciB0 aGUgc2Vjb25kIHJhY2UgY29uZGl0aW9uLgo+IAo+IEFsc28gbm90ZSB0aGF0IHRoZSBmaXJzdCBy YWNlIHdhcyBmaXhlZCBieSAzNmU1OWUwZDcwZDYgKCJjZGMtYWNtOiBmaXgKPiByYWNlIGJldHdl ZW4gY2FsbGJhY2sgYW5kIHVudGhyb3R0bGUiKSBiYWNrIGluIDIwMTUsIGJ1dCB0aGUgYnVnIHdh cwo+IHJlaW50cm9kdWNlZCBhIHllYXIgbGF0ZXIuCj4gCj4gRml4ZXM6IDFhYmE1NzlmM2NmNSAo ImNkYy1hY206IGhhbmRsZSByZWFkIHBpcGUgZXJyb3JzIikKPiBGaXhlczogMDg4YzY0ZjgxMjg0 ICgiVVNCOiBjZGMtYWNtOiByZS13cml0ZSByZWFkIHByb2Nlc3NpbmciKQo+IFNpZ25lZC1vZmYt Ynk6IEpvaGFuIEhvdm9sZCA8am9oYW5Aa2VybmVsLm9yZz4KQWNrZWQtYnk6IE9saXZlciBOZXVr dW0gPG9uZXVrdW1Ac3VzZS5jb20+Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E598DC43219 for ; Mon, 29 Apr 2019 10:09:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AC3FB2087B for ; Mon, 29 Apr 2019 10:09:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727537AbfD2KJi (ORCPT ); Mon, 29 Apr 2019 06:09:38 -0400 Received: from mx2.suse.de ([195.135.220.15]:60222 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727428AbfD2KJi (ORCPT ); Mon, 29 Apr 2019 06:09:38 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id C8794AC50; Mon, 29 Apr 2019 10:09:36 +0000 (UTC) Message-ID: <1556532564.20085.10.camel@suse.com> Subject: Re: [PATCH 4/5] USB: cdc-acm: fix unthrottle races From: Oliver Neukum To: Johan Hovold , Greg Kroah-Hartman , Alan Stern Cc: linux-usb@vger.kernel.org Date: Mon, 29 Apr 2019 12:09:24 +0200 In-Reply-To: <20190425160540.10036-5-johan@kernel.org> References: <20190425160540.10036-1-johan@kernel.org> <20190425160540.10036-5-johan@kernel.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.26.6 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org Message-ID: <20190429100924.AMg41kpwK1lSFJDXDJ278ZxuY1blGjCBhBLHDsTg3Jc@z> On Do, 2019-04-25 at 18:05 +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. > > Also note that the first race was fixed by 36e59e0d70d6 ("cdc-acm: fix > race between callback and unthrottle") back in 2015, but the bug was > reintroduced a year later. > > Fixes: 1aba579f3cf5 ("cdc-acm: handle read pipe errors") > Fixes: 088c64f81284 ("USB: cdc-acm: re-write read processing") > Signed-off-by: Johan Hovold Acked-by: Oliver Neukum