netdev - [PATCH] net_sched: Fix qdisc

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:	Sun, 23 May 2010 08:37:44 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	hadi@...erus.ca, David Miller <davem@...emloft.net>
Cc:	Patrick McHardy <kaber@...sh.net>, Ben Pfaff <blp@...ira.com>,
	netdev@...r.kernel.org
Subject: [PATCH] net_sched: Fix qdisc_notify()

Le samedi 22 mai 2010 à 08:31 -0400, jamal a écrit :
> On Sat, 2010-05-22 at 11:51 +0200, Patrick McHardy wrote:
> 
> > 
> > We already use TCQ_F_BUILTIN in tc_qdisc_dump_ignore(), so I
> > think it would be more consistent than checking for a handle
> > to use it here as well.
> 
> Agree - it is more semantically correct..
> 
> I wonder though if it is better to do the check in tc_get_qdisc()
> and tc_modify_qdisc()?

Lets check tc_qdisc_dump_ignore() before tc_fill_qdisc(), its more
consistent ;)

David, this is a stable candidate (2.6.26 and up)

Thanks

[PATCH] net_sched: Fix qdisc_notify()

Ben Pfaff reported a kernel oops and provided a test program to
reproduce it.

https://kerneltrap.org/mailarchive/linux-netdev/2010/5/21/6277805

tc_fill_qdisc() should not be called for builtin qdisc, or it
dereference a NULL pointer to get device ifindex.

Fix is to always use tc_qdisc_dump_ignore() before calling
tc_fill_qdisc().

Reported-by: Ben Pfaff <blp@...ira.com>
Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
---
 net/sched/sch_api.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index fe35c1f..b9e8c3b 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1195,6 +1195,11 @@ nla_put_failure:
 	return -1;
 }
 
+static bool tc_qdisc_dump_ignore(struct Qdisc *q)
+{
+	return (q->flags & TCQ_F_BUILTIN) ? true : false;
+}
+
 static int qdisc_notify(struct net *net, struct sk_buff *oskb,
 			struct nlmsghdr *n, u32 clid,
 			struct Qdisc *old, struct Qdisc *new)
@@ -1206,11 +1211,11 @@ static int qdisc_notify(struct net *net, struct sk_buff *oskb,
 	if (!skb)
 		return -ENOBUFS;
 
-	if (old && old->handle) {
+	if (old && !tc_qdisc_dump_ignore(old)) {
 		if (tc_fill_qdisc(skb, old, clid, pid, n->nlmsg_seq, 0, RTM_DELQDISC) < 0)
 			goto err_out;
 	}
-	if (new) {
+	if (new && !tc_qdisc_dump_ignore(new)) {
 		if (tc_fill_qdisc(skb, new, clid, pid, n->nlmsg_seq, old ? NLM_F_REPLACE : 0, RTM_NEWQDISC) < 0)
 			goto err_out;
 	}
@@ -1223,11 +1228,6 @@ err_out:
 	return -EINVAL;
 }
 
-static bool tc_qdisc_dump_ignore(struct Qdisc *q)
-{
-	return (q->flags & TCQ_F_BUILTIN) ? true : false;
-}
-
 static int tc_dump_qdisc_root(struct Qdisc *root, struct sk_buff *skb,
 			      struct netlink_callback *cb,
 			      int *q_idx_p, int s_q_idx)


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