Skip to content

Commit 7a6f22d

Browse files
jhovoldgregkh
authored andcommitted
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: 2824bd2 ("[PATCH] USB: add ldusb driver") Cc: stable <[email protected]> # 2.6.13 Reported-by: [email protected] Signed-off-by: Johan Hovold <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent ec83e4c commit 7a6f22d

File tree

1 file changed

+11
-7
lines changed

1 file changed

+11
-7
lines changed

drivers/usb/misc/ldusb.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ static ssize_t ld_usb_read(struct file *file, char __user *buffer, size_t count,
464464

465465
/* wait for data */
466466
spin_lock_irq(&dev->rbsl);
467-
if (dev->ring_head == dev->ring_tail) {
467+
while (dev->ring_head == dev->ring_tail) {
468468
dev->interrupt_in_done = 0;
469469
spin_unlock_irq(&dev->rbsl);
470470
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,
474474
retval = wait_event_interruptible(dev->read_wait, dev->interrupt_in_done);
475475
if (retval < 0)
476476
goto unlock_exit;
477-
} else {
478-
spin_unlock_irq(&dev->rbsl);
477+
478+
spin_lock_irq(&dev->rbsl);
479479
}
480+
spin_unlock_irq(&dev->rbsl);
480481

481482
/* actual_buffer contains actual_length + interrupt_in_buffer */
482483
actual_buffer = (size_t *)(dev->ring_buffer + dev->ring_tail * (sizeof(size_t)+dev->interrupt_in_endpoint_size));
484+
if (*actual_buffer > dev->interrupt_in_endpoint_size) {
485+
retval = -EIO;
486+
goto unlock_exit;
487+
}
483488
bytes_to_read = min(count, *actual_buffer);
484489
if (bytes_to_read < *actual_buffer)
485490
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 *
690695
dev_warn(&intf->dev, "Interrupt out endpoint not found (using control endpoint instead)\n");
691696

692697
dev->interrupt_in_endpoint_size = usb_endpoint_maxp(dev->interrupt_in_endpoint);
693-
dev->ring_buffer =
694-
kmalloc_array(ring_buffer_size,
695-
sizeof(size_t) + dev->interrupt_in_endpoint_size,
696-
GFP_KERNEL);
698+
dev->ring_buffer = kcalloc(ring_buffer_size,
699+
sizeof(size_t) + dev->interrupt_in_endpoint_size,
700+
GFP_KERNEL);
697701
if (!dev->ring_buffer)
698702
goto error;
699703
dev->interrupt_in_buffer = kmalloc(dev->interrupt_in_endpoint_size, GFP_KERNEL);

0 commit comments

Comments
 (0)