hamza.kesraoui/sst-linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

USB: chaoskey: fail open after removal

Browse Source 
[ 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 <oneukum@suse.com>
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 <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Oliver Neukum 2024-10-02 15:21:41 +02:00 committed by Greg Kroah-Hartman
parent cfb7f88ed3
commit 78e892874c
1 changed files with 24 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
drivers/usb/misc/chaoskey.c
View File

@ -27,6 +27,8 @@ static struct usb_class_driver chaoskey_class;
static int chaoskey_rng_read(struct hwrng *rng, void *data, static int chaoskey_rng_read(struct hwrng *rng, void *data,
size_t max, bool wait); size_t max, bool wait);
static DEFINE_MUTEX(chaoskey_list_lock);
#define usb_dbg(usb_if, format, arg...) \ #define usb_dbg(usb_if, format, arg...) \
dev_dbg(&(usb_if)->dev, format, ## arg) dev_dbg(&(usb_if)->dev, format, ## arg)
@ -231,6 +233,7 @@ static void chaoskey_disconnect(struct usb_interface *interface)
if (dev->hwrng_registered) if (dev->hwrng_registered)
hwrng_unregister(&dev->hwrng); hwrng_unregister(&dev->hwrng);
mutex_lock(&chaoskey_list_lock);
usb_deregister_dev(interface, &chaoskey_class); usb_deregister_dev(interface, &chaoskey_class);
usb_set_intfdata(interface, NULL); usb_set_intfdata(interface, NULL);
@ -245,6 +248,7 @@ static void chaoskey_disconnect(struct usb_interface *interface)
} else } else
mutex_unlock(&dev->lock); mutex_unlock(&dev->lock);
mutex_unlock(&chaoskey_list_lock);
usb_dbg(interface, "disconnect done"); usb_dbg(interface, "disconnect done");
} }
@ -252,6 +256,7 @@ static int chaoskey_open(struct inode *inode, struct file *file)
{ {
struct chaoskey *dev; struct chaoskey *dev;
struct usb_interface *interface; struct usb_interface *interface;
int rv = 0;
/* get the interface from minor number and driver information */ /* get the interface from minor number and driver information */
interface = usb_find_interface(&chaoskey_driver, iminor(inode)); interface = usb_find_interface(&chaoskey_driver, iminor(inode));
@ -267,18 +272,23 @@ static int chaoskey_open(struct inode *inode, struct file *file)
} }
file->private_data = dev; file->private_data = dev;
mutex_lock(&chaoskey_list_lock);
mutex_lock(&dev->lock); mutex_lock(&dev->lock);
if (dev->present)
++dev->open; ++dev->open;
else
rv = -ENODEV;
mutex_unlock(&dev->lock); mutex_unlock(&dev->lock);
mutex_unlock(&chaoskey_list_lock);
usb_dbg(interface, "open success"); return rv;
return 0;
} }
static int chaoskey_release(struct inode *inode, struct file *file) static int chaoskey_release(struct inode *inode, struct file *file)
{ {
struct chaoskey *dev = file->private_data; struct chaoskey *dev = file->private_data;
struct usb_interface *interface; struct usb_interface *interface;
int rv = 0;
if (dev == NULL) if (dev == NULL)
return -ENODEV; return -ENODEV;
@ -287,14 +297,15 @@ static int chaoskey_release(struct inode *inode, struct file *file)
usb_dbg(interface, "release"); usb_dbg(interface, "release");
mutex_lock(&chaoskey_list_lock);
mutex_lock(&dev->lock); mutex_lock(&dev->lock);
usb_dbg(interface, "open count at release is %d", dev->open); usb_dbg(interface, "open count at release is %d", dev->open);
if (dev->open <= 0) { if (dev->open <= 0) {
usb_dbg(interface, "invalid open count (%d)", dev->open); usb_dbg(interface, "invalid open count (%d)", dev->open);
mutex_unlock(&dev->lock); rv = -ENODEV;
return -ENODEV; goto bail;
} }
--dev->open; --dev->open;
@ -303,13 +314,15 @@ static int chaoskey_release(struct inode *inode, struct file *file)
if (dev->open == 0) { if (dev->open == 0) {
mutex_unlock(&dev->lock); mutex_unlock(&dev->lock);
chaoskey_free(dev); chaoskey_free(dev);
} else goto destruction;
}
}
bail:
mutex_unlock(&dev->lock); mutex_unlock(&dev->lock);
} else destruction:
mutex_unlock(&dev->lock); mutex_lock(&chaoskey_list_lock);
usb_dbg(interface, "release success"); usb_dbg(interface, "release success");
return 0; return rv;
} }
static void chaos_read_callback(struct urb *urb) static void chaos_read_callback(struct urb *urb)