[PATCH v2] USB: cdc-wdm: fix read buffer overflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Bjørn Mork <bjorn@xxxxxxx>
---
Oliver Neukum <oneukum@xxxxxxx> 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 :-)

Returning an error is a problem.  This patch sets an error code, but wdm_read
will not return any error if there are data in the buffer.  This is nothing
special for this case.  The same problem will affect any read error.  We may 
consider fixing that in the future, but I do not want to mix that fix into
this critical one.  Better keep this simple and minimalistic.  I verified that
it applies cleanly to 3.0-stable.

Slightly tested on an Ericsson F5521gw by feeding it "ATI\r" without reading
the response until it starts barfing:

 Feb 14 17:46:29 nemi kernel: [13125.530918] cdc_wdm 8-1:1.5: buffer full. dropping 84 bytes
 Feb 14 17:46:29 nemi kernel: [13125.546932] cdc_wdm 8-1:1.5: buffer full. dropping 84 bytes

etc.


Bjørn


 drivers/usb/class/cdc-wdm.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 5f0cb41..1388828 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -184,6 +184,13 @@ static void wdm_in_callback(struct urb *urb)
 		}
 	}
 
+	/* not enough space left in the buffer? */
+	if (desc->length + urb->actual_length > desc->wMaxCommand) {
+		dev_dbg(&desc->intf->dev, "buffer full. dropping %d bytes\n",
+			urb->actual_length);
+		desc->rerr = -ENOBUFS;
+		goto skip_error;
+	}
 	desc->rerr = status;
 	desc->reslength = urb->actual_length;
 	memmove(desc->ubuf + desc->length, desc->inbuf, desc->reslength);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux