Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH

Eugene Shatokhin [off-list ref] writes:

The problem is not in the reordering but rather in the fact that
"dev->flags = 0" is not necessarily atomic
w.r.t. "clear_bit(EVENT_RX_KILL, &dev->flags)", and vice versa.

So the following might be possible, although unlikely:

CPU0             CPU1
                 clear_bit: read dev->flags
                 clear_bit: clear EVENT_RX_KILL in the read value

dev->flags=0;

                 clear_bit: write updated dev->flags

As a result, dev->flags may become non-zero again.

Ah, right.  Thanks for explaining.

I cannot prove yet that this is an impossible situation. If anyone
can, please explain. If so, this part of the patch will not be needed.

I wonder if we could simply move the dev->flags = 0 down a few lines to
fix both issues?  It doesn't seem to do anything useful except for
resetting the flags to a sane initial state after the device is down.

Stopping the tasklet rescheduling etc depends only on netif_running(),
which will be false when usbnet_stop is called.  There is no need to
touch dev->flags for this to happen.

quoted

The EVENT_NO_RUNTIME_PM bug should definitely be fixed.  Please split
that out as a separate fix.  It's a separate issue, and should be
backported to all maintained stable releases it applies to (anything
from v3.8 and newer)

Yes, that makes sense. However, this fix was originally provided by
Oliver Neukum rather than me, so I would like to hear his opinion as
well first.

If what I write above is correct (please help me verify...), then maybe
it does make sense to do these together anyway.



Bjørn