netdev - [PATCH][NET_SCHED] sch_api: fix qdisc_tree_decrease

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080414202605.GA6164@ami.dom.local>
Date:	Mon, 14 Apr 2008 22:26:06 +0200
From:	Jarek Poplawski <jarkao2@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	Enrico Demarin <enrico@...erclick.com>, netdev@...r.kernel.org,
	Patrick McHardy <kaber@...sh.net>
Subject: [PATCH][NET_SCHED] sch_api: fix qdisc_tree_decrease_qlen() loop

On Sun, Apr 13, 2008 at 02:10:31PM +0200, Jarek Poplawski wrote:
> Enrico Demarin wrote, On 04/11/2008 08:37 PM:
...
> > I stumbled by chance in a deadlock condition using HTB and the ingress 
> > scheduler at the same time.
...
> I think one of possible reasons could be a qdisc with the same
> handle and parent ids, like ingress, but I can't see how it could
> be referenced by your htb classes yet...

I think I got it! Enrico wrote to me he will not be able to test
before monday, but IMHO this fix is obvious here...

Regards,
Jarek P. 

--------------->

[NET_SCHED] sch_api: fix qdisc_tree_decrease_qlen() loop

TC_H_MAJ(parentid) for root classes is the same as for ingress, and if
ingress qdisc is created qdisc_lookup() returns its pointer (without
ingress NULL is returned). After this all qdisc_lookups give the same,
and we get endless loop. (I don't know how this could hide for so
long - it should trigger with every leaf class deleted if it's qdisc
isn't empty.)

After this fix qdisc_lookup() is omitted both for ingress and root
parents, but looking for root is only wasting a little time here...
Many thanks to Enrico Demarin for finding a test for catching this
bug, which probably bothered quite a lot of admins.
 

Reported-by: Enrico Demarin <enrico@...erclick.com>,
Signed-off-by: Jarek Poplawski <jarkao2@...il.com>
Cc: Patrick McHardy <kaber@...sh.net>

---

 net/sched/sch_api.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 15b91a9..c40773c 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -386,6 +386,9 @@ void qdisc_tree_decrease_qlen(struct Qdisc *sch, unsigned int n)
 	if (n == 0)
 		return;
 	while ((parentid = sch->parent)) {
+		if (TC_H_MAJ(parentid) == TC_H_MAJ(TC_H_INGRESS))
+			return;
+
 		sch = qdisc_lookup(sch->dev, TC_H_MAJ(parentid));
 		if (sch == NULL) {
 			WARN_ON(parentid != TC_H_ROOT);
--
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