On 2013.04.04 17:04, Toby Gray wrote: > As I mentioned in a previous email, I've been tracking down a rare NULL > pointer dereference in the windows desktop backend.
Thanks! > The problem is that the backend private information for transfers wasn't > getting correctly cleared before the transfer completion callback was > called. > > This meant that the fake fd number inside the transfer still contained > the old value. As there is a period of time where a transfer is in the > flying transfer list but hasn't been submitted to the backend, this > meant that the wrong transfer could be completed. > > The problematic sequence is along the lines of the following: > > 1. Client creates a transfer using libusb_alloc_transfer. Let's call > this transfer A. > 2. Transfer A is submitted, e.g. reading from bulk endpoint 0x81. > 3. The backend assigns a fake fd of 1 to transfer A and then submits it > to the OS. > 4. The OS completes the request. > 5. The backend detects this when next handling events and the client has > the callbacks called. > 6. Without the fix in this patch, transfer_priv->pollable_fd.fd still > has a value of one. > 7. Client creates a new transfer using libusb_alloc_transfer. Let's call > this transfer B. > 8. Transer B is submitted, e.g. reading from bulk endpoing 0x81 with > infinite timeout. > 9. The backend assigns a fake fd of 1 to transfer B and then submits it > to the OS. > 10. The client then submits transfer A, reading from bulk endpoint 0x82 > with a timeout of 2 seconds. > 11. Before the backend is given transfer A it is added to the flying > transfer list. > 12. The OS completes transfer B. > 13. A separate thread starts to run and asks libusb to handle events. > 14. The windows backend notices that fake fd 1 has been signalled and > starts to look through the flying transfer list. > 15. The windows backend finds transfer A as it is earlier in the flying > transfer list as it has an earlier timeout and has the old fake fd value > of 1 and thinks it's been completed. However the OVERLAPPED is NULL so > it causes a NULL pointer dereference. > > Tracking this down was further complicated by the NULL pointer > dereference taking a bit of time to handle. By which time the thread > which was resubmitting transfer A had gone and updated the fd value and > set a valid OVERLAPPED structure. > > The attached patch makes sure this can't happen by making sure > windows_clear_transfer_priv correctly clears the pollable_fd. This means > that step 15 in the above sequence correctly skips over transfer A as > the fake fd value will be -1 (until the transfer is actually submitted > to the OS). > > It's also a problem with the WinCE backend, so I've included a similar > fix in the patch. > > I understand that this is probably too risky a change to get into > 1.0.15, especially as no-one else seems to have reported it. Well this is a short patch, but I want to have a closer look at it before it gets pushed. In all likelihood, it'll be applied for the 1.0.15 release since resetting the pollable_fd after usbi_free_fd() is a harmless operation anyway. But I'd like to make sure there isn't more we need to look into. At any rate, thanks for investigating these hard to track issues. Much appreciated! Regards, /Pete ------------------------------------------------------------------------------ Minimize network downtime and maximize team effectiveness. Reduce network management and security costs.Learn how to hire the most talented Cisco Certified professionals. Visit the Employer Resources Portal http://www.cisco.com/web/learning/employer_resources/index.html _______________________________________________ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
