From patchwork Fri Oct 18 15:19:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johan Hovold X-Patchwork-Id: 11198673 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3E71213BD for ; Fri, 18 Oct 2019 15:20:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1EF40222C9 for ; Fri, 18 Oct 2019 15:20:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1571412007; bh=kuEJApMoHLL/9elbWnqOsRnyHcTfOGQdxJ1YleOYdOA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=qVgB56PJqRPrhBpjNvJ2DaLhvkRHQ1J+XSCNh83Vmt0vfM3JMbBhPx9pMBzlWV2oI dFIS1z9lqxm+qtp/kqaDBJTJUfhaDvp6zvIWnenJQM2LkRdl7fX5l/VKgJELqQfj98 SYz+Vv01XwwPKjitAgS66TqKNf3NLOOtqdsygoMg= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2439565AbfJRPUD (ORCPT ); Fri, 18 Oct 2019 11:20:03 -0400 Received: from mail-lj1-f194.google.com ([209.85.208.194]:45297 "EHLO mail-lj1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390388AbfJRPUC (ORCPT ); Fri, 18 Oct 2019 11:20:02 -0400 Received: by mail-lj1-f194.google.com with SMTP id q64so6588494ljb.12; Fri, 18 Oct 2019 08:20:01 -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=NWEsXs2dpg7Z5tah6xitYaTbPYgefyb7xkTWhkm2Qdg=; b=RIJXSBl/VZkTLWruORCmOEYauiHEoxNiBYt2tKuZuWo4lBqT4RKwWH8clwAetLlSjg y9m2NwyQr9jJdsGN9PLH7CwQN/xbs2xdV40o3grwQQTKvyT4YapYvkIbt6r+px8+HjVC mCHpmTVjT8rLYOqXQmG7cJk+mymp8KYcvcYySfrpAWcWb9w1VbyLh1e8ddWwOntG+2Tl Oqmj3lpgUtKmGrCLHqoOcSowC7iRJjnHQTFPTuQIVjhB7JWAPCQxPenZ/CHXq0AA8oVb BLKneJZAzybZqdvT3kFH6DPvlDDEh36fBk0urlkFYd0wbLAG4iY8lyRwQWAfi7FcgVrb C68g== X-Gm-Message-State: APjAAAVvf9/TaGcCg27mD7loPY48RAtn7hu2vcpREroK3sl+Bt4CQjdx ywArCeBgt9Zw7AYvio//lTMmU/xW X-Google-Smtp-Source: APXvYqzXBMIbd9C3xS8GBqkUeuj4fmTPf93b5jl3oFBp4ALul+QVERZ5wgPQOftfyeX8gkzQrbE26Q== X-Received: by 2002:a2e:8e87:: with SMTP id z7mr6588549ljk.9.1571412000231; Fri, 18 Oct 2019 08:20:00 -0700 (PDT) Received: from xi.terra (c-51f1e055.07-184-6d6c6d4.bbcust.telenor.se. [85.224.241.81]) by smtp.gmail.com with ESMTPSA id u21sm5340202lje.92.2019.10.18.08.19.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Oct 2019 08:19:58 -0700 (PDT) Received: from johan by xi.terra with local (Exim 4.92.2) (envelope-from ) id 1iLU31-0006YH-3u; Fri, 18 Oct 2019 17:20:11 +0200 From: Johan Hovold To: Greg Kroah-Hartman Cc: Alan Stern , Oliver Neukum , "Paul E . McKenney" , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Johan Hovold , stable , syzbot+6fe95b826644f7f12b0b@syzkaller.appspotmail.com Subject: [PATCH v2 1/2] USB: ldusb: fix read info leaks Date: Fri, 18 Oct 2019 17:19:54 +0200 Message-Id: <20191018151955.25135-2-johan@kernel.org> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191018151955.25135-1-johan@kernel.org> References: <20191018151955.25135-1-johan@kernel.org> MIME-Version: 1.0 Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org Fix broken read implementation, which could be used to trigger slab info leaks. The driver failed to check if the custom ring buffer was still empty when waking up after having waited for more data. This would happen on every interrupt-in completion, even if no data had been added to the ring buffer (e.g. on disconnect events). Due to missing sanity checks and uninitialised (kmalloced) ring-buffer entries, this meant that huge slab info leaks could easily be triggered. Note that the empty-buffer check after wakeup is enough to fix the info leak on disconnect, but let's clear the buffer on allocation and add a sanity check to read() to prevent further leaks. Fixes: 2824bd250f0b ("[PATCH] USB: add ldusb driver") Cc: stable # 2.6.13 Reported-by: syzbot+6fe95b826644f7f12b0b@syzkaller.appspotmail.com Signed-off-by: Johan Hovold --- drivers/usb/misc/ldusb.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/usb/misc/ldusb.c b/drivers/usb/misc/ldusb.c index 147c90c2a4e5..15b5f06fb0b3 100644 --- a/drivers/usb/misc/ldusb.c +++ b/drivers/usb/misc/ldusb.c @@ -464,7 +464,7 @@ static ssize_t ld_usb_read(struct file *file, char __user *buffer, size_t count, /* wait for data */ spin_lock_irq(&dev->rbsl); - if (dev->ring_head == dev->ring_tail) { + while (dev->ring_head == dev->ring_tail) { dev->interrupt_in_done = 0; spin_unlock_irq(&dev->rbsl); if (file->f_flags & O_NONBLOCK) { @@ -474,12 +474,17 @@ static ssize_t ld_usb_read(struct file *file, char __user *buffer, size_t count, retval = wait_event_interruptible(dev->read_wait, dev->interrupt_in_done); if (retval < 0) goto unlock_exit; - } else { - spin_unlock_irq(&dev->rbsl); + + spin_lock_irq(&dev->rbsl); } + spin_unlock_irq(&dev->rbsl); /* actual_buffer contains actual_length + interrupt_in_buffer */ actual_buffer = (size_t *)(dev->ring_buffer + dev->ring_tail * (sizeof(size_t)+dev->interrupt_in_endpoint_size)); + if (*actual_buffer > dev->interrupt_in_endpoint_size) { + retval = -EIO; + goto unlock_exit; + } bytes_to_read = min(count, *actual_buffer); if (bytes_to_read < *actual_buffer) dev_warn(&dev->intf->dev, "Read buffer overflow, %zd bytes dropped\n", @@ -690,10 +695,9 @@ static int ld_usb_probe(struct usb_interface *intf, const struct usb_device_id * dev_warn(&intf->dev, "Interrupt out endpoint not found (using control endpoint instead)\n"); dev->interrupt_in_endpoint_size = usb_endpoint_maxp(dev->interrupt_in_endpoint); - dev->ring_buffer = - kmalloc_array(ring_buffer_size, - sizeof(size_t) + dev->interrupt_in_endpoint_size, - GFP_KERNEL); + dev->ring_buffer = kcalloc(ring_buffer_size, + sizeof(size_t) + dev->interrupt_in_endpoint_size, + GFP_KERNEL); if (!dev->ring_buffer) goto error; dev->interrupt_in_buffer = kmalloc(dev->interrupt_in_endpoint_size, GFP_KERNEL); From patchwork Fri Oct 18 15:19:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johan Hovold X-Patchwork-Id: 11198675 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 82AF2112C for ; Fri, 18 Oct 2019 15:20:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 62324222C6 for ; Fri, 18 Oct 2019 15:20:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1571412011; bh=xU5Dx7eteAD2BW458bVRl1RoNIe8FslE9TVPgGdDUdE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=qrscgRRAlhgol1J3lUMy5HsReX59wqzc4Zjqo3FKRw+BqawD7HOwOp5Z88OEAijbG Ek/rfM1Ms1PWTZrIJ0fkAOdg4B8SFbybHjYztmcXLasuultqq5qCiPSwCeF7+5qalg 4fTQA4WsDUJrPkV6kFUz+2Cor6ro48nMD73+FomA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2504733AbfJRPUG (ORCPT ); Fri, 18 Oct 2019 11:20:06 -0400 Received: from mail-lf1-f68.google.com ([209.85.167.68]:35527 "EHLO mail-lf1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2501908AbfJRPUF (ORCPT ); Fri, 18 Oct 2019 11:20:05 -0400 Received: by mail-lf1-f68.google.com with SMTP id w6so5030121lfl.2; Fri, 18 Oct 2019 08:20:02 -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=+AnJxrgOsF5pdHMRsdpMSMexStt/hgIWSycUyGA4QAg=; b=JRFvI3s7v+dr38puTvmC9iBqpU9pPsECM026W3PKld7UNAXFPbFz5r9bdS4GFfP/eG Dy+aWu+AXVRp9HoMfO+UZ3LYoxW/bFdgZrMu898bwULHsJ5fHZtFIOZofdxqRGNbvJ8T NwlybpP7mUpSJgV6QM7b/ioPiyKDi6bea1tYcdVObMF2PFaeUuGUe0QMOykiaUqNGBlp 6M5KOL0N6nP7xQ861JL+sGHvmLghc852AP56TlmdZoK1kWrDihqTQBl+DJMrIGlOHLwh Wdl0LVBv6yXnlcUKqrwk9ceSOaACbTIhidQdyiRJj050D8e8utsKCK6flI56tInSvgkx 121A== X-Gm-Message-State: APjAAAUn7mKJtsJaI4tKF3zZBDXDYxuRLYzJmSE4RxYFUaNWBYsa0lE6 h+VqRAFUoIWIOZ1oYhLe14U= X-Google-Smtp-Source: APXvYqwZnHP5fM5ugg6DkmTBhXwuX0nTdcQH4m5Fbj3XQEpFQnlDh8mVi/DdcZNeL+QLbwYoMnxK9w== X-Received: by 2002:a19:c6d6:: with SMTP id w205mr5561511lff.17.1571412001935; Fri, 18 Oct 2019 08:20:01 -0700 (PDT) Received: from xi.terra (c-51f1e055.07-184-6d6c6d4.bbcust.telenor.se. [85.224.241.81]) by smtp.gmail.com with ESMTPSA id x5sm3197980lfg.71.2019.10.18.08.19.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Oct 2019 08:19:58 -0700 (PDT) Received: from johan by xi.terra with local (Exim 4.92.2) (envelope-from ) id 1iLU31-0006YM-6x; Fri, 18 Oct 2019 17:20:11 +0200 From: Johan Hovold To: Greg Kroah-Hartman Cc: Alan Stern , Oliver Neukum , "Paul E . McKenney" , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Johan Hovold , stable Subject: [PATCH RFC v2 2/2] USB: ldusb: fix ring-buffer locking Date: Fri, 18 Oct 2019 17:19:55 +0200 Message-Id: <20191018151955.25135-3-johan@kernel.org> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191018151955.25135-1-johan@kernel.org> References: <20191018151955.25135-1-johan@kernel.org> MIME-Version: 1.0 Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org The custom ring-buffer implementation was merged without any locking whatsoever, but a spinlock was later added by commit 9d33efd9a791 ("USB: ldusb bugfix"). The lock did not cover the loads from the ring-buffer entry after determining the buffer was non-empty, nor the update of the tail index once the entry had been processed. The former could lead to stale data being returned, while the latter could lead to memory corruption on sufficiently weakly ordered architectures. Fixes: 2824bd250f0b ("[PATCH] USB: add ldusb driver") Fixes: 9d33efd9a791 ("USB: ldusb bugfix") Cc: stable # 2.6.13 Signed-off-by: Johan Hovold --- drivers/usb/misc/ldusb.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/usb/misc/ldusb.c b/drivers/usb/misc/ldusb.c index 15b5f06fb0b3..6b5843b0071e 100644 --- a/drivers/usb/misc/ldusb.c +++ b/drivers/usb/misc/ldusb.c @@ -477,11 +477,11 @@ static ssize_t ld_usb_read(struct file *file, char __user *buffer, size_t count, spin_lock_irq(&dev->rbsl); } - spin_unlock_irq(&dev->rbsl); /* actual_buffer contains actual_length + interrupt_in_buffer */ actual_buffer = (size_t *)(dev->ring_buffer + dev->ring_tail * (sizeof(size_t)+dev->interrupt_in_endpoint_size)); if (*actual_buffer > dev->interrupt_in_endpoint_size) { + spin_unlock_irq(&dev->rbsl); retval = -EIO; goto unlock_exit; } @@ -489,17 +489,26 @@ static ssize_t ld_usb_read(struct file *file, char __user *buffer, size_t count, if (bytes_to_read < *actual_buffer) dev_warn(&dev->intf->dev, "Read buffer overflow, %zd bytes dropped\n", *actual_buffer-bytes_to_read); + spin_unlock_irq(&dev->rbsl); + + /* + * Pairs with spin_unlock_irqrestore() in + * ld_usb_interrupt_in_callback() and makes sure the ring-buffer entry + * has been updated before copy_to_user(). + */ + smp_rmb(); /* copy one interrupt_in_buffer from ring_buffer into userspace */ if (copy_to_user(buffer, actual_buffer+1, bytes_to_read)) { retval = -EFAULT; goto unlock_exit; } - dev->ring_tail = (dev->ring_tail+1) % ring_buffer_size; - retval = bytes_to_read; spin_lock_irq(&dev->rbsl); + + dev->ring_tail = (dev->ring_tail + 1) % ring_buffer_size; + if (dev->buffer_overflow) { dev->buffer_overflow = 0; spin_unlock_irq(&dev->rbsl);