01.09.2015 17:05, Eugene Shatokhin пишет:
The race may happen when a device (e.g. YOTA 4G LTE Modem) is
unplugged while the system is downloading a large file from the Net.
Hardware breakpoints and Kprobes with delays were used to confirm that
the race does actually happen.
The race is on
"!skb_queue_empty(&dev->done)" is checked. So
usbnet_terminate_urbs() may stop waiting and return while dev->done
queue still has an item.
Locking in defer_bh() and usbnet_terminate_urbs() was revisited to avoid
this race.
Signed-off-by: Eugene Shatokhin
---
drivers/net/usb
01.09.2015 10:58, Oliver Neukum пишет:
On Mon, 2015-08-31 at 11:50 +0300, Eugene Shatokhin wrote:
But I would have liked it much better if the code became simpler
instead
of more complex.
Me too, but I can see no other way here. The code is simpler without
locking, indeed, but locking is
31.08.2015 10:32, Bjørn Mork пишет:
Eugene Shatokhin writes:
28.08.2015 11:55, Bjørn Mork пишет:
I guess you are right. At least I cannot prove that you are not :)
There is a bit too much complexity involved here for me...
:-)
Yes, it is quite complex.
I admit, it was easier for me to
28.08.2015 11:55, Bjørn Mork пишет:
Eugene Shatokhin writes:
25.08.2015 00:01, Bjørn Mork пишет:
Eugene Shatokhin writes:
The race may happen when a device (e.g. YOTA 4G LTE Modem) is
unplugged while the system is downloading a large file from the Net.
Hardware breakpoints and Kprobes
25.08.2015 00:01, Bjørn Mork пишет:
Eugene Shatokhin writes:
The race may happen when a device (e.g. YOTA 4G LTE Modem) is
unplugged while the system is downloading a large file from the Net.
Hardware breakpoints and Kprobes with delays were used to confirm that
the race does actually happen
The following problems found when investigating races in usbnet module
are fixed here:
1. EVENT_NO_RUNTIME_PM bit of dev->flags should be read before it is
cleared by "dev->flags = 0". Thanks to Oliver Neukum for spotting this
problem and providing a fix.
2. A race on on skb_queue between usbne
It is needed to check EVENT_NO_RUNTIME_PM bit of dev->flags in
usbnet_stop(), but its value should be read before it is cleared
when dev->flags is set to 0.
The problem was spotted and the fix was provided by
Oliver Neukum .
Signed-off-by: Eugene Shatokhin
---
drivers/net/usb/usbnet
"!skb_queue_empty(&dev->done)" is checked. So
usbnet_terminate_urbs() may stop waiting and return while dev->done
queue still has an item.
Locking in defer_bh() and usbnet_terminate_urbs() was revisited to avoid
this race.
Signed-off-by: Eugene Shatokhin
---
drivers/net/usb
24.08.2015 20:43, David Miller пишет:
From: Eugene Shatokhin
Date: Wed, 19 Aug 2015 14:59:01 +0300
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
24.08.2015 16:29, Bjørn Mork пишет:
Eugene Shatokhin writes:
19.08.2015 15:31, Bjørn Mork пишет:
Eugene Shatokhin 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, &
19.08.2015 15:31, Bjørn Mork пишет:
Eugene Shatokhin 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
19.08.2015 13:54, Bjørn Mork пишет:
Eugene Shatokhin writes:
19.08.2015 04:54, David Miller пишет:
From: Eugene Shatokhin
Date: Fri, 14 Aug 2015 19:58:36 +0300
2. The second race is on dev->flags.
dev->flags is set to 0 here:
*0 usbnet_stop (usbnet.c:816)
/* deferred work
19.08.2015 04:54, David Miller пишет:
From: Eugene Shatokhin
Date: Fri, 14 Aug 2015 19:58:36 +0300
2. The second race is on dev->flags.
dev->flags is set to 0 here:
*0 usbnet_stop (usbnet.c:816)
/* deferred work (task, timer, softirq) must also stop.
* can't flush_sch
rate */
clear_bit(EVENT_RX_KILL, &dev->flags);
It seems, setting dev->flags to 0 is not necessarily atomic w.r.t.
clear_bit() and other bit operations with dev->flags. It is safer to
make it atomic and this way, make the race harmless.
While at i
Hi,
21.07.2015 17:22, Oliver Neukum пишет:
On Mon, 2015-07-20 at 21:13 +0300, Eugene Shatokhin wrote:
And here, the code clears EVENT_RX_KILL bit in dev->flags, which may
execute concurrently with the above operation:
#0 clear_bit (bitops.h:113, inlined)
#1 usbnet_bh (usbnet.c:1
27.07.2015 13:00, Oliver Neukum пишет:
On Fri, 2015-07-24 at 17:41 +0300, Eugene Shatokhin wrote:
23.07.2015 12:15, Oliver Neukum пишет:
From what I see now in Documentation/atomic_ops.txt, stores to the
properly aligned memory locations are in fact atomic.
They are, but again only with
27.07.2015 15:29, Oliver Neukum пишет:
On Fri, 2015-07-24 at 20:38 +0300, Eugene Shatokhin wrote:
21.07.2015 15:04, Oliver Neukum пишет:
your analysis is correct and it looks like in addition to your proposed
fix locking needs to be simplified and a common lock to be taken.
Suggestions
21.07.2015 15:04, Oliver Neukum пишет:
On Mon, 2015-07-20 at 21:13 +0300, Eugene Shatokhin wrote:
Hi,
I have recently found several data races in "usbnet" module, checked on
vanilla kernel 4.1.0 on x86_64. The races do actually happen, I have
confirmed it by adding delays and usin
23.07.2015 12:15, Oliver Neukum пишет:
On Wed, 2015-07-22 at 21:33 +0300, Eugene Shatokhin wrote:
The following part is not necessary, I think. usbnet_bh() does not
touch
EVENT_NO_RUNTIME_PM bit explicitly and these bit operations are
atomic
w.r.t. each other.
+ mpn |= !test_and_clear_bit
23.07.2015 12:43, Oliver Neukum пишет:
On Mon, 2015-07-20 at 21:13 +0300, Eugene Shatokhin wrote:
[Race #5]
Race on dev->rx_urb_size. I reproduced it a similar way as the races
#2
and #3 (changing MTU while downloading files).
dev->rx_urb_size is written to here:
#0 usbnet_chan
21.07.2015 17:22, Oliver Neukum пишет:
On Mon, 2015-07-20 at 21:13 +0300, Eugene Shatokhin wrote:
And here, the code clears EVENT_RX_KILL bit in dev->flags, which may
execute concurrently with the above operation:
#0 clear_bit (bitops.h:113, inlined)
#1 usbnet_bh (usbnet.c:1
t() is atomic w.r.t. setting dev->flags to 0, this race is
not a problem, I guess. Otherwise, it may be.
--
[Race #5]
Race on dev->rx_urb_size. I reproduced it a similar way as the races #2
and #3 (changing MTU while downloading files).
dev->rx_urb_size is written
23 matches
Mail list logo
