On 2012.08.02 19:30, Orin Eman wrote:
> I've seen the list get screwed up by trying to reissue a transfer
> before getting the callback. The list got a circular segment in
> that case. Perhaps something like cancelling a transfer followed by
> freeing it before getting the callback causes this?
Thanks for the info. This may be what happens.
Another suspicious thing I have is with regards to what happens when
LIBUSB_ERROR_BUSY is returned in libusb_submit_transfer() [1].
From looking at Linux's submit_bulk_transfer(), which can indeed return
LIBUSB_ERROR_BUSY, I assume this can happen if the same transfer is
submitted twice for whatever reason. If that happens however, we're
still going to call list_del(), which we probably shouldn't do at this
stage as a second list_del() call for the same entry is likely to cause
some damage to our list.
> In my case, I was trapping the list getting a circular segment, so I
> made the following changes:
>
> static inline void list_del(struct list_head *entry)
> {
> entry->next->prev = entry->prev;
> entry->prev->next = entry->next;
> entry->next = entry->prev = NULL;
> }
I guess the change above is probably something we want, as it should
make any double list_del() call very explicit, if these are actually an
issue, through a NULL deref. Unless there are objections, I'll try to
push a commit with the nulling of prev/next in list_del.
I may also add the skipping of list_del() for LIBUSB_ERROR_BUSY if some
of you agree that we have a potential issue there.
> Setting next and prev to NULL in list_del() caused a crash in
> usbi_cond_destroy() in threads_windows.c (next was used after calling
> list_del). Using list_for_each_entry_safe fixed it and cleaned up the code.
>
> int usbi_cond_destroy(usbi_cond_t *cond) {
> // This assumes no one is using this anymore. The check MAY NOT BE
> safe.
> struct usbi_cond_perthread *pos, *next_pos = NULL;
> if(!cond) return ((errno=EINVAL));
> if(!list_empty(&cond->waiters)) return ((errno=EBUSY )); // (!see
> above!)
> list_for_each_entry_safe(pos, next_pos, &cond->not_waiting, list,
> struct usbi_cond_perthread) {
> CloseHandle(pos->event);
> list_del(&pos->list);
> free(pos);
> }
>
> return 0;
> }
Interesting. Do you know in which call next was being used?
Still, if we apply the nulling of prev/next, we're likely to get reports
from Windows users with the same issue, that we can address then.
> When I was looking at my problem, all the use of the flying transfer
> list looked properly protected, so my guess is still that the problem is
> outside of libusb and a transfer is being freed while still on the list.
Yeah. My money is on a freeing/deletion part right now.
Regards,
/Pete
[1] https://github.com/libusbx/libusbx/blob/master/libusb/io.c#L1307
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
