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: [1/5] USB: serial: fix unthrottle races From: Johan Hovold Message-Id: <20190425160540.10036-2-johan@kernel.org> Date: Thu, 25 Apr 2019 18:05:36 +0200 To: Alan Stern , Oliver Neukum , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, Johan Hovold List-ID: Rml4IHR3byBsb25nLXN0YW5kaW5nIGJ1Z3Mgd2hpY2ggY291bGQgcG90ZW50aWFsbHkgbGVhZCB0 byBtZW1vcnkKY29ycnVwdGlvbiBvciBsZWF2ZSB0aGUgcG9ydCB0aHJvdHRsZWQgdW50aWwgaXQg aXMgcmVvcGVuZWQgKG9uIHdlYWtseQpvcmRlcmVkIHN5c3RlbXMpLCByZXNwZWN0aXZlbHksIHdo ZW4gcmVhZC1VUkIgY29tcGxldGlvbiByYWNlcyB3aXRoCnVudGhyb3R0bGUoKS4KCkZpcnN0LCB0 aGUgVVJCIG11c3Qgbm90IGJlIG1hcmtlZCBhcyBmcmVlIGJlZm9yZSBwcm9jZXNzaW5nIGlzIGNv bXBsZXRlCnRvIHByZXZlbnQgaXQgZnJvbSBiZWluZyBzdWJtaXR0ZWQgYnkgdW50aHJvdHRsZSgp IG9uIGFub3RoZXIgQ1BVLgoKCUNQVSAxCQkJCUNQVSAyCgk9PT09PT09PT09PT09PT09CQk9PT09 PT09PT09PT09PT09Cgljb21wbGV0ZSgpCQkJdW50aHJvdHRsZSgpCgkgIHByb2Nlc3NfdXJiKCk7 CgkgIHNtcF9tYl9fYmVmb3JlX2F0b21pYygpOwoJICBzZXRfYml0KGksIGZyZWUpOwkJICBpZiAo dGVzdF9hbmRfY2xlYXJfYml0KGksIGZyZWUpKQoJICAJCQkJCSAgc3VibWl0X3VyYigpOwoKU2Vj b25kLCB0aGUgVVJCIG11c3QgYmUgbWFya2VkIGFzIGZyZWUgYmVmb3JlIGNoZWNraW5nIHRoZSB0 aHJvdHRsZWQKZmxhZyB0byBwcmV2ZW50IHVudGhyb3R0bGUoKSBvbiBhbm90aGVyIENQVSBmcm9t IGZhaWxpbmcgdG8gb2JzZXJ2ZSB0aGF0CnRoZSBVUkIgbmVlZHMgdG8gYmUgc3VibWl0dGVkIGlm IGNvbXBsZXRlKCkgc2VlcyB0aGF0IHRoZSB0aHJvdHRsZWQgZmxhZwppcyBzZXQuCgoJQ1BVIDEJ CQkJQ1BVIDIKCT09PT09PT09PT09PT09PT0JCT09PT09PT09PT09PT09PT0KCWNvbXBsZXRlKCkJ CQl1bnRocm90dGxlKCkKCSAgc2V0X2JpdChpLCBmcmVlKTsJCSAgdGhyb3R0bGVkID0gMDsKCSAg c21wX21iX19hZnRlcl9hdG9taWMoKTsJICBzbXBfbWIoKTsKCSAgaWYgKHRocm90dGxlZCkJCSAg aWYgKHRlc3RfYW5kX2NsZWFyX2JpdChpLCBmcmVlKSkKCSAgCSAgcmV0dXJuOwkJCSAgc3VibWl0 X3VyYigpOwoKTm90ZSB0aGF0IHRlc3RfYW5kX2NsZWFyX2JpdCgpIG9ubHkgaW1wbGllcyBiYXJy aWVycyB3aGVuIHRoZSB0ZXN0IGlzCnN1Y2Nlc3NmdWwuIFRvIGhhbmRsZSB0aGUgY2FzZSB3aGVy ZSB0aGUgVVJCIGlzIHN0aWxsIGluIHVzZSBhbiBleHBsaWNpdApiYXJyaWVyIG5lZWRzIHRvIGJl IGFkZGVkIHRvIHVudGhyb3R0bGUoKSBmb3IgdGhlIHNlY29uZCByYWNlIGNvbmRpdGlvbi4KCkZp eGVzOiBkODNiNDA1MzgzYzkgKCJVU0I6IHNlcmlhbDogYWRkIHN1cHBvcnQgZm9yIG11bHRpcGxl IHJlYWQgdXJicyIpClNpZ25lZC1vZmYtYnk6IEpvaGFuIEhvdm9sZCA8am9oYW5Aa2VybmVsLm9y Zz4KLS0tCiBkcml2ZXJzL3VzYi9zZXJpYWwvZ2VuZXJpYy5jIHwgMzkgKysrKysrKysrKysrKysr KysrKysrKysrKysrKystLS0tLS0tCiAxIGZpbGUgY2hhbmdlZCwgMzIgaW5zZXJ0aW9ucygrKSwg NyBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9kcml2ZXJzL3VzYi9zZXJpYWwvZ2VuZXJpYy5j IGIvZHJpdmVycy91c2Ivc2VyaWFsL2dlbmVyaWMuYwppbmRleCAyMjc0ZDk2MjVmNjMuLjBmZmY0 OTY4ZWExYiAxMDA2NDQKLS0tIGEvZHJpdmVycy91c2Ivc2VyaWFsL2dlbmVyaWMuYworKysgYi9k cml2ZXJzL3VzYi9zZXJpYWwvZ2VuZXJpYy5jCkBAIC0zNzYsNiArMzc2LDcgQEAgdm9pZCB1c2Jf c2VyaWFsX2dlbmVyaWNfcmVhZF9idWxrX2NhbGxiYWNrKHN0cnVjdCB1cmIgKnVyYikKIAlzdHJ1 Y3QgdXNiX3NlcmlhbF9wb3J0ICpwb3J0ID0gdXJiLT5jb250ZXh0OwogCXVuc2lnbmVkIGNoYXIg KmRhdGEgPSB1cmItPnRyYW5zZmVyX2J1ZmZlcjsKIAl1bnNpZ25lZCBsb25nIGZsYWdzOworCWJv b2wgc3RvcHBlZCA9IGZhbHNlOwogCWludCBzdGF0dXMgPSB1cmItPnN0YXR1czsKIAlpbnQgaTsK IApAQCAtMzgzLDMzICszODQsNTEgQEAgdm9pZCB1c2Jfc2VyaWFsX2dlbmVyaWNfcmVhZF9idWxr X2NhbGxiYWNrKHN0cnVjdCB1cmIgKnVyYikKIAkJaWYgKHVyYiA9PSBwb3J0LT5yZWFkX3VyYnNb aV0pCiAJCQlicmVhazsKIAl9Ci0Jc2V0X2JpdChpLCAmcG9ydC0+cmVhZF91cmJzX2ZyZWUpOwog CiAJZGV2X2RiZygmcG9ydC0+ZGV2LCAiJXMgLSB1cmIgJWQsIGxlbiAlZFxuIiwgX19mdW5jX18s IGksCiAJCQkJCQkJdXJiLT5hY3R1YWxfbGVuZ3RoKTsKIAlzd2l0Y2ggKHN0YXR1cykgewogCWNh c2UgMDoKKwkJdXNiX3NlcmlhbF9kZWJ1Z19kYXRhKCZwb3J0LT5kZXYsIF9fZnVuY19fLCB1cmIt PmFjdHVhbF9sZW5ndGgsCisJCQkJCQkJZGF0YSk7CisJCXBvcnQtPnNlcmlhbC0+dHlwZS0+cHJv Y2Vzc19yZWFkX3VyYih1cmIpOwogCQlicmVhazsKIAljYXNlIC1FTk9FTlQ6CiAJY2FzZSAtRUNP Tk5SRVNFVDoKIAljYXNlIC1FU0hVVERPV046CiAJCWRldl9kYmcoJnBvcnQtPmRldiwgIiVzIC0g dXJiIHN0b3BwZWQ6ICVkXG4iLAogCQkJCQkJCV9fZnVuY19fLCBzdGF0dXMpOwotCQlyZXR1cm47 CisJCXN0b3BwZWQgPSB0cnVlOworCQlicmVhazsKIAljYXNlIC1FUElQRToKIAkJZGV2X2Vycigm cG9ydC0+ZGV2LCAiJXMgLSB1cmIgc3RvcHBlZDogJWRcbiIsCiAJCQkJCQkJX19mdW5jX18sIHN0 YXR1cyk7Ci0JCXJldHVybjsKKwkJc3RvcHBlZCA9IHRydWU7CisJCWJyZWFrOwogCWRlZmF1bHQ6 CiAJCWRldl9kYmcoJnBvcnQtPmRldiwgIiVzIC0gbm9uemVybyB1cmIgc3RhdHVzOiAlZFxuIiwK IAkJCQkJCQlfX2Z1bmNfXywgc3RhdHVzKTsKLQkJZ290byByZXN1Ym1pdDsKKwkJYnJlYWs7CiAJ fQogCi0JdXNiX3NlcmlhbF9kZWJ1Z19kYXRhKCZwb3J0LT5kZXYsIF9fZnVuY19fLCB1cmItPmFj dHVhbF9sZW5ndGgsIGRhdGEpOwotCXBvcnQtPnNlcmlhbC0+dHlwZS0+cHJvY2Vzc19yZWFkX3Vy Yih1cmIpOworCS8qCisJICogTWFrZSBzdXJlIFVSQiBwcm9jZXNzaW5nIGlzIGRvbmUgYmVmb3Jl IG1hcmtpbmcgYXMgZnJlZSB0byBhdm9pZAorCSAqIHJhY2luZyB3aXRoIHVudGhyb3R0bGUoKSBv biBhbm90aGVyIENQVS4gTWF0Y2hlcyB0aGUgYmFycmllcnMKKwkgKiBpbXBsaWVkIGJ5IHRoZSB0 ZXN0X2FuZF9jbGVhcl9iaXQoKSBpbgorCSAqIHVzYl9zZXJpYWxfZ2VuZXJpY19zdWJtaXRfcmVh ZF91cmIoKS4KKwkgKi8KKwlzbXBfbWJfX2JlZm9yZV9hdG9taWMoKTsKKwlzZXRfYml0KGksICZw b3J0LT5yZWFkX3VyYnNfZnJlZSk7CisJLyoKKwkgKiBNYWtlIHN1cmUgVVJCIGlzIG1hcmtlZCBh cyBmcmVlIGJlZm9yZSBjaGVja2luZyB0aGUgdGhyb3R0bGVkIGZsYWcKKwkgKiB0byBhdm9pZCBy YWNpbmcgd2l0aCB1bnRocm90dGxlKCkgb24gYW5vdGhlciBDUFUuIE1hdGNoZXMgdGhlCisJICog c21wX21iKCkgaW4gdW50aHJvdHRsZSgpLgorCSAqLworCXNtcF9tYl9fYWZ0ZXJfYXRvbWljKCk7 CisKKwlpZiAoc3RvcHBlZCkKKwkJcmV0dXJuOwogCi1yZXN1Ym1pdDoKIAkvKiBUaHJvdHRsZSB0 aGUgZGV2aWNlIGlmIHJlcXVlc3RlZCBieSB0dHkgKi8KIAlzcGluX2xvY2tfaXJxc2F2ZSgmcG9y dC0+bG9jaywgZmxhZ3MpOwogCXBvcnQtPnRocm90dGxlZCA9IHBvcnQtPnRocm90dGxlX3JlcTsK QEAgLTQ4NCw2ICs1MDMsMTIgQEAgdm9pZCB1c2Jfc2VyaWFsX2dlbmVyaWNfdW50aHJvdHRsZShz dHJ1Y3QgdHR5X3N0cnVjdCAqdHR5KQogCXBvcnQtPnRocm90dGxlZCA9IHBvcnQtPnRocm90dGxl X3JlcSA9IDA7CiAJc3Bpbl91bmxvY2tfaXJxKCZwb3J0LT5sb2NrKTsKIAorCS8qCisJICogTWF0 Y2hlcyB0aGUgc21wX21iX19hZnRlcl9hdG9taWMoKSBpbgorCSAqIHVzYl9zZXJpYWxfZ2VuZXJp Y19yZWFkX2J1bGtfY2FsbGJhY2soKS4KKwkgKi8KKwlzbXBfbWIoKTsKKwogCWlmICh3YXNfdGhy b3R0bGVkKQogCQl1c2Jfc2VyaWFsX2dlbmVyaWNfc3VibWl0X3JlYWRfdXJicyhwb3J0LCBHRlBf S0VSTkVMKTsKIH0K 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=-9.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS, T_DKIMWL_WL_HIGH,USER_AGENT_GIT 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 70905C43219 for ; Thu, 25 Apr 2019 16:06:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B95BE2088F for ; Thu, 25 Apr 2019 16:06:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556208360; bh=3JgZsCuvaheDUdPCK8Du9Mbq3Ry3153ruW4FcGJ6cPc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=vVphttptNfmJ6WwQT+mbJt9w/RsfmoB9N1iRnuNNKhSA6OAcEzKVjIsqSG2YwFV7z 3fS3dIk6kLzGebtUfxQjImskQiEregiH/csTHA4aTEIbPiwQ07DDB0cdPbv4/5oVXI kdU11XZriX0CSMJJ9Iy+mG2glgmGdTP39HJU7OyQ= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726896AbfDYQF7 (ORCPT ); Thu, 25 Apr 2019 12:05:59 -0400 Received: from mail-lf1-f65.google.com ([209.85.167.65]:35886 "EHLO mail-lf1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726402AbfDYQF7 (ORCPT ); Thu, 25 Apr 2019 12:05:59 -0400 Received: by mail-lf1-f65.google.com with SMTP id u17so197192lfi.3 for ; Thu, 25 Apr 2019 09:05:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=jH+B5GnM61cAUeL6Upm3Ak3lJ7oLat4CM9piCjk6n50=; b=iNBSYhHPamRcbCgXc+KAxdYMMPWKF1ByH5EEw0ZM0L6hCZOcdR9+CD1/tFW3g/qisR BLXi9jAQvUHjuv2BRC9JybrBywMHlQ8YNkNcFPiY4rNtsmZ0eJcwTqzvQGvyiDb31e/M lD7TGB1Snmf+QOqvjNM5nod6TEfjS4qyL53NSY0KGvk2XU/CALnEa582OxLdRseSdigO vzQUkn75Y4GKXgBLrpwbOHYIrIMsK7qqfqseVZRZMWtEX2lgZ80aXJm90Dz1AKY69alo 2YBHNWBfRYQR1r/MNditkuiMmIdbJKMkmEsRl8pw756JPBq2Tuci2C+e9MFv+czpFoB5 E40A== X-Gm-Message-State: APjAAAWj8umivpHFGywsDAVcAlA8Zic39rLbQaGGLwd0gFnkoZcRDQu3 BSa2wQDm5ktRXYL2qKRYZLs= X-Google-Smtp-Source: APXvYqw/c1hJwjfjGJRhwG5s42NHZJMYY0m5YpteZ+is7Zw0Cl0C8wpUh1C7WUVy82Htgxs1r/y6Cw== X-Received: by 2002:ac2:4301:: with SMTP id l1mr5318832lfh.54.1556208357086; Thu, 25 Apr 2019 09:05:57 -0700 (PDT) Received: from xi.terra (c-74bee655.07-184-6d6c6d4.bbcust.telenor.se. [85.230.190.116]) by smtp.gmail.com with ESMTPSA id w22sm4636730ljd.42.2019.04.25.09.05.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 25 Apr 2019 09:05:55 -0700 (PDT) Received: from johan by xi.terra with local (Exim 4.91) (envelope-from ) id 1hJgsl-0002ci-Iq; Thu, 25 Apr 2019 18:05:55 +0200 From: Johan Hovold To: Alan Stern , Oliver Neukum , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, Johan Hovold Subject: [PATCH 1/5] USB: serial: fix unthrottle races Date: Thu, 25 Apr 2019 18:05:36 +0200 Message-Id: <20190425160540.10036-2-johan@kernel.org> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190425160540.10036-1-johan@kernel.org> References: <20190425160540.10036-1-johan@kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Message-ID: <20190425160536.lr2suO_m7yhzBE-DZrZyO_l07D-Qn-g-d-rPWJF45mk@z> 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 --- drivers/usb/serial/generic.c | 39 +++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c index 2274d9625f63..0fff4968ea1b 100644 --- a/drivers/usb/serial/generic.c +++ b/drivers/usb/serial/generic.c @@ -376,6 +376,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb) struct usb_serial_port *port = urb->context; unsigned char *data = urb->transfer_buffer; unsigned long flags; + bool stopped = false; int status = urb->status; int i; @@ -383,33 +384,51 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb) if (urb == port->read_urbs[i]) break; } - set_bit(i, &port->read_urbs_free); dev_dbg(&port->dev, "%s - urb %d, len %d\n", __func__, i, urb->actual_length); switch (status) { case 0: + usb_serial_debug_data(&port->dev, __func__, urb->actual_length, + data); + port->serial->type->process_read_urb(urb); break; case -ENOENT: case -ECONNRESET: case -ESHUTDOWN: dev_dbg(&port->dev, "%s - urb stopped: %d\n", __func__, status); - return; + stopped = true; + break; case -EPIPE: dev_err(&port->dev, "%s - urb stopped: %d\n", __func__, status); - return; + stopped = true; + break; default: dev_dbg(&port->dev, "%s - nonzero urb status: %d\n", __func__, status); - goto resubmit; + break; } - usb_serial_debug_data(&port->dev, __func__, urb->actual_length, data); - port->serial->type->process_read_urb(urb); + /* + * Make sure URB processing is done before marking as free to avoid + * racing with unthrottle() on another CPU. Matches the barriers + * implied by the test_and_clear_bit() in + * usb_serial_generic_submit_read_urb(). + */ + smp_mb__before_atomic(); + set_bit(i, &port->read_urbs_free); + /* + * Make sure URB is marked as free before checking the throttled flag + * to avoid racing with unthrottle() on another CPU. Matches the + * smp_mb() in unthrottle(). + */ + smp_mb__after_atomic(); + + if (stopped) + return; -resubmit: /* Throttle the device if requested by tty */ spin_lock_irqsave(&port->lock, flags); port->throttled = port->throttle_req; @@ -484,6 +503,12 @@ void usb_serial_generic_unthrottle(struct tty_struct *tty) port->throttled = port->throttle_req = 0; spin_unlock_irq(&port->lock); + /* + * Matches the smp_mb__after_atomic() in + * usb_serial_generic_read_bulk_callback(). + */ + smp_mb(); + if (was_throttled) usb_serial_generic_submit_read_urbs(port, GFP_KERNEL); } -- 2.21.0