net_sched: fix backward compatibility for TCA_KIND
authorCong Wang <xiyou.wangcong@gmail.com>
Mon, 7 Oct 2019 20:26:28 +0000 (13:26 -0700)
committerJakub Kicinski <jakub.kicinski@netronome.com>
Tue, 8 Oct 2019 23:21:58 +0000 (16:21 -0700)
Marcelo noticed a backward compatibility issue of TCA_KIND
after we move from NLA_STRING to NLA_NUL_STRING, so it is probably
too late to change it.

Instead, to make everyone happy, we can just insert a NUL to
terminate the string with nla_strlcpy() like we do for TC actions.

Fixes: 62794fc4fbf5 ("net_sched: add max len check for TCA_KIND")
Reported-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
net/sched/cls_api.c
net/sched/sch_api.c

index 64584a1..8717c0b 100644 (file)
@@ -162,11 +162,22 @@ static inline u32 tcf_auto_prio(struct tcf_proto *tp)
        return TC_H_MAJ(first);
 }
 
+static bool tcf_proto_check_kind(struct nlattr *kind, char *name)
+{
+       if (kind)
+               return nla_strlcpy(name, kind, IFNAMSIZ) >= IFNAMSIZ;
+       memset(name, 0, IFNAMSIZ);
+       return false;
+}
+
 static bool tcf_proto_is_unlocked(const char *kind)
 {
        const struct tcf_proto_ops *ops;
        bool ret;
 
+       if (strlen(kind) == 0)
+               return false;
+
        ops = tcf_proto_lookup_ops(kind, false, NULL);
        /* On error return false to take rtnl lock. Proto lookup/create
         * functions will perform lookup again and properly handle errors.
@@ -1843,6 +1854,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 {
        struct net *net = sock_net(skb->sk);
        struct nlattr *tca[TCA_MAX + 1];
+       char name[IFNAMSIZ];
        struct tcmsg *t;
        u32 protocol;
        u32 prio;
@@ -1899,13 +1911,19 @@ replay:
        if (err)
                return err;
 
+       if (tcf_proto_check_kind(tca[TCA_KIND], name)) {
+               NL_SET_ERR_MSG(extack, "Specified TC filter name too long");
+               err = -EINVAL;
+               goto errout;
+       }
+
        /* Take rtnl mutex if rtnl_held was set to true on previous iteration,
         * block is shared (no qdisc found), qdisc is not unlocked, classifier
         * type is not specified, classifier is not unlocked.
         */
        if (rtnl_held ||
            (q && !(q->ops->cl_ops->flags & QDISC_CLASS_OPS_DOIT_UNLOCKED)) ||
-           !tca[TCA_KIND] || !tcf_proto_is_unlocked(nla_data(tca[TCA_KIND]))) {
+           !tcf_proto_is_unlocked(name)) {
                rtnl_held = true;
                rtnl_lock();
        }
@@ -2063,6 +2081,7 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 {
        struct net *net = sock_net(skb->sk);
        struct nlattr *tca[TCA_MAX + 1];
+       char name[IFNAMSIZ];
        struct tcmsg *t;
        u32 protocol;
        u32 prio;
@@ -2102,13 +2121,18 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
        if (err)
                return err;
 
+       if (tcf_proto_check_kind(tca[TCA_KIND], name)) {
+               NL_SET_ERR_MSG(extack, "Specified TC filter name too long");
+               err = -EINVAL;
+               goto errout;
+       }
        /* Take rtnl mutex if flushing whole chain, block is shared (no qdisc
         * found), qdisc is not unlocked, classifier type is not specified,
         * classifier is not unlocked.
         */
        if (!prio ||
            (q && !(q->ops->cl_ops->flags & QDISC_CLASS_OPS_DOIT_UNLOCKED)) ||
-           !tca[TCA_KIND] || !tcf_proto_is_unlocked(nla_data(tca[TCA_KIND]))) {
+           !tcf_proto_is_unlocked(name)) {
                rtnl_held = true;
                rtnl_lock();
        }
@@ -2216,6 +2240,7 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 {
        struct net *net = sock_net(skb->sk);
        struct nlattr *tca[TCA_MAX + 1];
+       char name[IFNAMSIZ];
        struct tcmsg *t;
        u32 protocol;
        u32 prio;
@@ -2252,12 +2277,17 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
        if (err)
                return err;
 
+       if (tcf_proto_check_kind(tca[TCA_KIND], name)) {
+               NL_SET_ERR_MSG(extack, "Specified TC filter name too long");
+               err = -EINVAL;
+               goto errout;
+       }
        /* Take rtnl mutex if block is shared (no qdisc found), qdisc is not
         * unlocked, classifier type is not specified, classifier is not
         * unlocked.
         */
        if ((q && !(q->ops->cl_ops->flags & QDISC_CLASS_OPS_DOIT_UNLOCKED)) ||
-           !tca[TCA_KIND] || !tcf_proto_is_unlocked(nla_data(tca[TCA_KIND]))) {
+           !tcf_proto_is_unlocked(name)) {
                rtnl_held = true;
                rtnl_lock();
        }
index 81d58b2..1047825 100644 (file)
@@ -1390,8 +1390,7 @@ check_loop_fn(struct Qdisc *q, unsigned long cl, struct qdisc_walker *w)
 }
 
 const struct nla_policy rtm_tca_policy[TCA_MAX + 1] = {
-       [TCA_KIND]              = { .type = NLA_NUL_STRING,
-                                   .len = IFNAMSIZ - 1 },
+       [TCA_KIND]              = { .type = NLA_STRING },
        [TCA_RATE]              = { .type = NLA_BINARY,
                                    .len = sizeof(struct tc_estimator) },
        [TCA_STAB]              = { .type = NLA_NESTED },