Skip to contentSkip to conversation

net-sched 01/02: change tcf_destroy_chain() to clear start of filter list

PATCH Patrick McHardy 2 participants 2 messages Jul 1, 2008 – Jul 2, 2008

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

These two patches fix a potential use-after-free in hfsc and atm filter destruction. All filters must be destroyed before beginning class destruction since the destination class must still be alive to unbind the filter.

commit 887cfcf1e17d3c387dd43bb85f184253134648e4

Author: Patrick McHardy <[email protected]>
Date:   Thu Jun 26 18:31:02 2008 +0200

net-sched: change tcf_destroy_chain() to clear start of filter list

Pass double tcf_proto pointers to tcf_destroy_chain() to make it clear the start of the filter list for more consistency.

include/net/sch_generic.h +1 -1
index ab502ec..a87fc03 100644
@@ -178,7 +178,7 @@ extern struct Qdisc *qdisc_alloc(struct net_device *dev, struct Qdisc_ops *ops);
 extern struct Qdisc *qdisc_create_dflt(struct net_device *dev,
 				       struct Qdisc_ops *ops, u32 parentid);
 extern void tcf_destroy(struct tcf_proto *tp);
-extern void tcf_destroy_chain(struct tcf_proto *fl);
+extern void tcf_destroy_chain(struct tcf_proto **fl);
 
 static inline int __qdisc_enqueue_tail(struct sk_buff *skb, struct Qdisc *sch,
 				       struct sk_buff_head *list)
net/mac80211/wme.c +1 -2
index cfa8fbb..58d8b1b 100644
@@ -308,8 +308,7 @@ static void wme_qdiscop_destroy(struct Qdisc* qd)
 	struct ieee80211_hw *hw = &local->hw;
 	int queue;
 
-	tcf_destroy_chain(q->filter_list);
-	q->filter_list = NULL;
+	tcf_destroy_chain(&q->filter_list);
 
 	for (queue = 0; queue < QD_NUM(hw); queue++) {
 		skb_queue_purge(&q->requeued[queue]);
net/sched/sch_api.c +3 -3
index c40773c..10f01ad 100644
@@ -1252,12 +1252,12 @@ void tcf_destroy(struct tcf_proto *tp)
 	kfree(tp);
 }
 
-void tcf_destroy_chain(struct tcf_proto *fl)
+void tcf_destroy_chain(struct tcf_proto **fl)
 {
 	struct tcf_proto *tp;
 
-	while ((tp = fl) != NULL) {
-		fl = tp->next;
+	while ((tp = *fl) != NULL) {
+		*fl = tp->next;
 		tcf_destroy(tp);
 	}
 }
net/sched/sch_atm.c +2 -3
index 3352734..8e5f70b 100644
@@ -160,7 +160,7 @@ static void atm_tc_put(struct Qdisc *sch, unsigned long cl)
 	*prev = flow->next;
 	pr_debug("atm_tc_put: qdisc %p\n", flow->q);
 	qdisc_destroy(flow->q);
-	tcf_destroy_chain(flow->filter_list);
+	tcf_destroy_chain(&flow->filter_list);
 	if (flow->sock) {
 		pr_debug("atm_tc_put: f_count %d\n",
 			file_count(flow->sock->file));
@@ -588,8 +588,7 @@ static void atm_tc_destroy(struct Qdisc *sch)
 	pr_debug("atm_tc_destroy(sch %p,[qdisc %p])\n", sch, p);
 	/* races ? */
 	while ((flow = p->flows)) {
-		tcf_destroy_chain(flow->filter_list);
-		flow->filter_list = NULL;
+		tcf_destroy_chain(&flow->filter_list);
 		if (flow->ref > 1)
 			printk(KERN_ERR "atm_destroy: %p->ref = %d\n", flow,
 			       flow->ref);
net/sched/sch_cbq.c +3 -5
index 09969c1..2a3c97f 100644
@@ -1704,7 +1704,7 @@ static void cbq_destroy_class(struct Qdisc *sch, struct cbq_class *cl)
 
 	BUG_TRAP(!cl->filters);
 
-	tcf_destroy_chain(cl->filter_list);
+	tcf_destroy_chain(&cl->filter_list);
 	qdisc_destroy(cl->q);
 	qdisc_put_rtab(cl->R_tab);
 	gen_kill_estimator(&cl->bstats, &cl->rate_est);
@@ -1728,10 +1728,8 @@ cbq_destroy(struct Qdisc* sch)
 	 * be bound to classes which have been destroyed already. --TGR '04
 	 */
 	for (h = 0; h < 16; h++) {
-		for (cl = q->classes[h]; cl; cl = cl->next) {
-			tcf_destroy_chain(cl->filter_list);
-			cl->filter_list = NULL;
-		}
+		for (cl = q->classes[h]; cl; cl = cl->next)
+			tcf_destroy_chain(&cl->filter_list);
 	}
 	for (h = 0; h < 16; h++) {
 		struct cbq_class *next;
net/sched/sch_dsmark.c +1 -1
index 64465ba..c4c1317 100644
@@ -416,7 +416,7 @@ static void dsmark_destroy(struct Qdisc *sch)
 
 	pr_debug("dsmark_destroy(sch %p,[qdisc %p])\n", sch, p);
 
-	tcf_destroy_chain(p->filter_list);
+	tcf_destroy_chain(&p->filter_list);
 	qdisc_destroy(p->q);
 	kfree(p->mask);
 }
net/sched/sch_hfsc.c +1 -1
index fdfaa3f..eca83a3 100644
@@ -1123,7 +1123,7 @@ hfsc_destroy_class(struct Qdisc *sch, struct hfsc_class *cl)
 {
 	struct hfsc_sched *q = qdisc_priv(sch);
 
-	tcf_destroy_chain(cl->filter_list);
+	tcf_destroy_chain(&cl->filter_list);
 	qdisc_destroy(cl->qdisc);
 	gen_kill_estimator(&cl->bstats, &cl->rate_est);
 	if (cl != &q->root)
net/sched/sch_htb.c +2 -2
index 2cef8f3..0284791 100644
@@ -1236,7 +1236,7 @@ static void htb_destroy_class(struct Qdisc *sch, struct htb_class *cl)
 	qdisc_put_rtab(cl->rate);
 	qdisc_put_rtab(cl->ceil);
 
-	tcf_destroy_chain(cl->filter_list);
+	tcf_destroy_chain(&cl->filter_list);
 
 	while (!list_empty(&cl->children))
 		htb_destroy_class(sch, list_entry(cl->children.next,
@@ -1265,7 +1265,7 @@ static void htb_destroy(struct Qdisc *sch)
 	   and surprisingly it worked in 2.4. But it must precede it
 	   because filter need its target class alive to be able to call
 	   unbind_filter on it (without Oops). */
-	tcf_destroy_chain(q->filter_list);
+	tcf_destroy_chain(&q->filter_list);
 
 	while (!list_empty(&q->root))
 		htb_destroy_class(sch, list_entry(q->root.next,
net/sched/sch_ingress.c +1 -1
index 274b1dd..956c80a 100644
@@ -104,7 +104,7 @@ static void ingress_destroy(struct Qdisc *sch)
 {
 	struct ingress_qdisc_data *p = qdisc_priv(sch);
 
-	tcf_destroy_chain(p->filter_list);
+	tcf_destroy_chain(&p->filter_list);
 }
 
 static int ingress_dump(struct Qdisc *sch, struct sk_buff *skb)
net/sched/sch_prio.c +1 -1
index 4aa2b45..5532f10 100644
@@ -219,7 +219,7 @@ prio_destroy(struct Qdisc* sch)
 	int prio;
 	struct prio_sched_data *q = qdisc_priv(sch);
 
-	tcf_destroy_chain(q->filter_list);
+	tcf_destroy_chain(&q->filter_list);
 	for (prio=0; prio<q->bands; prio++)
 		qdisc_destroy(q->queues[prio]);
 }
net/sched/sch_sfq.c +1 -1
index f0463d7..6a97afb 100644
@@ -520,7 +520,7 @@ static void sfq_destroy(struct Qdisc *sch)
 {
 	struct sfq_sched_data *q = qdisc_priv(sch);
 
-	tcf_destroy_chain(q->filter_list);
+	tcf_destroy_chain(&q->filter_list);
 	q->perturb_period = 0;
 	del_timer_sync(&q->perturb_timer);
 }
maintainer
#1 lore raw

From: Patrick McHardy <[email protected]>
Date: Tue, 01 Jul 2008 12:06:22 +0200

Quoted 4 lines

These two patches fix a potential use-after-free in hfsc and atm filter destruction. All filters must be destroyed before beginning class destruction since the destination class must still be alive to unbind the filter.

Both patches applied to net-2.6, thanks!