On Tue, Nov 08, 2005 at 07:24:29AM +0100, Oliver Neukum wrote:
> Am Dienstag, 8. November 2005 01:56 schrieb Herbert Xu:
> > 
> > The kaweth driver does not delete the TX URB in kaweth_close().
> > As a result the TX URB may still be active in the USB subsystem.
> > If kaweth_open() is called quickly afterwards, the networking
> > layer could submit another packet before the existing TX URB has
> > completed.
> 
> This begs the question how your first patch manages to avoid
> a race between hard_start_xmit() and disconnect()?

The existing usb_kill_urb calls in kaweth_disconnect() is racy:

CPU0                            CPU1
kaweth_start_xmit
        removed == 0
                                kaweth_disconnect
                                        removed = 1
                                        usb_kill_urb(tx_urb)
        usb_submit_urb(tx_urb)

With my patches you have either:

CPU0                            CPU1
dev_queue_xmit                  kaweth_disconnect -> unregister_netdev
                                        dev_deactivate
        spin_lock(queue_lock)
        enqueue
        qdisc_run
        spin_unlock(queue_lock)
                                                spin_lock(queue_lock)
                                                qdisc = noop
                                                qdisc_reset
                                                spin_unlock(queue_lock)
                                        dev_close
                                                kaweth_close
                                                        usb_kill_urb(tx_urb)

Or

CPU0                            CPU1
dev_queue_xmit                  kaweth_disconnect -> unregister_netdev
                                        dev_deactivate
                                                spin_lock(queue_lock)
                                                qdisc = noop
                                                qdisc_reset
                                                spin_unlock(queue_lock)
                                        dev_close
                                                kaweth_close
                                                        usb_kill_urb(tx_urb)
        spin_lock(queue_lock)
        enqueue -> kfree_skb
        qdisc_run
        spin_unlock(queue_lock)

In either cases by the time dev_deactivate terminates, no further calls
to kaweth_start_xmit are possible.  All calls made before its termination
will have terminated due to the spin_unlock_wait in dev_deactivate and
any tx_urb submitted will be taken care of by the usb_kill_urb in
kaweth_close.

Actually, there may be one unusual case where it could break.  Namely when
the user uses a qdisc with no enqueue function then the queue_lock won't
be taken and the above analysis is no longer valid.

However, this would really a bug in dev_queue_xmit since it won't only
affect kaweth, but many other NIC drivers as well.  So we should fix it
there (if this bug exists at all).

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to