>Number: 126884
>Category: usb
>Synopsis: [patch] Bug in buffer handling in ugen.c
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-usb
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Wed Aug 27 11:40:02 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator: Daan Vreeken [PA4DAN]
>Release: FreeBSD 7.0-STABLE amd64
>Organization:
VEHosting.nl
>Environment:
System: FreeBSD TVlottePert.VEHosting.nl 7.0-STABLE FreeBSD 7.0-STABLE #0: Fri
Jun 13 18:02:09 CEST 2008 [EMAIL PROTECTED]:/usr/obj/usr/src/sys/GENERIC amd64
>Description:
In ugen_isoc_rintr() old data is thrown away when the fifo is full.
This code contains multiple errors :
o Only in the case of sce->fill < sce->cur data is thrown away.
(No data is discarded when sce->fill is near the end of the buffer
and sce->cur is near the beginning of the buffer.)
o Too much data is discarded. If 10 bytes need to be written with
only space left for 8 bytes, 10 bytes of input are discarded instead
of just 2.
o When sce->cur reaches/passes the end of the buffer, the pointer is
set to sce->ibuf + (sce->limit - sce->cur) instead of to
sce->ibuf + (sce->cur - sce->limit). This could leave the sce->cur
pointer ahead of the buffer pointing to unknown memory.
>How-To-Repeat:
Try to use the ugen interface to read data from a device with an
isochronous endpoint that delivers isochronous packets of varying
length and see that data gets corrupted.
>Fix:
The attached patch is to -CURRENT, but the bug is found in other
branches too.
The patch moves the deletion of data into the while loop that copies
data from the incoming packet. This way data is also discarded in the
case where sce->fill >= sce->cur initially (because the while loop
already breaks filling the end & beginning of the buffer into two
itterations).
--- ugen.c.patch begins here ---
--- ugen.c-1.112 2008-08-27 11:48:37.000000000 +0200
+++ ugen.c 2008-08-27 12:08:10.000000000 +0200
@@ -1054,15 +1054,6 @@
DPRINTFN(5,("ugen_isoc_rintr: xfer %td, count=%d\n", req - sce->isoreqs,
count));
- /* throw away oldest input if the buffer is full */
- if(sce->fill < sce->cur && sce->cur <= sce->fill + count) {
- sce->cur += count;
- if(sce->cur >= sce->limit)
- sce->cur = sce->ibuf + (sce->limit - sce->cur);
- DPRINTFN(5, ("ugen_isoc_rintr: throwing away %d bytes\n",
- count));
- }
-
isize = UGETW(sce->edesc->wMaxPacketSize);
for (i = 0; i < UGEN_NISORFRMS; i++) {
u_int32_t actlen = req->sizes[i];
@@ -1071,6 +1062,22 @@
/* copy data to buffer */
while (actlen > 0) {
n = min(actlen, sce->limit - sce->fill);
+
+ /* throw away oldest input if the buffer is full */
+ if(sce->fill < sce->cur &&
+ sce->cur <= sce->fill + n) {
+ u_int32_t drop;
+
+ /* calculate fow many bytes we have to drop */
+ drop = scr->cur - sce->fill;
+
+ sce->cur += drop;
+ if(sce->cur >= sce->limit)
+ sce->cur = sce->ibuf;
+ DPRINTFN(5, ("ugen_isoc_rintr: throwing "
+ "away %d bytes\n", drop));
+ }
+
memcpy(sce->fill, buf, n);
buf += n;
--- ugen.c.patch ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
