diff mbox

net_sched: reset pointers to tcf blocks in classful qdiscs' destructors

Message ID 150280412126.717517.16274927544808177918.stgit@buzz
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Konstantin Khlebnikov Aug. 15, 2017, 1:35 p.m. UTC
Traffic filters could keep direct pointers to classes in classful qdisc,
thus qdisc destruction first removes all filters before freeing classes.
Class destruction methods also tries to free attached filters but now
this isn't safe because tcf_block_put() unlike to tcf_destroy_chain()
cannot be called second time.

This patch set class->block to NULL after first tcf_block_put() and
turn second call into no-op.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Fixes: 6529eaba33f0 ("net: sched: introduce tcf block infractructure")
---
 net/sched/sch_atm.c  |    4 +++-
 net/sched/sch_cbq.c  |    4 +++-
 net/sched/sch_hfsc.c |    4 +++-
 net/sched/sch_htb.c  |    4 +++-
 4 files changed, 12 insertions(+), 4 deletions(-)

Comments

Jiri Pirko Aug. 15, 2017, 2 p.m. UTC | #1
Tue, Aug 15, 2017 at 03:35:21PM CEST, khlebnikov@yandex-team.ru wrote:
>Traffic filters could keep direct pointers to classes in classful qdisc,
>thus qdisc destruction first removes all filters before freeing classes.
>Class destruction methods also tries to free attached filters but now
>this isn't safe because tcf_block_put() unlike to tcf_destroy_chain()
>cannot be called second time.
>
>This patch set class->block to NULL after first tcf_block_put() and
>turn second call into no-op.
>
>Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>Fixes: 6529eaba33f0 ("net: sched: introduce tcf block infractructure")

Acked-by: Jiri Pirko <jiri@mellanox.com>

Thanks!
David Miller Aug. 16, 2017, 12:16 a.m. UTC | #2
From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Date: Tue, 15 Aug 2017 16:35:21 +0300

> Traffic filters could keep direct pointers to classes in classful qdisc,
> thus qdisc destruction first removes all filters before freeing classes.
> Class destruction methods also tries to free attached filters but now
> this isn't safe because tcf_block_put() unlike to tcf_destroy_chain()
> cannot be called second time.
> 
> This patch set class->block to NULL after first tcf_block_put() and
> turn second call into no-op.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Fixes: 6529eaba33f0 ("net: sched: introduce tcf block infractructure")

Applied and queued up for -stable.
diff mbox

Patch

diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
index 572fe2584e48..c403c87aff7a 100644
--- a/net/sched/sch_atm.c
+++ b/net/sched/sch_atm.c
@@ -572,8 +572,10 @@  static void atm_tc_destroy(struct Qdisc *sch)
 	struct atm_flow_data *flow, *tmp;
 
 	pr_debug("atm_tc_destroy(sch %p,[qdisc %p])\n", sch, p);
-	list_for_each_entry(flow, &p->flows, list)
+	list_for_each_entry(flow, &p->flows, list) {
 		tcf_block_put(flow->block);
+		flow->block = NULL;
+	}
 
 	list_for_each_entry_safe(flow, tmp, &p->flows, list) {
 		if (flow->ref > 1)
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 481036f6b54e..780db43300b1 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1431,8 +1431,10 @@  static void cbq_destroy(struct Qdisc *sch)
 	 * be bound to classes which have been destroyed already. --TGR '04
 	 */
 	for (h = 0; h < q->clhash.hashsize; h++) {
-		hlist_for_each_entry(cl, &q->clhash.hash[h], common.hnode)
+		hlist_for_each_entry(cl, &q->clhash.hash[h], common.hnode) {
 			tcf_block_put(cl->block);
+			cl->block = NULL;
+		}
 	}
 	for (h = 0; h < q->clhash.hashsize; h++) {
 		hlist_for_each_entry_safe(cl, next, &q->clhash.hash[h],
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 3ad02bbe6903..fd15200f8627 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1530,8 +1530,10 @@  hfsc_destroy_qdisc(struct Qdisc *sch)
 	unsigned int i;
 
 	for (i = 0; i < q->clhash.hashsize; i++) {
-		hlist_for_each_entry(cl, &q->clhash.hash[i], cl_common.hnode)
+		hlist_for_each_entry(cl, &q->clhash.hash[i], cl_common.hnode) {
 			tcf_block_put(cl->block);
+			cl->block = NULL;
+		}
 	}
 	for (i = 0; i < q->clhash.hashsize; i++) {
 		hlist_for_each_entry_safe(cl, next, &q->clhash.hash[i],
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 203286ab4427..5d65ec5207e9 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1258,8 +1258,10 @@  static void htb_destroy(struct Qdisc *sch)
 	tcf_block_put(q->block);
 
 	for (i = 0; i < q->clhash.hashsize; i++) {
-		hlist_for_each_entry(cl, &q->clhash.hash[i], common.hnode)
+		hlist_for_each_entry(cl, &q->clhash.hash[i], common.hnode) {
 			tcf_block_put(cl->block);
+			cl->block = NULL;
+		}
 	}
 	for (i = 0; i < q->clhash.hashsize; i++) {
 		hlist_for_each_entry_safe(cl, next, &q->clhash.hash[i],