DFU upload causes memory corruption (patch)

[Harald Welte]

On Tue, Mar 13, 2007 at 06:14:18AM -0300, Werner Almesberger wrote:
> Harald Welte wrote:
> > The fundamental question was: Why doesn't the current code work?
> > Because urb->buffer is too small for remain?  Then I suggest we restrict
> > the length of a transfer to the size of urb->buffer_data
> 
> Hmm, wouldn't dfu-util treat this as EOF ? At least sam7dfu.c says
> 
>                 if (rc < xfer_size) {
>                         /* last block, return */

oh yes, indeed. you're right.

> > If we stay with the current hack, an audit whether urb->buffer really is
> > properly reset for every control point request would be good.
> 
> How many URBs are you juggling in the system anyway ? If there's
> just one, or just a small number, why not give each just a "worst
> case" constant buffer ?

Well, the problem is that it doesn't distinguish between EP0 urbs
(which usually are a couple of bytes large, only DFU transfers huge data
over the control endpoint) and actual data URB's.  

I haven't looked at the usbtty code into much detail, but I'm not sure
how often it allocates/free's URB's.  If that happens every character
(worst case) then we don't want to allocate several kilobytes of memory
each time.

So in any caes, maybe we shoul get rid of that static buffer in 'struct
urb' alltogether and dynamically allocate, just like "first class
citizen" usb code does (and the usbdcore code did before somebody hacked
it for u-boot).

At least in the DFU case, EP0 would then be allocated with 4096 byte
data buffer, just to accomodate one 'transfer size' block.

Cheers,
-- 
- Harald Welte <laforge at openmoko.org>          	        http://openmoko.org/
============================================================================
Software for the world's first truly open Free Software mobile phone