netdev - [PATCH][NET_SCHED] sch_tree_lock in cbq_put, hfsc_put_class and htb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080203145234.GA3170@ami.dom.local>
Date:	Sun, 3 Feb 2008 15:52:34 +0100
From:	Jarek Poplawski <jarkao2@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	Stephen Hemminger <shemminger@...ux-foundation.org>,
	jamal <hadi@...erus.ca>, Patrick McHardy <kaber@...sh.net>,
	netdev@...r.kernel.org
Subject: [PATCH][NET_SCHED] sch_tree_lock in cbq_put, hfsc_put_class and
	htb_put

[NET_SCHED] sch_tree_lock in cbq_put, hfsc_put_class and htb_put

Qdisc_class_ops ->put() "methods" call xxx_destroy_class() functions
without sch_tree_lock(), which is needed at least for qdisc_destroy()
of a subqueue, but also for deactivating and list/rb_tree updates in
case of HTB. (Since errors caused by such a bug could be very hard to
reproduce the effectiveness of this patch wasn't confirmed by tests,
but IMHO the need of this lock is quite obvious here.)

Signed-off-by: Jarek Poplawski <jarkao2@...il.com>
 
---

 net/sched/sch_cbq.c  |   14 ++++++++------
 net/sched/sch_hfsc.c |    5 ++++-
 net/sched/sch_htb.c  |    5 ++++-
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 09969c1..bbf47fe 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1748,16 +1748,18 @@ static void cbq_put(struct Qdisc *sch, unsigned long arg)
 	struct cbq_class *cl = (struct cbq_class*)arg;
 
 	if (--cl->refcnt == 0) {
+		sch_tree_lock(sch);
 #ifdef CONFIG_NET_CLS_ACT
-		struct cbq_sched_data *q = qdisc_priv(sch);
+		{
+			struct cbq_sched_data *q = qdisc_priv(sch);
 
-		spin_lock_bh(&sch->dev->queue_lock);
-		if (q->rx_class == cl)
-			q->rx_class = NULL;
-		spin_unlock_bh(&sch->dev->queue_lock);
+			if (q->rx_class == cl)
+				q->rx_class = NULL;
+		}
 #endif
 
 		cbq_destroy_class(sch, cl);
+		sch_tree_unlock(sch);
 	}
 }
 
@@ -1965,10 +1967,10 @@ static int cbq_delete(struct Qdisc *sch, unsigned long arg)
 	cbq_sync_defmap(cl);
 
 	cbq_rmprio(q, cl);
-	sch_tree_unlock(sch);
 
 	if (--cl->refcnt == 0)
 		cbq_destroy_class(sch, cl);
+	sch_tree_unlock(sch);
 
 	return 0;
 }
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 87293d0..494d042 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1262,8 +1262,11 @@ hfsc_put_class(struct Qdisc *sch, unsigned long arg)
 {
 	struct hfsc_class *cl = (struct hfsc_class *)arg;
 
-	if (--cl->refcnt == 0)
+	if (--cl->refcnt == 0) {
+		sch_tree_lock(sch);
 		hfsc_destroy_class(sch, cl);
+		sch_tree_unlock(sch);
+	}
 }
 
 static unsigned long
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index e1a579e..268a2d1 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1305,8 +1305,11 @@ static void htb_put(struct Qdisc *sch, unsigned long arg)
 {
 	struct htb_class *cl = (struct htb_class *)arg;
 
-	if (--cl->refcnt == 0)
+	if (--cl->refcnt == 0) {
+		sch_tree_lock(sch);
 		htb_destroy_class(sch, cl);
+		sch_tree_unlock(sch);
+	}
 }
 
 static int htb_change_class(struct Qdisc *sch, u32 classid,
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html