diff options
author | Patrick McHardy <kaber@trash.net> | 2006-09-27 19:45:45 -0400 |
---|---|---|
committer | David S. Miller <davem@sunset.davemloft.net> | 2006-09-28 21:01:50 -0400 |
commit | 85670cc1faa2e1472e4a423cbf0b5e3d55c5ba88 (patch) | |
tree | a5da67836995f1b04c844071db97608bc2c37b85 /net/core | |
parent | 787e0617e5176176c494a787f1b0a5248a3db568 (diff) |
[NET_SCHED]: Fix fallout from dev->qdisc RCU change
The move of qdisc destruction to a rcu callback broke locking in the
entire qdisc layer by invalidating previously valid assumptions about
the context in which changes to the qdisc tree occur.
The two assumptions were:
- since changes only happen in process context, read_lock doesn't need
bottem half protection. Now invalid since destruction of inner qdiscs,
classifiers, actions and estimators happens in the RCU callback unless
they're manually deleted, resulting in dead-locks when read_lock in
process context is interrupted by write_lock_bh in bottem half context.
- since changes only happen under the RTNL, no additional locking is
necessary for data not used during packet processing (f.e. u32_list).
Again, since destruction now happens in the RCU callback, this assumption
is not valid anymore, causing races while using this data, which can
result in corruption or use-after-free.
Instead of "fixing" this by disabling bottem halfs everywhere and adding
new locks/refcounting, this patch makes these assumptions valid again by
moving destruction back to process context. Since only the dev->qdisc
pointer is protected by RCU, but ->enqueue and the qdisc tree are still
protected by dev->qdisc_lock, destruction of the tree can be performed
immediately and only the final free needs to happen in the rcu callback
to make sure dev_queue_xmit doesn't access already freed memory.
Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/core')
-rw-r--r-- | net/core/dev.c | 14 |
1 files changed, 8 insertions, 6 deletions
diff --git a/net/core/dev.c b/net/core/dev.c index 14de297d024d..4d891beab138 100644 --- a/net/core/dev.c +++ b/net/core/dev.c | |||
@@ -1480,14 +1480,16 @@ gso: | |||
1480 | if (q->enqueue) { | 1480 | if (q->enqueue) { |
1481 | /* Grab device queue */ | 1481 | /* Grab device queue */ |
1482 | spin_lock(&dev->queue_lock); | 1482 | spin_lock(&dev->queue_lock); |
1483 | q = dev->qdisc; | ||
1484 | if (q->enqueue) { | ||
1485 | rc = q->enqueue(skb, q); | ||
1486 | qdisc_run(dev); | ||
1487 | spin_unlock(&dev->queue_lock); | ||
1483 | 1488 | ||
1484 | rc = q->enqueue(skb, q); | 1489 | rc = rc == NET_XMIT_BYPASS ? NET_XMIT_SUCCESS : rc; |
1485 | 1490 | goto out; | |
1486 | qdisc_run(dev); | 1491 | } |
1487 | |||
1488 | spin_unlock(&dev->queue_lock); | 1492 | spin_unlock(&dev->queue_lock); |
1489 | rc = rc == NET_XMIT_BYPASS ? NET_XMIT_SUCCESS : rc; | ||
1490 | goto out; | ||
1491 | } | 1493 | } |
1492 | 1494 | ||
1493 | /* The device has no queue. Common case for software devices: | 1495 | /* The device has no queue. Common case for software devices: |