Date: Wed, 27 Aug 2008 12:58:39 +0200 (CEST) From: "Daan Vreeken [PA4DAN]" <Daan@VEHosting.nl> To: FreeBSD-gnats-submit@FreeBSD.org Cc: FreeBSD@VEHosting.nl Subject: usb/126884: [patch] Bug in buffer handling in ugen.c Message-ID: <200808271058.m7RAwd0P043130@TVlottePert.VEHosting.nl> Resent-Message-ID: <200808271140.m7RBe39c023272@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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 root@TVlottePert.VEHosting.nl:/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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200808271058.m7RAwd0P043130>