Commit 7a6f22d7 authored by Johan Hovold's avatar Johan Hovold Committed by Greg Kroah-Hartman

USB: ldusb: fix read info leaks

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: 2824bd25 ("[PATCH] USB: add ldusb driver")
Cc: stable <stable@vger.kernel.org>     # 2.6.13
Reported-by: syzbot+6fe95b826644f7f12b0b@syzkaller.appspotmail.com
Signed-off-by: default avatarJohan Hovold <johan@kernel.org>
Link: https://lore.kernel.org/r/20191018151955.25135-2-johan@kernel.orgSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent ec83e4c9
...@@ -464,7 +464,7 @@ static ssize_t ld_usb_read(struct file *file, char __user *buffer, size_t count, ...@@ -464,7 +464,7 @@ static ssize_t ld_usb_read(struct file *file, char __user *buffer, size_t count,
/* wait for data */ /* wait for data */
spin_lock_irq(&dev->rbsl); spin_lock_irq(&dev->rbsl);
if (dev->ring_head == dev->ring_tail) { while (dev->ring_head == dev->ring_tail) {
dev->interrupt_in_done = 0; dev->interrupt_in_done = 0;
spin_unlock_irq(&dev->rbsl); spin_unlock_irq(&dev->rbsl);
if (file->f_flags & O_NONBLOCK) { 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, ...@@ -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); retval = wait_event_interruptible(dev->read_wait, dev->interrupt_in_done);
if (retval < 0) if (retval < 0)
goto unlock_exit; 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 contains actual_length + interrupt_in_buffer */
actual_buffer = (size_t *)(dev->ring_buffer + dev->ring_tail * (sizeof(size_t)+dev->interrupt_in_endpoint_size)); 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); bytes_to_read = min(count, *actual_buffer);
if (bytes_to_read < *actual_buffer) if (bytes_to_read < *actual_buffer)
dev_warn(&dev->intf->dev, "Read buffer overflow, %zd bytes dropped\n", 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 * ...@@ -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_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->interrupt_in_endpoint_size = usb_endpoint_maxp(dev->interrupt_in_endpoint);
dev->ring_buffer = dev->ring_buffer = kcalloc(ring_buffer_size,
kmalloc_array(ring_buffer_size, sizeof(size_t) + dev->interrupt_in_endpoint_size,
sizeof(size_t) + dev->interrupt_in_endpoint_size, GFP_KERNEL);
GFP_KERNEL);
if (!dev->ring_buffer) if (!dev->ring_buffer)
goto error; goto error;
dev->interrupt_in_buffer = kmalloc(dev->interrupt_in_endpoint_size, GFP_KERNEL); dev->interrupt_in_buffer = kmalloc(dev->interrupt_in_endpoint_size, GFP_KERNEL);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment