* [PATCH net-next] netlink: add missing length check of rtnl msg
@ 2013-03-25 17:22 Hong Zhiguo
2013-03-25 17:54 ` David Miller
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Hong Zhiguo @ 2013-03-25 17:22 UTC (permalink / raw)
To: netdev; +Cc: davem, stephen, tgraf, Hong Zhiguo
When the legacy array rtm_min still exists, the length check within these
functions is covered by rtm_min[RTM_NEWTFILTER], rtm_min[RTM_NEWQDISC]
and rtm_min[RTM_NEWTCLASS].
But after Thomas Graf removed rtm_min several days ago, these checks are
missing.
Other doit functions should be OK.
Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
---
net/sched/cls_api.c | 4 ++++
net/sched/sch_api.c | 22 ++++++++++++++++++----
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 9a04b98..01bfa9c 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -141,6 +141,10 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
if ((n->nlmsg_type != RTM_GETTFILTER) && !capable(CAP_NET_ADMIN))
return -EPERM;
+
+ if (nlmsg_len(n) < sizeof(*t))
+ return -EINVAL;
+
replay:
t = nlmsg_data(n);
protocol = TC_H_MIN(t->tcm_info);
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 0bbce22..b30a988 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -977,7 +977,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n)
struct tcmsg *tcm = nlmsg_data(n);
struct nlattr *tca[TCA_MAX + 1];
struct net_device *dev;
- u32 clid = tcm->tcm_parent;
+ u32 clid;
struct Qdisc *q = NULL;
struct Qdisc *p = NULL;
int err;
@@ -985,6 +985,9 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n)
if ((n->nlmsg_type != RTM_GETQDISC) && !capable(CAP_NET_ADMIN))
return -EPERM;
+ if (nlmsg_len(n) < sizeof(*tcm))
+ return -EINVAL;
+
dev = __dev_get_by_index(net, tcm->tcm_ifindex);
if (!dev)
return -ENODEV;
@@ -993,6 +996,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n)
if (err < 0)
return err;
+ clid = tcm->tcm_parent;
if (clid) {
if (clid != TC_H_ROOT) {
if (TC_H_MAJ(clid) != TC_H_MAJ(TC_H_INGRESS)) {
@@ -1051,6 +1055,9 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n)
if (!capable(CAP_NET_ADMIN))
return -EPERM;
+ if (nlmsg_len(n) < sizeof(*tcm))
+ return -EINVAL;
+
replay:
/* Reinit, just in case something touches this. */
tcm = nlmsg_data(n);
@@ -1382,14 +1389,17 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n)
const struct Qdisc_class_ops *cops;
unsigned long cl = 0;
unsigned long new_cl;
- u32 portid = tcm->tcm_parent;
- u32 clid = tcm->tcm_handle;
- u32 qid = TC_H_MAJ(clid);
+ u32 portid;
+ u32 clid;
+ u32 qid;
int err;
if ((n->nlmsg_type != RTM_GETTCLASS) && !capable(CAP_NET_ADMIN))
return -EPERM;
+ if (nlmsg_len(n) < sizeof(*tcm))
+ return -EINVAL;
+
dev = __dev_get_by_index(net, tcm->tcm_ifindex);
if (!dev)
return -ENODEV;
@@ -1413,6 +1423,10 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n)
/* Step 1. Determine qdisc handle X:0 */
+ portid = tcm->tcm_parent;
+ clid = tcm->tcm_handle;
+ qid = TC_H_MAJ(clid);
+
if (portid != TC_H_ROOT) {
u32 qid1 = TC_H_MAJ(portid);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] netlink: add missing length check of rtnl msg
2013-03-25 17:22 [PATCH net-next] netlink: add missing length check of rtnl msg Hong Zhiguo
@ 2013-03-25 17:54 ` David Miller
2013-03-25 23:12 ` Thomas Graf
2013-03-26 3:36 ` [PATCH v2 net-next] netlink: have length check of rtnl msg before deref Hong Zhiguo
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-03-25 17:54 UTC (permalink / raw)
To: honkiko; +Cc: netdev, stephen, tgraf
From: Hong Zhiguo <honkiko@gmail.com>
Date: Tue, 26 Mar 2013 01:22:23 +0800
> When the legacy array rtm_min still exists, the length check within these
> functions is covered by rtm_min[RTM_NEWTFILTER], rtm_min[RTM_NEWQDISC]
> and rtm_min[RTM_NEWTCLASS].
>
> But after Thomas Graf removed rtm_min several days ago, these checks are
> missing.
>
> Other doit functions should be OK.
>
> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
This looks good, Thomas please review.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] netlink: add missing length check of rtnl msg
2013-03-25 17:22 [PATCH net-next] netlink: add missing length check of rtnl msg Hong Zhiguo
2013-03-25 17:54 ` David Miller
@ 2013-03-25 23:12 ` Thomas Graf
2013-03-26 3:36 ` [PATCH v2 net-next] netlink: have length check of rtnl msg before deref Hong Zhiguo
2 siblings, 0 replies; 6+ messages in thread
From: Thomas Graf @ 2013-03-25 23:12 UTC (permalink / raw)
To: Hong Zhiguo; +Cc: netdev, davem, stephen
On 03/26/13 at 01:22am, Hong Zhiguo wrote:
> protocol = TC_H_MIN(t->tcm_info);
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index 0bbce22..b30a988 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -977,7 +977,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n)
> struct tcmsg *tcm = nlmsg_data(n);
> struct nlattr *tca[TCA_MAX + 1];
> struct net_device *dev;
> - u32 clid = tcm->tcm_parent;
> + u32 clid;
> struct Qdisc *q = NULL;
> struct Qdisc *p = NULL;
> int err;
> @@ -985,6 +985,9 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n)
> if ((n->nlmsg_type != RTM_GETQDISC) && !capable(CAP_NET_ADMIN))
> return -EPERM;
>
> + if (nlmsg_len(n) < sizeof(*tcm))
> + return -EINVAL;
> +
> dev = __dev_get_by_index(net, tcm->tcm_ifindex);
> if (!dev)
> return -ENODEV;
What do you think about moving the nlmsg_parse() call from below here
to before __dev_get_by_index()? It also verifies the header length and
it's how other netlink code verifies the message type specific header
as well.
> if (err < 0)
> @@ -1051,6 +1055,9 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n)
> if (!capable(CAP_NET_ADMIN))
> return -EPERM;
>
> + if (nlmsg_len(n) < sizeof(*tcm))
> + return -EINVAL;
> +
Same here.
> replay:
> /* Reinit, just in case something touches this. */
> tcm = nlmsg_data(n);
> @@ -1382,14 +1389,17 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n)
> const struct Qdisc_class_ops *cops;
> unsigned long cl = 0;
> unsigned long new_cl;
> - u32 portid = tcm->tcm_parent;
> - u32 clid = tcm->tcm_handle;
> - u32 qid = TC_H_MAJ(clid);
> + u32 portid;
> + u32 clid;
> + u32 qid;
> int err;
>
> if ((n->nlmsg_type != RTM_GETTCLASS) && !capable(CAP_NET_ADMIN))
> return -EPERM;
>
> + if (nlmsg_len(n) < sizeof(*tcm))
> + return -EINVAL;
> +
> dev = __dev_get_by_index(net, tcm->tcm_ifindex);
> if (!dev)
> return -ENODEV;
And here.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 net-next] netlink: have length check of rtnl msg before deref
2013-03-25 17:22 [PATCH net-next] netlink: add missing length check of rtnl msg Hong Zhiguo
2013-03-25 17:54 ` David Miller
2013-03-25 23:12 ` Thomas Graf
@ 2013-03-26 3:36 ` Hong Zhiguo
2013-03-26 8:59 ` Thomas Graf
2 siblings, 1 reply; 6+ messages in thread
From: Hong Zhiguo @ 2013-03-26 3:36 UTC (permalink / raw)
To: netdev; +Cc: davem, stephen, tgraf
When the legacy array rtm_min still exists, the length check within
these functions is covered by rtm_min[RTM_NEWTFILTER],
rtm_min[RTM_NEWQDISC] and rtm_min[RTM_NEWTCLASS].
But after Thomas Graf removed rtm_min several days ago, these checks
are missing. Other doit functions should be OK.
Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
---
net/sched/cls_api.c | 9 +++++----
net/sched/sch_api.c | 36 +++++++++++++++++++++---------------
2 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 9a04b98..9d71d4d 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -141,7 +141,12 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
if ((n->nlmsg_type != RTM_GETTFILTER) && !capable(CAP_NET_ADMIN))
return -EPERM;
+
replay:
+ err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL);
+ if (err < 0)
+ return err;
+
t = nlmsg_data(n);
protocol = TC_H_MIN(t->tcm_info);
prio = TC_H_MAJ(t->tcm_info);
@@ -164,10 +169,6 @@ replay:
if (dev == NULL)
return -ENODEV;
- err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL);
- if (err < 0)
- return err;
-
/* Find qdisc */
if (!parent) {
q = dev->qdisc;
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 0bbce22..d7468ba 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -977,7 +977,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n)
struct tcmsg *tcm = nlmsg_data(n);
struct nlattr *tca[TCA_MAX + 1];
struct net_device *dev;
- u32 clid = tcm->tcm_parent;
+ u32 clid;
struct Qdisc *q = NULL;
struct Qdisc *p = NULL;
int err;
@@ -985,14 +985,15 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n)
if ((n->nlmsg_type != RTM_GETQDISC) && !capable(CAP_NET_ADMIN))
return -EPERM;
- dev = __dev_get_by_index(net, tcm->tcm_ifindex);
- if (!dev)
- return -ENODEV;
-
err = nlmsg_parse(n, sizeof(*tcm), tca, TCA_MAX, NULL);
if (err < 0)
return err;
+ dev = __dev_get_by_index(net, tcm->tcm_ifindex);
+ if (!dev)
+ return -ENODEV;
+
+ clid = tcm->tcm_parent;
if (clid) {
if (clid != TC_H_ROOT) {
if (TC_H_MAJ(clid) != TC_H_MAJ(TC_H_INGRESS)) {
@@ -1053,6 +1054,10 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n)
replay:
/* Reinit, just in case something touches this. */
+ err = nlmsg_parse(n, sizeof(*tcm), tca, TCA_MAX, NULL);
+ if (err < 0)
+ return err;
+
tcm = nlmsg_data(n);
clid = tcm->tcm_parent;
q = p = NULL;
@@ -1061,9 +1066,6 @@ replay:
if (!dev)
return -ENODEV;
- err = nlmsg_parse(n, sizeof(*tcm), tca, TCA_MAX, NULL);
- if (err < 0)
- return err;
if (clid) {
if (clid != TC_H_ROOT) {
@@ -1382,22 +1384,22 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n)
const struct Qdisc_class_ops *cops;
unsigned long cl = 0;
unsigned long new_cl;
- u32 portid = tcm->tcm_parent;
- u32 clid = tcm->tcm_handle;
- u32 qid = TC_H_MAJ(clid);
+ u32 portid;
+ u32 clid;
+ u32 qid;
int err;
if ((n->nlmsg_type != RTM_GETTCLASS) && !capable(CAP_NET_ADMIN))
return -EPERM;
- dev = __dev_get_by_index(net, tcm->tcm_ifindex);
- if (!dev)
- return -ENODEV;
-
err = nlmsg_parse(n, sizeof(*tcm), tca, TCA_MAX, NULL);
if (err < 0)
return err;
+ dev = __dev_get_by_index(net, tcm->tcm_ifindex);
+ if (!dev)
+ return -ENODEV;
+
/*
parent == TC_H_UNSPEC - unspecified parent.
parent == TC_H_ROOT - class is root, which has no parent.
@@ -1413,6 +1415,10 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n)
/* Step 1. Determine qdisc handle X:0 */
+ portid = tcm->tcm_parent;
+ clid = tcm->tcm_handle;
+ qid = TC_H_MAJ(clid);
+
if (portid != TC_H_ROOT) {
u32 qid1 = TC_H_MAJ(portid);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 net-next] netlink: have length check of rtnl msg before deref
2013-03-26 3:36 ` [PATCH v2 net-next] netlink: have length check of rtnl msg before deref Hong Zhiguo
@ 2013-03-26 8:59 ` Thomas Graf
2013-03-26 16:36 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Graf @ 2013-03-26 8:59 UTC (permalink / raw)
To: Hong Zhiguo; +Cc: netdev, davem, stephen
On 03/26/13 at 11:36am, Hong Zhiguo wrote:
> When the legacy array rtm_min still exists, the length check within
> these functions is covered by rtm_min[RTM_NEWTFILTER],
> rtm_min[RTM_NEWQDISC] and rtm_min[RTM_NEWTCLASS].
>
> But after Thomas Graf removed rtm_min several days ago, these checks
> are missing. Other doit functions should be OK.
>
> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
Acked-by: Thomas Graf <tgraf@suug.ch>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 net-next] netlink: have length check of rtnl msg before deref
2013-03-26 8:59 ` Thomas Graf
@ 2013-03-26 16:36 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-03-26 16:36 UTC (permalink / raw)
To: tgraf; +Cc: honkiko, netdev, stephen
From: Thomas Graf <tgraf@suug.ch>
Date: Tue, 26 Mar 2013 08:59:37 +0000
> On 03/26/13 at 11:36am, Hong Zhiguo wrote:
>> When the legacy array rtm_min still exists, the length check within
>> these functions is covered by rtm_min[RTM_NEWTFILTER],
>> rtm_min[RTM_NEWQDISC] and rtm_min[RTM_NEWTCLASS].
>>
>> But after Thomas Graf removed rtm_min several days ago, these checks
>> are missing. Other doit functions should be OK.
>>
>> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>
>
> Acked-by: Thomas Graf <tgraf@suug.ch>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-03-26 16:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-25 17:22 [PATCH net-next] netlink: add missing length check of rtnl msg Hong Zhiguo
2013-03-25 17:54 ` David Miller
2013-03-25 23:12 ` Thomas Graf
2013-03-26 3:36 ` [PATCH v2 net-next] netlink: have length check of rtnl msg before deref Hong Zhiguo
2013-03-26 8:59 ` Thomas Graf
2013-03-26 16:36 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).