Patch "net: sched: fix refcount imbalance in actions" has been added to the 4.1-stable tree

This is a note to let you know that I've just added the patch titled

net: sched: fix refcount imbalance in actions

to the 4.1-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
net-sched-fix-refcount-imbalance-in-actions.patch
and it can be found in the queue-4.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From: Daniel Borkmann <[email protected]>
Date: Wed, 29 Jul 2015 23:35:25 +0200
Subject: net: sched: fix refcount imbalance in actions

From: Daniel Borkmann <[email protected]>

[ Upstream commit 28e6b67f0b292f557468c139085303b15f1a678f ]

Since commit 55334a5db5cd ("net_sched: act: refuse to remove bound action outside"), we end up with a wrong reference count for a tc action.

Test case 1:

FOO="1,6 0 0 4294967295,"
BAR="1,6 0 0 4294967294,"
tc filter add dev foo parent 1: bpf bytecode "$FOO" flowid 1:1 \
action bpf bytecode "$FOO"
tc actions show action bpf
action order 0: bpf bytecode '1,6 0 0 4294967295' default-action pipe index 1 ref 1 bind 1
tc actions replace action bpf bytecode "$BAR" index 1
tc actions show action bpf
action order 0: bpf bytecode '1,6 0 0 4294967294' default-action pipe index 1 ref 2 bind 1
tc actions replace action bpf bytecode "$FOO" index 1
tc actions show action bpf
action order 0: bpf bytecode '1,6 0 0 4294967295' default-action pipe index 1 ref 3 bind 1

Test case 2:

FOO="1,6 0 0 4294967295,"
tc filter add dev foo parent 1: bpf bytecode "$FOO" flowid 1:1 action ok tc actions show action gact
action order 0: gact action pass
random type none pass val 0
index 1 ref 1 bind 1
tc actions add action drop index 1
RTNETLINK answers: File exists [...]
tc actions show action gact
action order 0: gact action pass
random type none pass val 0
index 1 ref 2 bind 1
tc actions add action drop index 1
RTNETLINK answers: File exists [...]
tc actions show action gact
action order 0: gact action pass
random type none pass val 0
index 1 ref 3 bind 1

What happens is that in tcf_hash_check(), we check tcf_common for a given index and increase tcfc_refcnt and conditionally tcfc_bindcnt when we've found an existing action. Now there are the following cases:

1) We do a late binding of an action. In that case, we leave the tcfc_refcnt/tcfc_bindcnt increased and are done with the ->init() handler. This is correctly handeled.

2) We replace the given action, or we try to add one without replacing and find out that the action at a specific index already exists (thus, we go out with error in that case).

In case of 2), we have to undo the reference count increase from tcf_hash_check() in the tcf_hash_check() function. Currently, we fail to do so because of the 'tcfc_bindcnt > 0' check which bails out early with an -EPERM error.

Now, while commit 55334a5db5cd prevents 'tc actions del action ...' on an already classifier-bound action to drop the reference count (which could then become negative, wrap around etc), this restriction only accounts for invocations outside a specific action's ->init() handler.

One possible solution would be to add a flag thus we possibly trigger the -EPERM ony in situations where it is indeed relevant.

After the patch, above test cases have correct reference count again.

---
 include/net/act_api.h |    8 +++++++-
 net/sched/act_api.c   |   11 ++++++-----
 2 files changed, 13 insertions(+), 6 deletions(-)

@@ -99,7 +99,6 @@ struct tc_action_ops {
 
 int tcf_hash_search(struct tc_action *a, u32 index);
 void tcf_hash_destroy(struct tc_action *a);
-int tcf_hash_release(struct tc_action *a, int bind);
 u32 tcf_hash_new_index(struct tcf_hashinfo *hinfo);
 int tcf_hash_check(u32 index, struct tc_action *a, int bind);
 int tcf_hash_create(u32 index, struct nlattr *est, struct tc_action *a,

@@ -107,6 +106,13 @@ int tcf_hash_create(u32 index, struct nl
 void tcf_hash_cleanup(struct tc_action *a, struct nlattr *est);
 void tcf_hash_insert(struct tc_action *a);
 
+int __tcf_hash_release(struct tc_action *a, bool bind, bool strict);
+
+static inline int tcf_hash_release(struct tc_action *a, bool bind)
+{
+	return __tcf_hash_release(a, bind, false);
+}
+
 int tcf_register_action(struct tc_action_ops *a, unsigned int mask);
 int tcf_unregister_action(struct tc_action_ops *a);
 int tcf_action_destroy(struct list_head *actions, int bind);

@@ -45,7 +45,7 @@ void tcf_hash_destroy(struct tc_action *
 }
 EXPORT_SYMBOL(tcf_hash_destroy);
 
-int tcf_hash_release(struct tc_action *a, int bind)
+int __tcf_hash_release(struct tc_action *a, bool bind, bool strict)
 {
 	struct tcf_common *p = a->priv;
 	int ret = 0;

@@ -53,7 +53,7 @@ int tcf_hash_release(struct tc_action *a
 	if (p) {
 		if (bind)
 			p->tcfc_bindcnt--;
-		else if (p->tcfc_bindcnt > 0)
+		else if (strict && p->tcfc_bindcnt > 0)
 			return -EPERM;
 
 		p->tcfc_refcnt--;

@@ -64,9 +64,10 @@ int tcf_hash_release(struct tc_action *a
 			ret = 1;
 		}
 	}
+
 	return ret;
 }
-EXPORT_SYMBOL(tcf_hash_release);
+EXPORT_SYMBOL(__tcf_hash_release);
 
 static int tcf_dump_walker(struct sk_buff *skb, struct netlink_callback *cb,
 			   struct tc_action *a)

@@ -136,7 +137,7 @@ static int tcf_del_walker(struct sk_buff
 		head = &hinfo->htab[tcf_hash(i, hinfo->hmask)];
 		hlist_for_each_entry_safe(p, n, head, tcfc_head) {
 			a->priv = p;
-			ret = tcf_hash_release(a, 0);
+			ret = __tcf_hash_release(a, false, true);
 			if (ret == ACT_P_DELETED) {
 				module_put(a->ops->owner);
 				n_i++;

@@ -413,7 +414,7 @@ int tcf_action_destroy(struct list_head
 	int ret = 0;
 
 	list_for_each_entry_safe(a, tmp, actions, list) {
-		ret = tcf_hash_release(a, bind);
+		ret = __tcf_hash_release(a, bind, true);
 		if (ret == ACT_P_DELETED)
 			module_put(a->ops->owner);
 		else if (ret < 0)

Patches currently in stable-queue which might be from [email protected] are

queue-4.1/sched-cls_bpf-fix-panic-on-filter-replace.patch queue-4.1/net-sched-fix-refcount-imbalance-in-actions.patch queue-4.1/sched-cls_flow-fix-panic-on-filter-replace.patch queue-4.1/act_bpf-fix-memory-leaks-when-replacing-bpf-programs.patch queue-4.1/packet-tpacket_snd-fix-signed-unsigned-comparison.patch queue-4.1/rtnetlink-verify-ifla_vf_info-attributes-before-passing-them-to-driver.patch queue-4.1/netlink-make-sure-ebusy-won-t-escape-from-netlink_insert.patch