netdev - Re: size overflow in function qdisc_tree_decrease_qlen net/sched/sch

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]

Date:	Tue, 01 Dec 2015 08:34:20 -0800
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	pageexec@...email.hu
Cc:	Daniele Fucini <dfucini@...il.com>,
	Cong Wang <cwang@...pensource.com>,
	netdev <netdev@...r.kernel.org>,
	Jamal Hadi Salim <jhs@...atatu.com>,
	David Miller <davem@...emloft.net>, spender@...ecurity.net,
	re.emese@...il.com
Subject: Re: size overflow in function qdisc_tree_decrease_qlen
 net/sched/sch_api.c

On Tue, 2015-12-01 at 17:13 +0100, PaX Team wrote:
> On 1 Dec 2015 at 6:10, Eric Dumazet wrote:
> 
> > On Tue, 2015-12-01 at 06:06 -0800, Eric Dumazet wrote:
> > > On Tue, 2015-12-01 at 12:19 +0100, Daniele Fucini wrote:
> > > > Thanks for the reply. Here's the output of `tc qdisc show`:
> > > > https://gist.github.com/1847102c8fe08f63e9e7
> > 
> > > Hmm... I do not think we ever took care of MQ in
> > > qdisc_tree_decrease_qlen()
> > 
> > This looks like a false positive, because MQ recomputes backlog/qlen at
> > the time (stats) dumps are requested.
> > 
> > I would say there is no bug.
> 
> is it correct for sk_buff_head.qlen to underflow in general
> or just in this particular sched code? (we can exclude overflow
> checking for either case but obviously would like to retain as
> much coverage as possible)

We do not particularly care on the qlen value for MQ, although we simply
should avoid doing anything.

Otherwise we might report slightly wrong qlen values while doing "tc -s
qdisc show" if the qdisc_tree_decrease_qlen() happens right after we
computed the folded values in mq_dump()

Please try the patch I sent. I am pretty sure it should be the right
fix.

I will submit it formally once you tell me it is fixing the issue you
reported.


diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index f43c8f33f09e..72f2c1dfdcde 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -759,6 +759,8 @@ void qdisc_tree_decrease_qlen(struct Qdisc *sch, unsigned int n)
 			WARN_ON(parentid != TC_H_ROOT);
 			return;
 		}
+		if (sch->flags & TCQ_F_MQROOT)
+			return;
 		cops = sch->ops->cl_ops;
 		if (cops->qlen_notify) {
 			cl = cops->get(sch, parentid);


--
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