Re: Crash in reads with short timeout - race.

On Mon, Apr 2, 2018 at 12:32 AM, Peter Dons Tychsen via
Libusb-win32-devel <[email protected]> wrote:
>
> The first revision that had this was:
>
> https://github.com/mcuee/libusb-win32/commit/6eaec78aed8e34e1aaf2e835b7
> 01497d352bc3e6#diff-b5691da501465b646e19ae693a116e5e

Wow, that was in the early days of libusb-win32.

> It was later reformatted into being a part of _usb_cancel_io().
>
> However, WaitForSingleObject() is not called correctly. If you want to
> be sure that the cancel has been executed, you must call
> WaitForSingleObject() with a timeout that is large enough to ensure
> that the request is processed. In many cases on a loaded system, this
> ends tragically.
>
> A simple statement can crash, e.g.:
>
> -- snip --
>
> void test(void)
> {
>   uint8_t data[20];
>   usb_interrupt_read(handle, ep_int_in, data, 20, 1);
> }
>
> -- snip --
>
> The above code crashes in some cases, as the read is in some rare cases
> executed *after* leaving usb_interrupt_read() and test(), which then
> corrupts the local stack as "data" is not valid any more.
>
> To fix this, simply use INFINITE as timeout, as you can *not* return
> from _usb_cancel_io until the operation has been truely cancelled. It
> is also described on MSDN for WaitForSingleObject() that the only was
> to be sure is to call it with INFINITE.
>
> I have tested the fix on lots of setups, and i think the fix is OK, and
> does not seem to create other new side-effects.

Thanks a lot for your detailed information. I will see if Travis still monitors
this mailing list and can comment on this fix.

The good thing is that it only touches the library (DLL) part and not
the driver part (which requires digital signature).

> It was also partially fixable by cancelling all I/O to the driver using
> CancelIo(). However, this will cancel all I/O and will ruin other
> running endpoints (async readers and such), so that will not work.
> Newer version of windows have CancelIoEx() which can cancel a specific
> I/O operation, but this is only available in 2008/win7 and forward, and
> i need this to still run on xp/vista.

I think CancellIOEx() is available since Vista. But it does not matter
if you still need XP.


-- 
Xiaofan

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot