Thread (7 messages) flat view 7 messages, 4 authors, 2014-03-11

Re: usbnet: driver_info->stop required to stop USB interrupts?

From: Julius Werner <jwerner@chromium.org>
Date: 2014-03-11 02:53:36

I think usbnet_stop() raced with the dev->bh tasklet, which by itself
might not be a problem (usbnet_stop() later kills the tasklet itself,
so it should expect that it can be running before that). The issue is
that it calls usbnet_terminate_urbs() before that, which temporarily
installs a waitqueue in dev->wait in order to be able to wait on the
tasklet to run and finish up some queues. The waiting itself looks
okay, but the access to 'dev->wait' is totally unprotected and can
race arbitrarily. I think in this case usbnet_bh() managed to succeed
it's dev->wait check just before usbnet_terminate_urbs() sets it back
to NULL. The latter then finishes and the waitqueue_t structure on its
stack gets overwritten by other functions halfway through the
wake_up() call in usbnet_bh().

I think the best solution would be to just make dev->wait a directly
embedded structure inside struct usbnet instead of a pointer to
something stack-allocated. usbnet_bh() could just call wake_up()
unconditionally (if empty it will be a noop), and then one other check
for !dev->wait could be replaced with a call to waitqueue_active().
Then the waitqueue-internal locks should be enough to protect all
accesses.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help