summaryrefslogtreecommitdiffstats
path: root/net/core/dev.c
diff options
context:
space:
mode:
authorPatrick McHardy <kaber@trash.net>2006-09-27 16:45:45 -0700
committerDavid S. Miller <davem@sunset.davemloft.net>2006-09-28 18:01:50 -0700
commit85670cc1faa2e1472e4a423cbf0b5e3d55c5ba88 (patch)
treea5da67836995f1b04c844071db97608bc2c37b85 /net/core/dev.c
parent787e0617e5176176c494a787f1b0a5248a3db568 (diff)
downloadblackbird-op-linux-85670cc1faa2e1472e4a423cbf0b5e3d55c5ba88.tar.gz
blackbird-op-linux-85670cc1faa2e1472e4a423cbf0b5e3d55c5ba88.zip
[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/dev.c')
-rw-r--r--net/core/dev.c14
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:
if (q->enqueue) {
/* Grab device queue */
spin_lock(&dev->queue_lock);
+ q = dev->qdisc;
+ if (q->enqueue) {
+ rc = q->enqueue(skb, q);
+ qdisc_run(dev);
+ spin_unlock(&dev->queue_lock);
- rc = q->enqueue(skb, q);
-
- qdisc_run(dev);
-
+ rc = rc == NET_XMIT_BYPASS ? NET_XMIT_SUCCESS : rc;
+ goto out;
+ }
spin_unlock(&dev->queue_lock);
- rc = rc == NET_XMIT_BYPASS ? NET_XMIT_SUCCESS : rc;
- goto out;
}
/* The device has no queue. Common case for software devices:
OpenPOWER on IntegriCloud