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: <alpine.LNX.2.00.1606281846500.6874@cbobk.fhfr.pm>
Date:	Tue, 28 Jun 2016 18:49:28 +0200 (CEST)
From:	Jiri Kosina <jikos@...nel.org>
To:	Eric Dumazet <eric.dumazet@...il.com>
cc:	Jamal Hadi Salim <jhs@...atatu.com>, Phil Sutter <phil@....cc>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [RFC PATCH] sch_tbf: avoid silent fallback to noop_qdisc (was Re:
 Deleting child qdisc doesn't reset parent to default qdisc?)

On Fri, 15 Apr 2016, Eric Dumazet wrote:

> Then you need to save the initial qdisc (bfifo for TBF) in a special
> place, to make sure the delete operation is guaranteed to succeed.
> 
> Or fail the delete if the bfifo can not be allocated.
> 
> I can tell that determinism if far more interesting than usability for
> some users occasionally playing with tc.
> 
> Surely the silent fallback to noop_qdisc is wrong.

So before we go further and fix the fact that we actually do have hidden 
qdiscs (by refactoring qdisc_match_from_root() and friends), I'd still 
like to bring the patch below up for consideration.

Thanks.




From: Jiri Kosina <jkosina@...e.cz>
Subject: [PATCH] sch_tbf: avoid silent fallback to noop_qdisc

TBF started its life as a classless qdisc with a single builtin FIFO queue
which was being shaped.

When it got later turned into classful qdisc, it was written in a way that
the fallback qdisc was noop_qdisc, which produces bad user experience (delete
of last manually added class doesn't reset it to initial default, but renders
networking unusable instead).

Switch the default fallback to bfifo; this also mimics how the other guys
(HTB, HFSC, CBQ, ...) are behaving.

Signed-off-by: Jiri Kosina <jkosina@...e.cz>
---
 net/sched/sch_tbf.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 3161e49..b06dffe 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -508,8 +508,12 @@ static int tbf_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
 {
 	struct tbf_sched_data *q = qdisc_priv(sch);
 
-	if (new == NULL)
-		new = &noop_qdisc;
+	if (new == NULL) {
+		/* reset to default qdisc */
+		new = qdisc_create_dflt(sch->dev_queue, &bfifo_qdisc_ops, sch->parent);
+		if (!new)
+			return -ENOBUFS;
+	}
 
 	*old = qdisc_replace(sch, new, &q->qdisc);
 	return 0;
-- 
Jiri Kosina
SUSE Labs

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ