From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AED641FCD11; Fri, 6 Dec 2024 15:11:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733497907; cv=none; b=X/6Vx7sPPJ35h40CH3N0ZgIvz1aRvPOHeyHNpAhNvJMdXM/11tNNpWQ0B1+QYqf1pHXw8DvoTyS5jICdPjs7fMdi3/Gzdr5yEmbgQMeMX/wD4rgblkfZ/dBLRO8wj2SBfRy6T1cZHCiL6QZzwqlbY9VBnhZQtnhc58A2ro5xWiw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733497907; c=relaxed/simple; bh=MsuQdS/mqNUYT2FqgSLMrNI9xE8DQnRKN6nLYOoNvbs=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=XpF/8EC3vzwyi9LVnitP/UbRJuXf/QujF5CXPOq++hBJhAPZodhLXsU4yo6K7kauw5YPrn9toT0roCXUAVIzMXB78WNjM0h1MLub1sRSA99PNlRkDhgT/UmD5adTf/6nZUlEiKRLgxoSUIIiHHMKkv7kzczHxiltJEXBgAZxXXE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=Yl9Yolhz; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="Yl9Yolhz" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 13112C4CED1; Fri, 6 Dec 2024 15:11:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1733497907; bh=MsuQdS/mqNUYT2FqgSLMrNI9xE8DQnRKN6nLYOoNvbs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Yl9YolhzG7sNzWjoJ4Lz8XexopCARxLpIovcx4VfWjyiAejIZ6NT1TI58YoehaPks NW6Ddt+G4k0Qxtmivj1uSHmWAmPRc2uYxjsvSWUTMBvkZJr45k8OJ37go++092Fc6U j3GB3M1+7IhH+Q1BXZqmBLlDMC8vAxeGVube+89o= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Oliver Neukum , syzbot+422188bce66e76020e55@syzkaller.appspotmail.com, Sasha Levin Subject: [PATCH 6.6 429/676] USB: chaoskey: fail open after removal Date: Fri, 6 Dec 2024 15:34:08 +0100 Message-ID: <20241206143710.100935505@linuxfoundation.org> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20241206143653.344873888@linuxfoundation.org> References: <20241206143653.344873888@linuxfoundation.org> User-Agent: quilt/0.67 X-stable: review X-Patchwork-Hint: ignore Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 6.6-stable review patch. If anyone has any objections, please let me know. ------------------ From: Oliver Neukum [ Upstream commit 422dc0a4d12d0b80dd3aab3fe5943f665ba8f041 ] chaoskey_open() takes the lock only to increase the counter of openings. That means that the mutual exclusion with chaoskey_disconnect() cannot prevent an increase of the counter and chaoskey_open() returning a success. If that race is hit, chaoskey_disconnect() will happily free all resources associated with the device after it has dropped the lock, as it has read the counter as zero. To prevent this race chaoskey_open() has to check the presence of the device under the lock. However, the current per device lock cannot be used, because it is a part of the data structure to be freed. Hence an additional global mutex is needed. The issue is as old as the driver. Signed-off-by: Oliver Neukum Reported-by: syzbot+422188bce66e76020e55@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=422188bce66e76020e55 Fixes: 66e3e591891da ("usb: Add driver for Altus Metrum ChaosKey device (v2)") Rule: add Link: https://lore.kernel.org/stable/20241002132201.552578-1-oneukum%40suse.com Link: https://lore.kernel.org/r/20241002132201.552578-1-oneukum@suse.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin --- drivers/usb/misc/chaoskey.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c index 6fb5140e29b9d..e8b63df5f9759 100644 --- a/drivers/usb/misc/chaoskey.c +++ b/drivers/usb/misc/chaoskey.c @@ -27,6 +27,8 @@ static struct usb_class_driver chaoskey_class; static int chaoskey_rng_read(struct hwrng *rng, void *data, size_t max, bool wait); +static DEFINE_MUTEX(chaoskey_list_lock); + #define usb_dbg(usb_if, format, arg...) \ dev_dbg(&(usb_if)->dev, format, ## arg) @@ -230,6 +232,7 @@ static void chaoskey_disconnect(struct usb_interface *interface) if (dev->hwrng_registered) hwrng_unregister(&dev->hwrng); + mutex_lock(&chaoskey_list_lock); usb_deregister_dev(interface, &chaoskey_class); usb_set_intfdata(interface, NULL); @@ -244,6 +247,7 @@ static void chaoskey_disconnect(struct usb_interface *interface) } else mutex_unlock(&dev->lock); + mutex_unlock(&chaoskey_list_lock); usb_dbg(interface, "disconnect done"); } @@ -251,6 +255,7 @@ static int chaoskey_open(struct inode *inode, struct file *file) { struct chaoskey *dev; struct usb_interface *interface; + int rv = 0; /* get the interface from minor number and driver information */ interface = usb_find_interface(&chaoskey_driver, iminor(inode)); @@ -266,18 +271,23 @@ static int chaoskey_open(struct inode *inode, struct file *file) } file->private_data = dev; + mutex_lock(&chaoskey_list_lock); mutex_lock(&dev->lock); - ++dev->open; + if (dev->present) + ++dev->open; + else + rv = -ENODEV; mutex_unlock(&dev->lock); + mutex_unlock(&chaoskey_list_lock); - usb_dbg(interface, "open success"); - return 0; + return rv; } static int chaoskey_release(struct inode *inode, struct file *file) { struct chaoskey *dev = file->private_data; struct usb_interface *interface; + int rv = 0; if (dev == NULL) return -ENODEV; @@ -286,14 +296,15 @@ static int chaoskey_release(struct inode *inode, struct file *file) usb_dbg(interface, "release"); + mutex_lock(&chaoskey_list_lock); mutex_lock(&dev->lock); usb_dbg(interface, "open count at release is %d", dev->open); if (dev->open <= 0) { usb_dbg(interface, "invalid open count (%d)", dev->open); - mutex_unlock(&dev->lock); - return -ENODEV; + rv = -ENODEV; + goto bail; } --dev->open; @@ -302,13 +313,15 @@ static int chaoskey_release(struct inode *inode, struct file *file) if (dev->open == 0) { mutex_unlock(&dev->lock); chaoskey_free(dev); - } else - mutex_unlock(&dev->lock); - } else - mutex_unlock(&dev->lock); - + goto destruction; + } + } +bail: + mutex_unlock(&dev->lock); +destruction: + mutex_lock(&chaoskey_list_lock); usb_dbg(interface, "release success"); - return 0; + return rv; } static void chaos_read_callback(struct urb *urb) -- 2.43.0