[PATCH net] net/sched: cls_api: add missing validation of netlink attributes

Similarly to what has been done in 8b4c3cdd9dd8 ("net: sched: Add policy
validation for tc attributes"), add validation for TCA_CHAIN and TCA_KIND
netlink attributes.

tested with:
 # ./tdc.py -c filter

Fixes: 5bc1701881e39 ("net: sched: introduce multichain support for filters")
Signed-off-by: Davide Caratti <redacted>
---
 net/sched/cls_api.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 0a75cb2e5e7b..fb1afc0e130d 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c

@@ -37,6 +37,11 @@ static LIST_HEAD(tcf_proto_base);
 /* Protects list of registered TC modules. It is pure SMP lock. */
 static DEFINE_RWLOCK(cls_mod_lock);
 
+const struct nla_policy cls_tca_policy[TCA_MAX + 1] = {
+	[TCA_KIND]	= { .type = NLA_STRING },
+	[TCA_CHAIN]	= { .type = NLA_U32 },
+};
+
 /* Find classifier type by string name */
 
 static const struct tcf_proto_ops *__tcf_proto_lookup_ops(const char *kind)

@@ -1211,7 +1216,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 replay:
 	tp_created = 0;
 
-	err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack);
+	err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, cls_tca_policy, extack);
 	if (err < 0)
 		return err;
 

@@ -1360,7 +1365,7 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN))
 		return -EPERM;
 
-	err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack);
+	err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, cls_tca_policy, extack);
 	if (err < 0)
 		return err;
 

@@ -1475,7 +1480,7 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	void *fh = NULL;
 	int err;
 
-	err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack);
+	err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, cls_tca_policy, extack);
 	if (err < 0)
 		return err;
 

@@ -1838,7 +1843,7 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
 		return -EPERM;
 
 replay:
-	err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack);
+	err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, cls_tca_policy, extack);
 	if (err < 0)
 		return err;
 

@@ -1949,7 +1954,8 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
 	if (nlmsg_len(cb->nlh) < sizeof(*tcm))
 		return skb->len;
 
-	err = nlmsg_parse(cb->nlh, sizeof(*tcm), tca, TCA_MAX, NULL, NULL);
+	err = nlmsg_parse(cb->nlh, sizeof(*tcm), tca, TCA_MAX, cls_tca_policy,
+			  NULL);
 	if (err)
 		return err;
 

-- 
2.17.1

On 10/9/18 7:10 AM, Davide Caratti wrote:

That should be static since it can not be used outside this module.

it be nice to have a tc_common module so this stuff does not have to be
defined multiple times.

On Tue, 2018-10-09 at 08:46 -0600, David Ahern wrote:

On 10/9/18 7:10 AM, Davide Caratti wrote:

quoted

Similarly to what has been done in 8b4c3cdd9dd8 ("net: sched: Add policy
validation for tc attributes"), add validation for TCA_CHAIN and TCA_KIND
netlink attributes.

tested with:
 # ./tdc.py -c filter

Fixes: 5bc1701881e39 ("net: sched: introduce multichain support for filters")
Signed-off-by: Davide Caratti <redacted>
---
 net/sched/cls_api.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 0a75cb2e5e7b..fb1afc0e130d 100644

hi David,
thanks for looking at this.

quoted

--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c

@@ -37,6 +37,11 @@ static LIST_HEAD(tcf_proto_base);
 /* Protects list of registered TC modules. It is pure SMP lock. */
 static DEFINE_RWLOCK(cls_mod_lock);
 
+const struct nla_policy cls_tca_policy[TCA_MAX + 1] = {
+	[TCA_KIND]	= { .type = NLA_STRING },
+	[TCA_CHAIN]	= { .type = NLA_U32 },
+};
+

it be nice to have a tc_common module so this stuff does not have to be
defined multiple times.

it makes sense to avoid duplicating the declaration of that array. But I
don't think we can put it in a module, because CONFIG_NET_SCHED is 'bool'
and

obj-$(CONFIG_NET_SCHED)         += sch_api.o

I can try a v2 where 'rtm_tca_policy' symbol in sch_api is exported and
used by cls_api.c code. WDYT?

thanks,
-- 
davide

On 10/9/18 10:12 AM, Davide Caratti wrote:

quoted

quoted

--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c

@@ -37,6 +37,11 @@ static LIST_HEAD(tcf_proto_base);
 /* Protects list of registered TC modules. It is pure SMP lock. */
 static DEFINE_RWLOCK(cls_mod_lock);
 
+const struct nla_policy cls_tca_policy[TCA_MAX + 1] = {
+	[TCA_KIND]	= { .type = NLA_STRING },
+	[TCA_CHAIN]	= { .type = NLA_U32 },
+};
+

quoted

it be nice to have a tc_common module so this stuff does not have to be
defined multiple times.

it makes sense to avoid duplicating the declaration of that array. But I
don't think we can put it in a module, because CONFIG_NET_SCHED is 'bool'
and

obj-$(CONFIG_NET_SCHED)         += sch_api.o

I can try a v2 where 'rtm_tca_policy' symbol in sch_api is exported and
used by cls_api.c code. WDYT?

since NET_SCHED is a bool, that should work.