On Fri, 2 Aug 2013, Stalley, Sean wrote: > > > tbuf, however, is statically allocated on the stack with a size of 15 > > > bytes, regardless of the size specified in the URB. > > > When this buffer is passed to the hcd via the hub_control() call, it > > > is advertized as being as large as the URB buffer ( via wLength ). > > > > > > when the hcd attempts to write a descriptor containing more than > > > 15 bytes ( such as the Standard BOS Descriptor for hubs, defined in > > > the USB3.0 Spec, section 10.13.1 ) causes tbuf to overflow onto the > > > stack. > > > > No it doesn't. The usb_bos_descriptor data defined in xhci-hub.c is 15 > > bytes > > long. As far as I know, none of the descriptors written by the HCDs are > > longer > > than that. > > > > You are completely right, no current HCD writes more than a 15 byte BOS > descriptor. > However, if a future HCD were to attempt to write a descriptor longer than 15 > bytes, > the value of wLength would be larger than the space available in tbuf, > and an overflow would occur.
True. At that future time, tbuf would have to be expanded. > > > This patchset includes 2 independant options, it is not necessary to > > > apply both. Option 1 simply increases the size of tbuf, so that it can > > > hold larger descriptors. Option 2 circumvents tbuf and passes > > > nongeneric descriptors (such as BOS > > > descriptors) directly to the buffer in the urb, preventing the > > > overflow. > > > > Neither patch is acceptable. Increasing tbuf to 4096 bytes uses far too > > much > > stack space. Passing data directly to the URB's buffer won't work, because > > that > > buffer may be smaller than the amount of data copied by the HCD. > > > > I'll admit 4k is a little larger than necessary (especially considering no > current > implementation uses more than 15 bytes), but I am confused why we cannot pass > the data to the URB's buffer directly. wLength comes from URB, and indicates > the > size of the URB's buffer. When would this value be smaller than the size of > the URB's buffer? wLength won't be smaller than the URB's buffer, but it might be smaller than the amount of data that the HCD copies to the buffer. The HCDs copy their entire data structures; they don't limit the amount of data to wLength bytes. > > > A third option would be to rewrite the rh_call function. > > > > If you want the code to be a little less fragile than it is now, you could > > increase > > tbuf to 64 bytes, or something like that. However, at the moment I don't > > see > > why any change is needed. > > That would work, but we should probably change the value of wLength being > passed to the HCD > to the size of the smaller of the 2 buffers (tbuf and the URB buffer). > Otherwise a request that's too > large would still cause an overflow of tbuf. You must not change wLength. Increasing the size of tbuf is okay, as long as you don't add too much stack pressure. If necessary, tbuf can be allocated dynamically rather than statically. You could also change all the HCDs, to make them limit the amount of data they copy. I think changing tbuf would be easier. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
