On Wed, 16 Oct 2013, Sarah Sharp wrote: > I think there's some nasty race conditions here. There's several > different structures this code and other functions are manipulating: > > - the endpoint ring contents > - the endpoint ring dequeue pointer > - the endpoint's cur_td and cancelled_td lists > > The other functions you need to look at are xhci_urb_enqueue, > xhci_urb_dequeue, and xhci_handle_cmd_stop_ep. Arguably, we should be > doing something about drivers attempting to change alternate interface > settings at the same time they're resetting endpoints, but I think the > solution should be that drivers just shouldn't do that. However, we do > need to handle the case where the reset endpoint races with URB > cancellation and enqueue. You can simplify part of the problem by not allowing an endpoint to be reset if it has any pending URBs. Just fail the reset in this case. Also, while you're going through the whole remove-and-add procedure for endpoints that aren't halted, do you want to hold the bandwidth mutex? If the procedure isn't atomic, there's a possibility that some other device could change configs in the middle. This would also prevent concurrent altsetting changes. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
