From 052cbda41fdc243a8d40cce7ab3a6327b4b2887e Mon Sep 17 00:00:00 2001 From: WANG Cong Date: Mon, 13 Jul 2015 12:30:07 -0700 Subject: fq_codel: fix a use-after-free Fixes: 25331d6ce42b ("net: sched: implement qstat helper routines") Cc: John Fastabend Signed-off-by: Cong Wang Signed-off-by: Cong Wang Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- net/sched/sch_fq_codel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sched') diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c index d75993f89fac..06e7c845e24d 100644 --- a/net/sched/sch_fq_codel.c +++ b/net/sched/sch_fq_codel.c @@ -155,10 +155,10 @@ static unsigned int fq_codel_drop(struct Qdisc *sch) skb = dequeue_head(flow); len = qdisc_pkt_len(skb); q->backlogs[idx] -= len; - kfree_skb(skb); sch->q.qlen--; qdisc_qstats_drop(sch); qdisc_qstats_backlog_dec(sch, skb); + kfree_skb(skb); flow->dropped++; return idx; } -- cgit v1.2.3 From e8d092aafd9e68c04d7b468e95ff7a617998a796 Mon Sep 17 00:00:00 2001 From: WANG Cong Date: Tue, 14 Jul 2015 11:21:57 -0700 Subject: net_sched: fix a use-after-free in sfq Fixes: 25331d6ce42b ("net: sched: implement qstat helper routines") Cc: John Fastabend Signed-off-by: Cong Wang Signed-off-by: Cong Wang Signed-off-by: David S. Miller --- net/sched/sch_sfq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sched') diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c index 7d1492663360..52f75a5473e1 100644 --- a/net/sched/sch_sfq.c +++ b/net/sched/sch_sfq.c @@ -306,10 +306,10 @@ drop: len = qdisc_pkt_len(skb); slot->backlog -= len; sfq_dec(q, x); - kfree_skb(skb); sch->q.qlen--; qdisc_qstats_drop(sch); qdisc_qstats_backlog_dec(sch, skb); + kfree_skb(skb); return len; } -- cgit v1.2.3 From c0afd9ce4d6a646fb6433536f95a418bb348fab1 Mon Sep 17 00:00:00 2001 From: WANG Cong Date: Tue, 14 Jul 2015 11:21:58 -0700 Subject: fq_codel: fix return value of fq_codel_drop() The ->drop() is supposed to return the number of bytes it dropped, however fq_codel_drop() returns the index of the flow where it drops a packet from. Fix this by introducing a helper to wrap fq_codel_drop(). Cc: Eric Dumazet Signed-off-by: Cong Wang Signed-off-by: Cong Wang Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- net/sched/sch_fq_codel.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'net/sched') diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c index 06e7c845e24d..21ca33c9f036 100644 --- a/net/sched/sch_fq_codel.c +++ b/net/sched/sch_fq_codel.c @@ -163,6 +163,15 @@ static unsigned int fq_codel_drop(struct Qdisc *sch) return idx; } +static unsigned int fq_codel_qdisc_drop(struct Qdisc *sch) +{ + unsigned int prev_backlog; + + prev_backlog = sch->qstats.backlog; + fq_codel_drop(sch); + return prev_backlog - sch->qstats.backlog; +} + static int fq_codel_enqueue(struct sk_buff *skb, struct Qdisc *sch) { struct fq_codel_sched_data *q = qdisc_priv(sch); @@ -604,7 +613,7 @@ static struct Qdisc_ops fq_codel_qdisc_ops __read_mostly = { .enqueue = fq_codel_enqueue, .dequeue = fq_codel_dequeue, .peek = qdisc_peek_dequeued, - .drop = fq_codel_drop, + .drop = fq_codel_qdisc_drop, .init = fq_codel_init, .reset = fq_codel_reset, .destroy = fq_codel_destroy, -- cgit v1.2.3 From ddf06c1e569a64a44c4c750ae45b2604f19e45f0 Mon Sep 17 00:00:00 2001 From: Alexei Starovoitov Date: Tue, 14 Jul 2015 12:15:19 -0700 Subject: tc: act_bpf: fix memory leak prog->bpf_ops is populated when act_bpf is used with classic BPF and prog->bpf_name is optionally used with extended BPF. Fix memory leak when act_bpf is released. Fixes: d23b8ad8ab23 ("tc: add BPF based action") Fixes: a8cb5f556b56 ("act_bpf: add initial eBPF support for actions") Acked-by: Daniel Borkmann Signed-off-by: Alexei Starovoitov Signed-off-by: David S. Miller --- net/sched/act_bpf.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'net/sched') diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c index 1d56903fd4c7..1df78289e248 100644 --- a/net/sched/act_bpf.c +++ b/net/sched/act_bpf.c @@ -339,6 +339,9 @@ static void tcf_bpf_cleanup(struct tc_action *act, int bind) bpf_prog_put(prog->filter); else bpf_prog_destroy(prog->filter); + + kfree(prog->bpf_ops); + kfree(prog->bpf_name); } static struct tc_action_ops act_bpf_ops __read_mostly = { -- cgit v1.2.3 From f6bfc46da6292b630ba389592123f0dd02066172 Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Fri, 17 Jul 2015 22:38:43 +0200 Subject: sched: cls_bpf: fix panic on filter replace The following test case causes a NULL pointer dereference in cls_bpf: FOO="1,6 0 0 4294967295," tc filter add dev foo parent 1: bpf bytecode "$FOO" flowid 1:1 action ok tc filter replace dev foo parent 1: pref 49152 handle 0x1 \ bpf bytecode "$FOO" flowid 1:1 action drop The problem is that commit 1f947bf151e9 ("net: sched: rcu'ify cls_bpf") accidentally swapped the arguments of list_replace_rcu(), the old element needs to be the first argument and the new element the second. Fixes: 1f947bf151e9 ("net: sched: rcu'ify cls_bpf") Signed-off-by: Daniel Borkmann Acked-by: John Fastabend Acked-by: Alexei Starovoitov Signed-off-by: David S. Miller --- net/sched/cls_bpf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sched') diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c index c79ecfd36e0f..e5168f8b9640 100644 --- a/net/sched/cls_bpf.c +++ b/net/sched/cls_bpf.c @@ -378,7 +378,7 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb, goto errout; if (oldprog) { - list_replace_rcu(&prog->link, &oldprog->link); + list_replace_rcu(&oldprog->link, &prog->link); tcf_unbind_filter(tp, &oldprog->res); call_rcu(&oldprog->rcu, __cls_bpf_delete_prog); } else { -- cgit v1.2.3 From ff3532f2655b79058fec035ca54fced69a483084 Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Fri, 17 Jul 2015 22:38:44 +0200 Subject: sched: cls_flower: fix panic on filter replace The following test case causes a NULL pointer dereference in cls_flower: tc filter add dev foo parent 1: flower eth_type ipv4 action ok flowid 1:1 tc filter replace dev foo parent 1: pref 49152 handle 0x1 \ flower eth_type ipv6 action ok flowid 1:1 The problem is that commit 77b9900ef53a ("tc: introduce Flower classifier") accidentally swapped the arguments of list_replace_rcu(), the old element needs to be the first argument and the new element the second. Fixes: 77b9900ef53a ("tc: introduce Flower classifier") Signed-off-by: Daniel Borkmann Acked-by: Jiri Pirko Signed-off-by: David S. Miller --- net/sched/cls_flower.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/sched') diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index 9d37ccd95062..2f3d03f99487 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -499,7 +499,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb, *arg = (unsigned long) fnew; if (fold) { - list_replace_rcu(&fnew->list, &fold->list); + list_replace_rcu(&fold->list, &fnew->list); tcf_unbind_filter(tp, &fold->res); call_rcu(&fold->rcu, fl_destroy_filter); } else { -- cgit v1.2.3 From 32b2f4b196b37695fdb42b31afcbc15399d6ef91 Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Fri, 17 Jul 2015 22:38:45 +0200 Subject: sched: cls_flow: fix panic on filter replace The following test case causes a NULL pointer dereference in cls_flow: tc filter add dev foo parent 1: handle 0x1 flow hash keys dst action ok tc filter replace dev foo parent 1: pref 49152 handle 0x1 \ flow hash keys mark action drop To be more precise, actually two different panics are fixed, the first occurs because tcf_exts_init() is not called on the newly allocated filter when we do a replace. And the second panic uncovered after that happens since the arguments of list_replace_rcu() are swapped, the old element needs to be the first argument and the new element the second. Fixes: 70da9f0bf999 ("net: sched: cls_flow use RCU") Signed-off-by: Daniel Borkmann Acked-by: John Fastabend Signed-off-by: David S. Miller --- net/sched/cls_flow.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'net/sched') diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c index 76bc3a20ffdb..bb2a0f529c1f 100644 --- a/net/sched/cls_flow.c +++ b/net/sched/cls_flow.c @@ -425,6 +425,8 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, if (!fnew) goto err2; + tcf_exts_init(&fnew->exts, TCA_FLOW_ACT, TCA_FLOW_POLICE); + fold = (struct flow_filter *)*arg; if (fold) { err = -EINVAL; @@ -486,7 +488,6 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, fnew->mask = ~0U; fnew->tp = tp; get_random_bytes(&fnew->hashrnd, 4); - tcf_exts_init(&fnew->exts, TCA_FLOW_ACT, TCA_FLOW_POLICE); } fnew->perturb_timer.function = flow_perturbation; @@ -526,7 +527,7 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, if (*arg == 0) list_add_tail_rcu(&fnew->list, &head->filters); else - list_replace_rcu(&fnew->list, &fold->list); + list_replace_rcu(&fold->list, &fnew->list); *arg = (unsigned long)fnew; -- cgit v1.2.3