net_sched: fix backward compatibility for TCA_KIND

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: 62794fc4fb ("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>
This commit is contained in:
Cong Wang 2019-10-07 13:26:28 -07:00 committed by Jakub Kicinski
parent 0041412694
commit 6f96c3c690
2 changed files with 34 additions and 5 deletions

View File

@ -162,11 +162,22 @@ static inline u32 tcf_auto_prio(struct tcf_proto *tp)
return TC_H_MAJ(first); 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) static bool tcf_proto_is_unlocked(const char *kind)
{ {
const struct tcf_proto_ops *ops; const struct tcf_proto_ops *ops;
bool ret; bool ret;
if (strlen(kind) == 0)
return false;
ops = tcf_proto_lookup_ops(kind, false, NULL); ops = tcf_proto_lookup_ops(kind, false, NULL);
/* On error return false to take rtnl lock. Proto lookup/create /* On error return false to take rtnl lock. Proto lookup/create
* functions will perform lookup again and properly handle errors. * 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 net *net = sock_net(skb->sk);
struct nlattr *tca[TCA_MAX + 1]; struct nlattr *tca[TCA_MAX + 1];
char name[IFNAMSIZ];
struct tcmsg *t; struct tcmsg *t;
u32 protocol; u32 protocol;
u32 prio; u32 prio;
@ -1899,13 +1911,19 @@ replay:
if (err) if (err)
return 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, /* Take rtnl mutex if rtnl_held was set to true on previous iteration,
* block is shared (no qdisc found), qdisc is not unlocked, classifier * block is shared (no qdisc found), qdisc is not unlocked, classifier
* type is not specified, classifier is not unlocked. * type is not specified, classifier is not unlocked.
*/ */
if (rtnl_held || if (rtnl_held ||
(q && !(q->ops->cl_ops->flags & QDISC_CLASS_OPS_DOIT_UNLOCKED)) || (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_held = true;
rtnl_lock(); 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 net *net = sock_net(skb->sk);
struct nlattr *tca[TCA_MAX + 1]; struct nlattr *tca[TCA_MAX + 1];
char name[IFNAMSIZ];
struct tcmsg *t; struct tcmsg *t;
u32 protocol; u32 protocol;
u32 prio; u32 prio;
@ -2102,13 +2121,18 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
if (err) if (err)
return 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 /* Take rtnl mutex if flushing whole chain, block is shared (no qdisc
* found), qdisc is not unlocked, classifier type is not specified, * found), qdisc is not unlocked, classifier type is not specified,
* classifier is not unlocked. * classifier is not unlocked.
*/ */
if (!prio || if (!prio ||
(q && !(q->ops->cl_ops->flags & QDISC_CLASS_OPS_DOIT_UNLOCKED)) || (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_held = true;
rtnl_lock(); 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 net *net = sock_net(skb->sk);
struct nlattr *tca[TCA_MAX + 1]; struct nlattr *tca[TCA_MAX + 1];
char name[IFNAMSIZ];
struct tcmsg *t; struct tcmsg *t;
u32 protocol; u32 protocol;
u32 prio; u32 prio;
@ -2252,12 +2277,17 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
if (err) if (err)
return 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 /* Take rtnl mutex if block is shared (no qdisc found), qdisc is not
* unlocked, classifier type is not specified, classifier is not * unlocked, classifier type is not specified, classifier is not
* unlocked. * unlocked.
*/ */
if ((q && !(q->ops->cl_ops->flags & QDISC_CLASS_OPS_DOIT_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_held = true;
rtnl_lock(); rtnl_lock();
} }

View 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] = { const struct nla_policy rtm_tca_policy[TCA_MAX + 1] = {
[TCA_KIND] = { .type = NLA_NUL_STRING, [TCA_KIND] = { .type = NLA_STRING },
.len = IFNAMSIZ - 1 },
[TCA_RATE] = { .type = NLA_BINARY, [TCA_RATE] = { .type = NLA_BINARY,
.len = sizeof(struct tc_estimator) }, .len = sizeof(struct tc_estimator) },
[TCA_STAB] = { .type = NLA_NESTED }, [TCA_STAB] = { .type = NLA_NESTED },