* [Patch net] net_sched: fix backward compatibility for TCA_KIND
@ 2019-10-07 20:26 Cong Wang
2019-10-08 12:12 ` Marcelo Ricardo Leitner
2019-10-08 23:40 ` Jakub Kicinski
0 siblings, 2 replies; 3+ messages in thread
From: Cong Wang @ 2019-10-07 20:26 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Marcelo Ricardo Leitner, Jamal Hadi Salim, Jiri Pirko
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>
---
net/sched/cls_api.c | 36 +++++++++++++++++++++++++++++++++---
net/sched/sch_api.c | 3 +--
2 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 64584a1df425..8717c0b26c90 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -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 @@ static int tc_new_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 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();
}
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 81d58b280612..1047825d9f48 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -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 },
--
2.21.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Patch net] net_sched: fix backward compatibility for TCA_KIND
2019-10-07 20:26 [Patch net] net_sched: fix backward compatibility for TCA_KIND Cong Wang
@ 2019-10-08 12:12 ` Marcelo Ricardo Leitner
2019-10-08 23:40 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-10-08 12:12 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Jamal Hadi Salim, Jiri Pirko
On Mon, Oct 07, 2019 at 01:26:28PM -0700, Cong Wang wrote:
> 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>
Reviewed-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>
> ---
> net/sched/cls_api.c | 36 +++++++++++++++++++++++++++++++++---
> net/sched/sch_api.c | 3 +--
> 2 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 64584a1df425..8717c0b26c90 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -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 @@ static int tc_new_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 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();
> }
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index 81d58b280612..1047825d9f48 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -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 },
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch net] net_sched: fix backward compatibility for TCA_KIND
2019-10-07 20:26 [Patch net] net_sched: fix backward compatibility for TCA_KIND Cong Wang
2019-10-08 12:12 ` Marcelo Ricardo Leitner
@ 2019-10-08 23:40 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2019-10-08 23:40 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Marcelo Ricardo Leitner, Jamal Hadi Salim, Jiri Pirko
On Mon, 7 Oct 2019 13:26:28 -0700, Cong Wang wrote:
> 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>
Applied, queued for 4.14+, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-08 23:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-07 20:26 [Patch net] net_sched: fix backward compatibility for TCA_KIND Cong Wang
2019-10-08 12:12 ` Marcelo Ricardo Leitner
2019-10-08 23:40 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox