From patchwork Tue Apr 30 14:23:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Neukum X-Patchwork-Id: 10923701 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D826814C0 for ; Tue, 30 Apr 2019 14:23:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C676A26E51 for ; Tue, 30 Apr 2019 14:23:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B6F36287E5; Tue, 30 Apr 2019 14:23:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4FD6426E51 for ; Tue, 30 Apr 2019 14:23:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726698AbfD3OXt (ORCPT ); Tue, 30 Apr 2019 10:23:49 -0400 Received: from mx2.suse.de ([195.135.220.15]:49340 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726362AbfD3OXt (ORCPT ); Tue, 30 Apr 2019 10:23:49 -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 64199AD6B; Tue, 30 Apr 2019 14:23:48 +0000 (UTC) From: Oliver Neukum To: gregKH@linuxfoundation.org, miquel@df.uba.ar, linux-usb@vger.kernel.org Cc: Oliver Neukum Subject: [PATCH 1/3] rio500: refuse more than one device at a time Date: Tue, 30 Apr 2019 16:23:24 +0200 Message-Id: <20190430142326.25815-1-oneukum@suse.com> X-Mailer: git-send-email 2.16.4 Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This driver is using a global variable. It cannot handle more than one device at a time. The issue has been exisying since the dawn of the driver. Signed-off-by: Oliver Neukum Reported-by: syzbot+35f04d136fc975a70da4@syzkaller.appspotmail.com --- drivers/usb/misc/rio500.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c index 13e4889bc34f..a4b6fbea975f 100644 --- a/drivers/usb/misc/rio500.c +++ b/drivers/usb/misc/rio500.c @@ -449,7 +449,12 @@ static int probe_rio(struct usb_interface *intf, struct rio_usb_data *rio = &rio_instance; int retval; - dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum); + if (rio->present) { + dev_info(&intf->dev, "Second USB Rio at address %d refused\n", dev->devnum); + return -EBUSY; + } else { + dev_info(&intf->dev, "USB Rio found at address %d\n", dev->devnum); + } retval = usb_register_dev(intf, &usb_rio_class); if (retval) { From patchwork Tue Apr 30 14:23:25 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Neukum X-Patchwork-Id: 10923703 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8611E92A for ; Tue, 30 Apr 2019 14:23:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 73BA726E51 for ; Tue, 30 Apr 2019 14:23:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 65AAE287E5; Tue, 30 Apr 2019 14:23:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1C01426E51 for ; Tue, 30 Apr 2019 14:23:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726915AbfD3OXu (ORCPT ); Tue, 30 Apr 2019 10:23:50 -0400 Received: from mx2.suse.de ([195.135.220.15]:49350 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726538AbfD3OXt (ORCPT ); Tue, 30 Apr 2019 10:23:49 -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 99B88AD81; Tue, 30 Apr 2019 14:23:48 +0000 (UTC) From: Oliver Neukum To: gregKH@linuxfoundation.org, miquel@df.uba.ar, linux-usb@vger.kernel.org Cc: Oliver Neukum Subject: [PATCH 2/3] rio500: fix memeory leak in close after disconnect Date: Tue, 30 Apr 2019 16:23:25 +0200 Message-Id: <20190430142326.25815-2-oneukum@suse.com> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20190430142326.25815-1-oneukum@suse.com> References: <20190430142326.25815-1-oneukum@suse.com> Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP If a disconnected device is closed, rio_close() must free the buffers. Signed-off-by: Oliver Neukum --- drivers/usb/misc/rio500.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c index a4b6fbea975f..20c3eb0af7ad 100644 --- a/drivers/usb/misc/rio500.c +++ b/drivers/usb/misc/rio500.c @@ -86,9 +86,22 @@ static int close_rio(struct inode *inode, struct file *file) { struct rio_usb_data *rio = &rio_instance; - rio->isopen = 0; + /* against disconnect() */ + mutex_lock(&rio500_mutex); + mutex_lock(&(rio->lock)); - dev_info(&rio->rio_dev->dev, "Rio closed.\n"); + rio->isopen = 0; + if (!rio->present) { + /* cleanup has been delayed */ + kfree(rio->ibuf); + kfree(rio->obuf); + rio->ibuf = NULL; + rio->obuf = NULL; + } else { + dev_info(&rio->rio_dev->dev, "Rio closed.\n"); + } + mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return 0; } From patchwork Tue Apr 30 14:23:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Neukum X-Patchwork-Id: 10923705 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 492C092A for ; Tue, 30 Apr 2019 14:23:53 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 383AA26E51 for ; Tue, 30 Apr 2019 14:23:53 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2C112287E5; Tue, 30 Apr 2019 14:23:53 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9D89526E51 for ; Tue, 30 Apr 2019 14:23:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727138AbfD3OXv (ORCPT ); Tue, 30 Apr 2019 10:23:51 -0400 Received: from mx2.suse.de ([195.135.220.15]:49366 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726362AbfD3OXv (ORCPT ); Tue, 30 Apr 2019 10:23:51 -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 AAC4AAE14; Tue, 30 Apr 2019 14:23:49 +0000 (UTC) From: Oliver Neukum To: gregKH@linuxfoundation.org, miquel@df.uba.ar, linux-usb@vger.kernel.org Cc: Oliver Neukum Subject: [PATCH 3/3] rio500: simplify locking Date: Tue, 30 Apr 2019 16:23:26 +0200 Message-Id: <20190430142326.25815-3-oneukum@suse.com> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20190430142326.25815-1-oneukum@suse.com> References: <20190430142326.25815-1-oneukum@suse.com> Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Admitting that there can be only one device allows us to drop any pretense about locking one device or a table of devices. Signed-off-by: Oliver Neukum --- drivers/usb/misc/rio500.c | 43 ++++++++++++++++--------------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c index 20c3eb0af7ad..5110eaeda8b4 100644 --- a/drivers/usb/misc/rio500.c +++ b/drivers/usb/misc/rio500.c @@ -51,7 +51,6 @@ struct rio_usb_data { char *obuf, *ibuf; /* transfer buffers */ char bulk_in_ep, bulk_out_ep; /* Endpoint assignments */ wait_queue_head_t wait_q; /* for timeouts */ - struct mutex lock; /* general race avoidance */ }; static DEFINE_MUTEX(rio500_mutex); @@ -63,10 +62,8 @@ static int open_rio(struct inode *inode, struct file *file) /* against disconnect() */ mutex_lock(&rio500_mutex); - mutex_lock(&(rio->lock)); if (rio->isopen || !rio->present) { - mutex_unlock(&(rio->lock)); mutex_unlock(&rio500_mutex); return -EBUSY; } @@ -74,7 +71,6 @@ static int open_rio(struct inode *inode, struct file *file) init_waitqueue_head(&rio->wait_q); - mutex_unlock(&(rio->lock)); dev_info(&rio->rio_dev->dev, "Rio opened.\n"); mutex_unlock(&rio500_mutex); @@ -88,7 +84,6 @@ static int close_rio(struct inode *inode, struct file *file) /* against disconnect() */ mutex_lock(&rio500_mutex); - mutex_lock(&(rio->lock)); rio->isopen = 0; if (!rio->present) { @@ -100,7 +95,6 @@ static int close_rio(struct inode *inode, struct file *file) } else { dev_info(&rio->rio_dev->dev, "Rio closed.\n"); } - mutex_unlock(&(rio->lock)); mutex_unlock(&rio500_mutex); return 0; } @@ -115,7 +109,7 @@ static long ioctl_rio(struct file *file, unsigned int cmd, unsigned long arg) int retries; int retval=0; - mutex_lock(&(rio->lock)); + mutex_lock(&rio500_mutex); /* Sanity check to make sure rio is connected, powered, etc */ if (rio->present == 0 || rio->rio_dev == NULL) { retval = -ENODEV; @@ -259,7 +253,7 @@ static long ioctl_rio(struct file *file, unsigned int cmd, unsigned long arg) err_out: - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return retval; } @@ -279,12 +273,12 @@ write_rio(struct file *file, const char __user *buffer, int errn = 0; int intr; - intr = mutex_lock_interruptible(&(rio->lock)); + intr = mutex_lock_interruptible(&rio500_mutex); if (intr) return -EINTR; /* Sanity check to make sure rio is connected, powered, etc */ if (rio->present == 0 || rio->rio_dev == NULL) { - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return -ENODEV; } @@ -307,7 +301,7 @@ write_rio(struct file *file, const char __user *buffer, goto error; } if (signal_pending(current)) { - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return bytes_written ? bytes_written : -EINTR; } @@ -345,12 +339,12 @@ write_rio(struct file *file, const char __user *buffer, buffer += copy_size; } while (count > 0); - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return bytes_written ? bytes_written : -EIO; error: - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return errn; } @@ -367,12 +361,12 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos) char *ibuf; int intr; - intr = mutex_lock_interruptible(&(rio->lock)); + intr = mutex_lock_interruptible(&rio500_mutex); if (intr) return -EINTR; /* Sanity check to make sure rio is connected, powered, etc */ if (rio->present == 0 || rio->rio_dev == NULL) { - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return -ENODEV; } @@ -383,11 +377,11 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos) while (count > 0) { if (signal_pending(current)) { - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return read_count ? read_count : -EINTR; } if (!rio->rio_dev) { - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return -ENODEV; } this_read = (count >= IBUF_SIZE) ? IBUF_SIZE : count; @@ -405,7 +399,7 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos) count = this_read = partial; } else if (result == -ETIMEDOUT || result == 15) { /* FIXME: 15 ??? */ if (!maxretry--) { - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); dev_err(&rio->rio_dev->dev, "read_rio: maxretry timeout\n"); return -ETIME; @@ -415,19 +409,19 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos) finish_wait(&rio->wait_q, &wait); continue; } else if (result != -EREMOTEIO) { - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); dev_err(&rio->rio_dev->dev, "Read Whoops - result:%d partial:%u this_read:%u\n", result, partial, this_read); return -EIO; } else { - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return (0); } if (this_read) { if (copy_to_user(buffer, ibuf, this_read)) { - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return -EFAULT; } count -= this_read; @@ -435,7 +429,7 @@ read_rio(struct file *file, char __user *buffer, size_t count, loff_t * ppos) buffer += this_read; } } - mutex_unlock(&(rio->lock)); + mutex_unlock(&rio500_mutex); return read_count; } @@ -495,8 +489,6 @@ static int probe_rio(struct usb_interface *intf, } dev_dbg(&intf->dev, "ibuf address:%p\n", rio->ibuf); - mutex_init(&(rio->lock)); - usb_set_intfdata (intf, rio); rio->present = 1; @@ -512,12 +504,10 @@ static void disconnect_rio(struct usb_interface *intf) if (rio) { usb_deregister_dev(intf, &usb_rio_class); - mutex_lock(&(rio->lock)); if (rio->isopen) { rio->isopen = 0; /* better let it finish - the release will do whats needed */ rio->rio_dev = NULL; - mutex_unlock(&(rio->lock)); mutex_unlock(&rio500_mutex); return; } @@ -529,7 +519,6 @@ static void disconnect_rio(struct usb_interface *intf) dev_info(&intf->dev, "USB Rio disconnected.\n"); rio->present = 0; - mutex_unlock(&(rio->lock)); } mutex_unlock(&rio500_mutex); }