Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1209817

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

From Bjørn Mork <bjorn@mork.no>
Newsgroups linux.kernel
Subject Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH
Date 2015-08-19 14:40 +0200
Message-ID <pZaOS-5Zp-11@gated-at.bofh.it> (permalink)
References (1 earlier) <pXquL-7Tq-27@gated-at.bofh.it> <pZ0Pv-8bV-1@gated-at.bofh.it> <pZ6rU-83g-23@gated-at.bofh.it> <pZ9g6-3EV-1@gated-at.bofh.it> <pZaca-50h-17@gated-at.bofh.it>
Organization m

Show all headers | View raw


Eugene Shatokhin <eugene.shatokhin@rosalab.ru> 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.

>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] usbnet: Fix two races between usbnet_stop() and the BH Eugene Shatokhin <eugene.shatokhin@rosalab.ru> - 2015-08-14 19:00 +0200
  Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH David Miller <davem@davemloft.net> - 2015-08-19 04:00 +0200
    Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Eugene Shatokhin <eugene.shatokhin@rosalab.ru> - 2015-08-19 10:00 +0200
      Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Bjørn Mork <bjorn@mork.no> - 2015-08-19 13:00 +0200
        Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Eugene Shatokhin <eugene.shatokhin@rosalab.ru> - 2015-08-19 14:00 +0200
          Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Bjørn Mork <bjorn@mork.no> - 2015-08-19 14:40 +0200
            Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Eugene Shatokhin <eugene.shatokhin@rosalab.ru> - 2015-08-24 14:30 +0200
              Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Bjørn Mork <bjorn@mork.no> - 2015-08-24 15:40 +0200
                Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Eugene Shatokhin <eugene.shatokhin@rosalab.ru> - 2015-08-24 19:10 +0200
                Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Oliver Neukum <oneukum@suse.com> - 2015-08-25 14:40 +0200
          Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH David Miller <davem@davemloft.net> - 2015-08-24 19:50 +0200
            Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Alan Stern <stern@rowland.harvard.edu> - 2015-08-24 20:10 +0200
              Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Alan Stern <stern@rowland.harvard.edu> - 2015-08-24 20:30 +0200
                Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Oliver Neukum <oneukum@suse.com> - 2015-08-25 14:40 +0200
              Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH David Miller <davem@davemloft.net> - 2015-08-24 20:40 +0200
            Re: [PATCH] usbnet: Fix two races between usbnet_stop() and the BH Eugene Shatokhin <eugene.shatokhin@rosalab.ru> - 2015-08-24 20:20 +0200

csiph-web