On Thursday 14 February 2013 18:10:43 Bjørn Mork wrote:
> Do not scribble past end of buffer. Check if the userspace buffer has
> enough space available before attempting to move more data there. Drop
> new data on overflow.
>
> Cc: stable <[email protected]>
> Signed-off-by: Bjørn Mork <[email protected]>
> ---
> Oliver Neukum <[email protected]> writes:
>
> > I am afraid your diagnosis is correct. This is a buffer overflow. Not good.
> > The fix is a problem. It seems to me that we need to throw away the
> > newest data and report an error.
>
> OK. I preferred receiving the newest data for my use case, but I trust that
> you know what's best as usual :-)
We have to let user space recover. To do so we need to indicate when
exactly we dropped data.
How about this?
Regards
Oliver
>From 6be64bd7522f1c11a535741840797030c0436acf Mon Sep 17 00:00:00 2001
From: Oliver Neukum <[email protected]>
Date: Thu, 14 Feb 2013 21:26:35 +0100
Subject: [PATCH] cdc-wdm: fix buffer overflow
If a read overflows the limit under which a single transfer
can overflow the buffer we allow no further data into the buffer
and remember an error
Signed-off-by: Oliver Neukum <[email protected]>
---
drivers/usb/class/cdc-wdm.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 5f0cb41..2a5963b 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -184,10 +184,19 @@ static void wdm_in_callback(struct urb *urb)
}
}
+ /* hopeless congestion or error*/
+ if (desc->rerr < 0)
+ goto skip_error;
+
desc->rerr = status;
desc->reslength = urb->actual_length;
+ if (desc->length > desc->wMaxCommand) {
+ /* the buffer is full */
+ desc->rerr = -ENOBUFS;
+ }
memmove(desc->ubuf + desc->length, desc->inbuf, desc->reslength);
desc->length += desc->reslength;
+
skip_error:
wake_up(&desc->wait);
@@ -418,7 +427,7 @@ outnl:
static ssize_t wdm_read
(struct file *file, char __user *buffer, size_t count, loff_t *ppos)
{
- int rv, cntr;
+ int rv, cntr, err;
int i = 0;
struct wdm_device *desc = file->private_data;
@@ -465,10 +474,13 @@ retry:
spin_lock_irq(&desc->iuspin);
if (desc->rerr) { /* read completed, error happened */
- desc->rerr = 0;
- spin_unlock_irq(&desc->iuspin);
- rv = -EIO;
- goto err;
+ if (!desc->reslength) {
+ err = desc->rerr;
+ desc->rerr = 0;
+ spin_unlock_irq(&desc->iuspin);
+ rv = (err == -ENOBUFS) ? err : -EIO;
+ goto err;
+ }
}
/*
* recheck whether we've lost the race
@@ -714,7 +726,7 @@ static int wdm_create(struct usb_interface *intf, struct
usb_endpoint_descriptor
if (!desc->command)
goto err;
- desc->ubuf = kmalloc(desc->wMaxCommand, GFP_KERNEL);
+ desc->ubuf = kmalloc(desc->wMaxCommand * 2, GFP_KERNEL);
if (!desc->ubuf)
goto err;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
