Skip to content

Commit dd23879

Browse files
thejhjfvogel
authored andcommitted
usb: cdc-acm: Check control transfer buffer size before access
commit e563b01 upstream. If the first fragment is shorter than struct usb_cdc_notification, we can't calculate an expected_size. Log an error and discard the notification instead of reading lengths from memory outside the received data, which can lead to memory corruption when the expected_size decreases between fragments, causing `expected_size - acm->nb_index` to wrap. This issue has been present since the beginning of git history; however, it only leads to memory corruption since commit ea25835 ("cdc-acm: reassemble fragmented notifications"). A mitigating factor is that acm_ctrl_irq() can only execute after userspace has opened /dev/ttyACM*; but if ModemManager is running, ModemManager will do that automatically depending on the USB device's vendor/product IDs and its other interfaces. Cc: stable <stable@kernel.org> Fixes: 1da177e ("Linux-2.6.12-rc2") Signed-off-by: Jann Horn <jannh@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> (cherry picked from commit f64079bef6a8a7823358c3f352ea29a617844636) Signed-off-by: Jack Vogel <jack.vogel@oracle.com>
1 parent b26babb commit dd23879

File tree

1 file changed

+14
-3
lines changed

1 file changed

+14
-3
lines changed

drivers/usb/class/cdc-acm.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ static void acm_process_notification(struct acm *acm, unsigned char *buf)
371371
static void acm_ctrl_irq(struct urb *urb)
372372
{
373373
struct acm *acm = urb->context;
374-
struct usb_cdc_notification *dr = urb->transfer_buffer;
374+
struct usb_cdc_notification *dr;
375375
unsigned int current_size = urb->actual_length;
376376
unsigned int expected_size, copy_size, alloc_size;
377377
int retval;
@@ -398,9 +398,20 @@ static void acm_ctrl_irq(struct urb *urb)
398398

399399
usb_mark_last_busy(acm->dev);
400400

401-
if (acm->nb_index)
401+
if (acm->nb_index == 0) {
402+
/*
403+
* The first chunk of a message must contain at least the
404+
* notification header with the length field, otherwise we
405+
* can't get an expected_size.
406+
*/
407+
if (current_size < sizeof(struct usb_cdc_notification)) {
408+
dev_dbg(&acm->control->dev, "urb too short\n");
409+
goto exit;
410+
}
411+
dr = urb->transfer_buffer;
412+
} else {
402413
dr = (struct usb_cdc_notification *)acm->notification_buffer;
403-
414+
}
404415
/* size = notification-header + (optional) data */
405416
expected_size = sizeof(struct usb_cdc_notification) +
406417
le16_to_cpu(dr->wLength);

0 commit comments

Comments
 (0)