All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 00/13] net: use strict checks in doit handlers
@ 2019-01-17 22:52 Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 01/13] net: netlink: add helper to retrieve NETLINK_F_STRICT_CHK Jakub Kicinski
                   ` (13 more replies)
  0 siblings, 14 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Hi!

This series extends strict argument checking to doit handlers
of the GET* nature.  This is a bit tricky since strict checking
flag has already been released..

iproute2 did not have a release with strick checks enabled,
and it will only need a minor one-liner to pass strick checks
after all the work that DaveA has already done.

Big thanks to Dave Ahern for help and guidence.
DaveA, does this look good to you?

Jakub Kicinski (13):
  net: netlink: add helper to retrieve NETLINK_F_STRICT_CHK
  rtnetlink: stats: validate attributes in get as well as dumps
  rtnetlink: stats: reject requests for unknown stats
  rtnetlink: ifinfo: perform strict checks also for doit handler
  net: ipv4: perform strict checks also for doit handlers
  net: namespace: perform strict checks also for doit handlers
  net: ipv4: ipmr: perform strict checks also for doit handlers
  net: ipv6: addr: perform strict checks also for doit handlers
  net: ipv6: netconf: perform strict checks also for doit handlers
  net: ipv6: addrlabel: perform strict checks also for doit handlers
  net: ipv6: route: perform strict checks also for doit handlers
  net: mpls: route: perform strict checks also for doit handlers
  net: mpls: netconf: perform strict checks also for doit handlers

 include/linux/netlink.h  |   1 +
 net/core/net_namespace.c |  43 ++++++++++++++-
 net/core/rtnetlink.c     | 111 +++++++++++++++++++++++++++++++--------
 net/ipv4/devinet.c       |  43 +++++++++++++--
 net/ipv4/ipmr.c          |  61 +++++++++++++++++++--
 net/ipv6/addrconf.c      |  90 +++++++++++++++++++++++++++++--
 net/ipv6/addrlabel.c     |  47 ++++++++++++++++-
 net/ipv6/route.c         |  70 +++++++++++++++++++++++-
 net/mpls/af_mpls.c       | 103 ++++++++++++++++++++++++++++++++++--
 net/netlink/af_netlink.c |   8 +++
 10 files changed, 530 insertions(+), 47 deletions(-)

-- 
2.19.2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH net-next 01/13] net: netlink: add helper to retrieve NETLINK_F_STRICT_CHK
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 02/13] rtnetlink: stats: validate attributes in get as well as dumps Jakub Kicinski
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Dumps can read state of the NETLINK_F_STRICT_CHK flag from
a field in the callback structure.  For non-dump GET requests
we need a way to access the state of that flag from a socket.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/linux/netlink.h  | 1 +
 net/netlink/af_netlink.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 4e8add270200..593d1b9c33a8 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -126,6 +126,7 @@ void __netlink_clear_multicast_users(struct sock *sk, unsigned int group);
 void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
 		 const struct netlink_ext_ack *extack);
 int netlink_has_listeners(struct sock *sk, unsigned int group);
+bool netlink_strict_get_check(struct sk_buff *skb);
 
 int netlink_unicast(struct sock *ssk, struct sk_buff *skb, __u32 portid, int nonblock);
 int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, __u32 portid,
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 3c023d6120f6..8fa35df94c07 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1371,6 +1371,14 @@ int netlink_has_listeners(struct sock *sk, unsigned int group)
 }
 EXPORT_SYMBOL_GPL(netlink_has_listeners);
 
+bool netlink_strict_get_check(struct sk_buff *skb)
+{
+	const struct netlink_sock *nlk = nlk_sk(NETLINK_CB(skb).sk);
+
+	return nlk->flags & NETLINK_F_STRICT_CHK;
+}
+EXPORT_SYMBOL_GPL(netlink_strict_get_check);
+
 static int netlink_broadcast_deliver(struct sock *sk, struct sk_buff *skb)
 {
 	struct netlink_sock *nlk = nlk_sk(sk);
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 02/13] rtnetlink: stats: validate attributes in get as well as dumps
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 01/13] net: netlink: add helper to retrieve NETLINK_F_STRICT_CHK Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 03/13] rtnetlink: stats: reject requests for unknown stats Jakub Kicinski
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Make sure NETLINK_GET_STRICT_CHK influences both GETSTATS doit
as well as the dump.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/core/rtnetlink.c | 58 ++++++++++++++++++++++++++++----------------
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5ea1bed08ede..08f142b59403 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4901,6 +4901,36 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
 	return size;
 }
 
+static int rtnl_valid_stats_req(const struct nlmsghdr *nlh, bool strict_check,
+				bool is_dump, struct netlink_ext_ack *extack)
+{
+	struct if_stats_msg *ifsm;
+
+	if (nlh->nlmsg_len < sizeof(*ifsm)) {
+		NL_SET_ERR_MSG(extack, "Invalid header for stats dump");
+		return -EINVAL;
+	}
+
+	if (!strict_check)
+		return 0;
+
+	ifsm = nlmsg_data(nlh);
+
+	/* only requests using strict checks can pass data to influence
+	 * the dump. The legacy exception is filter_mask.
+	 */
+	if (ifsm->pad1 || ifsm->pad2 || (is_dump && ifsm->ifindex)) {
+		NL_SET_ERR_MSG(extack, "Invalid values in header for stats dump request");
+		return -EINVAL;
+	}
+	if (nlmsg_attrlen(nlh, sizeof(*ifsm))) {
+		NL_SET_ERR_MSG(extack, "Invalid attributes after stats header");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int rtnl_stats_get(struct sk_buff *skb, struct nlmsghdr *nlh,
 			  struct netlink_ext_ack *extack)
 {
@@ -4912,8 +4942,10 @@ static int rtnl_stats_get(struct sk_buff *skb, struct nlmsghdr *nlh,
 	u32 filter_mask;
 	int err;
 
-	if (nlmsg_len(nlh) < sizeof(*ifsm))
-		return -EINVAL;
+	err = rtnl_valid_stats_req(nlh, netlink_strict_get_check(skb),
+				   false, extack);
+	if (err)
+		return err;
 
 	ifsm = nlmsg_data(nlh);
 	if (ifsm->ifindex > 0)
@@ -4965,27 +4997,11 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
 
 	cb->seq = net->dev_base_seq;
 
-	if (nlmsg_len(cb->nlh) < sizeof(*ifsm)) {
-		NL_SET_ERR_MSG(extack, "Invalid header for stats dump");
-		return -EINVAL;
-	}
+	err = rtnl_valid_stats_req(cb->nlh, cb->strict_check, true, extack);
+	if (err)
+		return err;
 
 	ifsm = nlmsg_data(cb->nlh);
-
-	/* only requests using strict checks can pass data to influence
-	 * the dump. The legacy exception is filter_mask.
-	 */
-	if (cb->strict_check) {
-		if (ifsm->pad1 || ifsm->pad2 || ifsm->ifindex) {
-			NL_SET_ERR_MSG(extack, "Invalid values in header for stats dump request");
-			return -EINVAL;
-		}
-		if (nlmsg_attrlen(cb->nlh, sizeof(*ifsm))) {
-			NL_SET_ERR_MSG(extack, "Invalid attributes after stats header");
-			return -EINVAL;
-		}
-	}
-
 	filter_mask = ifsm->filter_mask;
 	if (!filter_mask) {
 		NL_SET_ERR_MSG(extack, "Filter mask must be set for stats dump");
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 03/13] rtnetlink: stats: reject requests for unknown stats
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 01/13] net: netlink: add helper to retrieve NETLINK_F_STRICT_CHK Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 02/13] rtnetlink: stats: validate attributes in get as well as dumps Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 04/13] rtnetlink: ifinfo: perform strict checks also for doit handler Jakub Kicinski
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

In the spirit of strict checks reject requests of stats the kernel
does not support when NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/core/rtnetlink.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 08f142b59403..3c134b928071 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4927,6 +4927,10 @@ static int rtnl_valid_stats_req(const struct nlmsghdr *nlh, bool strict_check,
 		NL_SET_ERR_MSG(extack, "Invalid attributes after stats header");
 		return -EINVAL;
 	}
+	if (ifsm->filter_mask >= IFLA_STATS_FILTER_BIT(IFLA_STATS_MAX + 1)) {
+		NL_SET_ERR_MSG(extack, "Invalid stats requested through filter mask");
+		return -EINVAL;
+	}
 
 	return 0;
 }
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 04/13] rtnetlink: ifinfo: perform strict checks also for doit handler
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (2 preceding siblings ...)
  2019-01-17 22:52 ` [PATCH net-next 03/13] rtnetlink: stats: reject requests for unknown stats Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 05/13] net: ipv4: perform strict checks also for doit handlers Jakub Kicinski
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Make RTM_GETLINK's doit handler use strict checks when
NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/core/rtnetlink.c | 49 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 3c134b928071..aef9cbca8358 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3242,6 +3242,53 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return ret;
 }
 
+static int rtnl_valid_getlink_req(struct sk_buff *skb,
+				  const struct nlmsghdr *nlh,
+				  struct nlattr **tb,
+				  struct netlink_ext_ack *extack)
+{
+	struct ifinfomsg *ifm;
+	int i, err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
+		NL_SET_ERR_MSG(extack, "Invalid header for get link");
+		return -EINVAL;
+	}
+
+	if (!netlink_strict_get_check(skb))
+		return nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy,
+				   extack);
+
+	ifm = nlmsg_data(nlh);
+	if (ifm->__ifi_pad || ifm->ifi_type || ifm->ifi_flags ||
+	    ifm->ifi_change) {
+		NL_SET_ERR_MSG(extack, "Invalid values in header for get link request");
+		return -EINVAL;
+	}
+
+	err = nlmsg_parse_strict(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy,
+				 extack);
+	if (err)
+		return err;
+
+	for (i = 0; i <= IFLA_MAX; i++) {
+		if (!tb[i])
+			continue;
+
+		switch (i) {
+		case IFLA_IFNAME:
+		case IFLA_EXT_MASK:
+		case IFLA_TARGET_NETNSID:
+			break;
+		default:
+			NL_SET_ERR_MSG(extack, "Unsupported attribute in get link request");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 			struct netlink_ext_ack *extack)
 {
@@ -3256,7 +3303,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 	int err;
 	u32 ext_filter_mask = 0;
 
-	err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
+	err = rtnl_valid_getlink_req(skb, nlh, tb, extack);
 	if (err < 0)
 		return err;
 
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 05/13] net: ipv4: perform strict checks also for doit handlers
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (3 preceding siblings ...)
  2019-01-17 22:52 ` [PATCH net-next 04/13] rtnetlink: ifinfo: perform strict checks also for doit handler Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-18 14:41   ` David Ahern
  2019-01-17 22:52 ` [PATCH net-next 06/13] net: namespace: " Jakub Kicinski
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Make RTM_GETNETCONF's doit handler use strict checks when
NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/ipv4/devinet.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index e258a00b4a3d..cd027639df2f 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2063,13 +2063,49 @@ static const struct nla_policy devconf_ipv4_policy[NETCONFA_MAX+1] = {
 	[NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN]	= { .len = sizeof(int) },
 };
 
+static int inet_netconf_valid_get_req(struct sk_buff *skb,
+				      const struct nlmsghdr *nlh,
+				      struct nlattr **tb,
+				      struct netlink_ext_ack *extack)
+{
+	int i, err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(struct netconfmsg))) {
+		NL_SET_ERR_MSG(extack, "ipv4: Invalid header for netconf get request");
+		return -EINVAL;
+	}
+
+	if (!netlink_strict_get_check(skb))
+		return nlmsg_parse(nlh, sizeof(struct netconfmsg), tb,
+				   NETCONFA_MAX, devconf_ipv4_policy, extack);
+
+	err = nlmsg_parse_strict(nlh, sizeof(struct netconfmsg), tb,
+				 NETCONFA_MAX, devconf_ipv4_policy, extack);
+	if (err)
+		return err;
+
+	for (i = 0; i <= NETCONFA_MAX; i++) {
+		if (!tb[i])
+			continue;
+
+		switch (i) {
+		case NETCONFA_IFINDEX:
+			break;
+		default:
+			NL_SET_ERR_MSG(extack, "ipv4: Unsupported attribute in netconf get request");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int inet_netconf_get_devconf(struct sk_buff *in_skb,
 				    struct nlmsghdr *nlh,
 				    struct netlink_ext_ack *extack)
 {
 	struct net *net = sock_net(in_skb->sk);
 	struct nlattr *tb[NETCONFA_MAX+1];
-	struct netconfmsg *ncm;
 	struct sk_buff *skb;
 	struct ipv4_devconf *devconf;
 	struct in_device *in_dev;
@@ -2077,9 +2113,8 @@ static int inet_netconf_get_devconf(struct sk_buff *in_skb,
 	int ifindex;
 	int err;
 
-	err = nlmsg_parse(nlh, sizeof(*ncm), tb, NETCONFA_MAX,
-			  devconf_ipv4_policy, extack);
-	if (err < 0)
+	err = inet_netconf_valid_get_req(in_skb, nlh, tb, extack);
+	if (err)
 		goto errout;
 
 	err = -EINVAL;
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 06/13] net: namespace: perform strict checks also for doit handlers
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (4 preceding siblings ...)
  2019-01-17 22:52 ` [PATCH net-next 05/13] net: ipv4: perform strict checks also for doit handlers Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-18  8:17   ` Nicolas Dichtel
  2019-01-17 22:52 ` [PATCH net-next 07/13] net: ipv4: ipmr: " Jakub Kicinski
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern
  Cc: netdev, oss-drivers, Jakub Kicinski, ktkhai, nicolas.dichtel

Make RTM_GETNSID's doit handler use strict checks when
NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
CC: ktkhai@virtuozzo.com
CC: nicolas.dichtel@6wind.com
---
 net/core/net_namespace.c | 43 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index b02fb19df2cc..1b45e3ab2b65 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -778,6 +778,46 @@ static int rtnl_net_fill(struct sk_buff *skb, struct net_fill_args *args)
 	return -EMSGSIZE;
 }
 
+static int rtnl_net_valid_getid_req(struct sk_buff *skb,
+				    const struct nlmsghdr *nlh,
+				    struct nlattr **tb,
+				    struct netlink_ext_ack *extack)
+{
+	int i, err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(struct rtgenmsg))) {
+		NL_SET_ERR_MSG(extack, "Invalid header for peer netns getid");
+		return -EINVAL;
+	}
+
+	if (!netlink_strict_get_check(skb))
+		return nlmsg_parse(nlh, sizeof(struct rtgenmsg), tb, NETNSA_MAX,
+				   rtnl_net_policy, extack);
+
+	err = nlmsg_parse_strict(nlh, sizeof(struct rtgenmsg), tb, NETNSA_MAX,
+				 rtnl_net_policy, extack);
+	if (err)
+		return err;
+
+	for (i = 0; i <= NETNSA_MAX; i++) {
+		if (!tb[i])
+			continue;
+
+		switch (i) {
+		case NETNSA_PID:
+		case NETNSA_FD:
+		case NETNSA_NSID:
+		case NETNSA_TARGET_NSID:
+			break;
+		default:
+			NL_SET_ERR_MSG(extack, "Unsupported attribute in peer netns getid request");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int rtnl_net_getid(struct sk_buff *skb, struct nlmsghdr *nlh,
 			  struct netlink_ext_ack *extack)
 {
@@ -793,8 +833,7 @@ static int rtnl_net_getid(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct sk_buff *msg;
 	int err;
 
-	err = nlmsg_parse(nlh, sizeof(struct rtgenmsg), tb, NETNSA_MAX,
-			  rtnl_net_policy, extack);
+	err = rtnl_net_valid_getid_req(skb, nlh, tb, extack);
 	if (err < 0)
 		return err;
 	if (tb[NETNSA_PID]) {
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 07/13] net: ipv4: ipmr: perform strict checks also for doit handlers
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (5 preceding siblings ...)
  2019-01-17 22:52 ` [PATCH net-next 06/13] net: namespace: " Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-18 14:24   ` David Ahern
  2019-01-17 22:52 ` [PATCH net-next 08/13] net: ipv6: addr: " Jakub Kicinski
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Make RTM_GETROUTE's doit handler use strict checks when
NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/ipv4/ipmr.c | 61 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 56 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index ddbf8c9a1abb..4842e36af482 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2467,6 +2467,61 @@ static void igmpmsg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt)
 	rtnl_set_sk_err(net, RTNLGRP_IPV4_MROUTE_R, -ENOBUFS);
 }
 
+static int ipmr_rtm_valid_getroute_req(struct sk_buff *skb,
+				       const struct nlmsghdr *nlh,
+				       struct nlattr **tb,
+				       struct netlink_ext_ack *extack)
+{
+	struct rtmsg *rtm;
+	int i, err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) {
+		NL_SET_ERR_MSG(extack, "ipv4: MR invalid header for route get");
+		return -EINVAL;
+	}
+
+	if (!netlink_strict_get_check(skb))
+		return nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX,
+				   rtm_ipv4_policy, extack);
+
+	rtm = nlmsg_data(nlh);
+	if ((rtm->rtm_src_len && rtm->rtm_src_len != 32) ||
+	    (rtm->rtm_dst_len && rtm->rtm_dst_len != 32) ||
+	    rtm->rtm_tos || rtm->rtm_table || rtm->rtm_protocol ||
+	    rtm->rtm_scope || rtm->rtm_type || rtm->rtm_flags) {
+		NL_SET_ERR_MSG(extack, "ipv4: MR invalid values in header for route get request");
+		return -EINVAL;
+	}
+
+	err = nlmsg_parse_strict(nlh, sizeof(*rtm), tb, RTA_MAX,
+				 rtm_ipv4_policy, extack);
+	if (err)
+		return err;
+
+	if ((tb[RTA_SRC] && !rtm->rtm_src_len) ||
+	    (tb[RTA_DST] && !rtm->rtm_dst_len)) {
+		NL_SET_ERR_MSG(extack, "ipv4: MR rtm_src_len and rtm_dst_len must be 32 for IPv4");
+		return -EINVAL;
+	}
+
+	for (i = 0; i <= RTA_MAX; i++) {
+		if (!tb[i])
+			continue;
+
+		switch (i) {
+		case RTA_SRC:
+		case RTA_DST:
+		case RTA_TABLE:
+			break;
+		default:
+			NL_SET_ERR_MSG(extack, "ipv4: MR unsupported attribute in route get request");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int ipmr_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 			     struct netlink_ext_ack *extack)
 {
@@ -2475,18 +2530,14 @@ static int ipmr_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 	struct sk_buff *skb = NULL;
 	struct mfc_cache *cache;
 	struct mr_table *mrt;
-	struct rtmsg *rtm;
 	__be32 src, grp;
 	u32 tableid;
 	int err;
 
-	err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX,
-			  rtm_ipv4_policy, extack);
+	err = ipmr_rtm_valid_getroute_req(in_skb, nlh, tb, extack);
 	if (err < 0)
 		goto errout;
 
-	rtm = nlmsg_data(nlh);
-
 	src = tb[RTA_SRC] ? nla_get_in_addr(tb[RTA_SRC]) : 0;
 	grp = tb[RTA_DST] ? nla_get_in_addr(tb[RTA_DST]) : 0;
 	tableid = tb[RTA_TABLE] ? nla_get_u32(tb[RTA_TABLE]) : 0;
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 08/13] net: ipv6: addr: perform strict checks also for doit handlers
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (6 preceding siblings ...)
  2019-01-17 22:52 ` [PATCH net-next 07/13] net: ipv4: ipmr: " Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 09/13] net: ipv6: netconf: " Jakub Kicinski
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Make RTM_GETADDR's doit handler use strict checks when
NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/ipv6/addrconf.c | 49 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 93d5ad2b1a69..339a82cfe5f2 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5179,6 +5179,52 @@ static int inet6_dump_ifacaddr(struct sk_buff *skb, struct netlink_callback *cb)
 	return inet6_dump_addr(skb, cb, type);
 }
 
+static int inet6_rtm_valid_getaddr_req(struct sk_buff *skb,
+				       const struct nlmsghdr *nlh,
+				       struct nlattr **tb,
+				       struct netlink_ext_ack *extack)
+{
+	struct ifaddrmsg *ifm;
+	int i, err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
+		NL_SET_ERR_MSG_MOD(extack, "Invalid header for get address request");
+		return -EINVAL;
+	}
+
+	ifm = nlmsg_data(nlh);
+	if (ifm->ifa_prefixlen || ifm->ifa_flags || ifm->ifa_scope) {
+		NL_SET_ERR_MSG_MOD(extack, "Invalid values in header for get address request");
+		return -EINVAL;
+	}
+
+	if (!netlink_strict_get_check(skb))
+		return nlmsg_parse(nlh, sizeof(*ifm), tb, IFA_MAX,
+				   ifa_ipv6_policy, extack);
+
+	err = nlmsg_parse_strict(nlh, sizeof(*ifm), tb, IFA_MAX,
+				 ifa_ipv6_policy, extack);
+	if (err)
+		return err;
+
+	for (i = 0; i <= IFA_MAX; i++) {
+		if (!tb[i])
+			continue;
+
+		switch (i) {
+		case IFA_TARGET_NETNSID:
+		case IFA_ADDRESS:
+		case IFA_LOCAL:
+			break;
+		default:
+			NL_SET_ERR_MSG_MOD(extack, "Unsupported attribute in get address request");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int inet6_rtm_getaddr(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 			     struct netlink_ext_ack *extack)
 {
@@ -5199,8 +5245,7 @@ static int inet6_rtm_getaddr(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 	struct sk_buff *skb;
 	int err;
 
-	err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFA_MAX, ifa_ipv6_policy,
-			  extack);
+	err = inet6_rtm_valid_getaddr_req(in_skb, nlh, tb, extack);
 	if (err < 0)
 		return err;
 
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 09/13] net: ipv6: netconf: perform strict checks also for doit handlers
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (7 preceding siblings ...)
  2019-01-17 22:52 ` [PATCH net-next 08/13] net: ipv6: addr: " Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 10/13] net: ipv6: addrlabel: " Jakub Kicinski
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Make RTM_GETNETCONF's doit handler use strict checks when
NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/ipv6/addrconf.c | 41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 339a82cfe5f2..57198b3c86da 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -597,6 +597,43 @@ static const struct nla_policy devconf_ipv6_policy[NETCONFA_MAX+1] = {
 	[NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN]	= { .len = sizeof(int) },
 };
 
+static int inet6_netconf_valid_get_req(struct sk_buff *skb,
+				       const struct nlmsghdr *nlh,
+				       struct nlattr **tb,
+				       struct netlink_ext_ack *extack)
+{
+	int i, err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(struct netconfmsg))) {
+		NL_SET_ERR_MSG_MOD(extack, "Invalid header for netconf get request");
+		return -EINVAL;
+	}
+
+	if (!netlink_strict_get_check(skb))
+		return nlmsg_parse(nlh, sizeof(struct netconfmsg), tb,
+				   NETCONFA_MAX, devconf_ipv6_policy, extack);
+
+	err = nlmsg_parse_strict(nlh, sizeof(struct netconfmsg), tb,
+				 NETCONFA_MAX, devconf_ipv6_policy, extack);
+	if (err)
+		return err;
+
+	for (i = 0; i <= NETCONFA_MAX; i++) {
+		if (!tb[i])
+			continue;
+
+		switch (i) {
+		case NETCONFA_IFINDEX:
+			break;
+		default:
+			NL_SET_ERR_MSG_MOD(extack, "Unsupported attribute in netconf get request");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int inet6_netconf_get_devconf(struct sk_buff *in_skb,
 				     struct nlmsghdr *nlh,
 				     struct netlink_ext_ack *extack)
@@ -605,14 +642,12 @@ static int inet6_netconf_get_devconf(struct sk_buff *in_skb,
 	struct nlattr *tb[NETCONFA_MAX+1];
 	struct inet6_dev *in6_dev = NULL;
 	struct net_device *dev = NULL;
-	struct netconfmsg *ncm;
 	struct sk_buff *skb;
 	struct ipv6_devconf *devconf;
 	int ifindex;
 	int err;
 
-	err = nlmsg_parse(nlh, sizeof(*ncm), tb, NETCONFA_MAX,
-			  devconf_ipv6_policy, extack);
+	err = inet6_netconf_valid_get_req(in_skb, nlh, tb, extack);
 	if (err < 0)
 		return err;
 
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 10/13] net: ipv6: addrlabel: perform strict checks also for doit handlers
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (8 preceding siblings ...)
  2019-01-17 22:52 ` [PATCH net-next 09/13] net: ipv6: netconf: " Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 11/13] net: ipv6: route: " Jakub Kicinski
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Make RTM_GETADDRLABEL's doit handler use strict checks when
NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/ipv6/addrlabel.c | 47 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
index 0d1ee82ee55b..d43d076c98f5 100644
--- a/net/ipv6/addrlabel.c
+++ b/net/ipv6/addrlabel.c
@@ -523,6 +523,50 @@ static inline int ip6addrlbl_msgsize(void)
 		+ nla_total_size(4);	/* IFAL_LABEL */
 }
 
+static int ip6addrlbl_valid_get_req(struct sk_buff *skb,
+				    const struct nlmsghdr *nlh,
+				    struct nlattr **tb,
+				    struct netlink_ext_ack *extack)
+{
+	struct ifaddrlblmsg *ifal;
+	int i, err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifal))) {
+		NL_SET_ERR_MSG_MOD(extack, "Invalid header for addrlabel get request");
+		return -EINVAL;
+	}
+
+	if (!netlink_strict_get_check(skb))
+		return nlmsg_parse(nlh, sizeof(*ifal), tb, IFAL_MAX,
+				   ifal_policy, extack);
+
+	ifal = nlmsg_data(nlh);
+	if (ifal->__ifal_reserved || ifal->ifal_flags || ifal->ifal_seq) {
+		NL_SET_ERR_MSG_MOD(extack, "Invalid values in header for addrlabel get request");
+		return -EINVAL;
+	}
+
+	err = nlmsg_parse_strict(nlh, sizeof(*ifal), tb, IFAL_MAX,
+				 ifal_policy, extack);
+	if (err)
+		return err;
+
+	for (i = 0; i <= IFAL_MAX; i++) {
+		if (!tb[i])
+			continue;
+
+		switch (i) {
+		case IFAL_ADDRESS:
+			break;
+		default:
+			NL_SET_ERR_MSG_MOD(extack, "Unsupported attribute in addrlabel get request");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int ip6addrlbl_get(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 			  struct netlink_ext_ack *extack)
 {
@@ -535,8 +579,7 @@ static int ip6addrlbl_get(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 	struct ip6addrlbl_entry *p;
 	struct sk_buff *skb;
 
-	err = nlmsg_parse(nlh, sizeof(*ifal), tb, IFAL_MAX, ifal_policy,
-			  extack);
+	err = ip6addrlbl_valid_get_req(in_skb, nlh, tb, extack);
 	if (err < 0)
 		return err;
 
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 11/13] net: ipv6: route: perform strict checks also for doit handlers
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (9 preceding siblings ...)
  2019-01-17 22:52 ` [PATCH net-next 10/13] net: ipv6: addrlabel: " Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-17 22:52 ` [PATCH net-next 12/13] net: mpls: " Jakub Kicinski
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Make RTM_GETROUTE's doit handler use strict checks when
NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/ipv6/route.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 68 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 964491cf3672..dc066fdf7e46 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4812,6 +4812,73 @@ int rt6_dump_route(struct fib6_info *rt, void *p_arg)
 			     arg->cb->nlh->nlmsg_seq, flags);
 }
 
+static int inet6_rtm_valid_getroute_req(struct sk_buff *skb,
+					const struct nlmsghdr *nlh,
+					struct nlattr **tb,
+					struct netlink_ext_ack *extack)
+{
+	struct rtmsg *rtm;
+	int i, err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Invalid header for get route request");
+		return -EINVAL;
+	}
+
+	if (!netlink_strict_get_check(skb))
+		return nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX,
+				   rtm_ipv6_policy, extack);
+
+	rtm = nlmsg_data(nlh);
+	if ((rtm->rtm_src_len && rtm->rtm_src_len != 128) ||
+	    (rtm->rtm_dst_len && rtm->rtm_dst_len != 128) ||
+	    rtm->rtm_table || rtm->rtm_protocol || rtm->rtm_scope ||
+	    rtm->rtm_type) {
+		NL_SET_ERR_MSG_MOD(extack, "Invalid values in header for get route request");
+		return -EINVAL;
+	}
+	if (rtm->rtm_flags & ~RTM_F_FIB_MATCH) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Invalid flags for get route request");
+		return -EINVAL;
+	}
+
+	err = nlmsg_parse_strict(nlh, sizeof(*rtm), tb, RTA_MAX,
+				 rtm_ipv6_policy, extack);
+	if (err)
+		return err;
+
+	if ((tb[RTA_SRC] && !rtm->rtm_src_len) ||
+	    (tb[RTA_DST] && !rtm->rtm_dst_len)) {
+		NL_SET_ERR_MSG_MOD(extack, "rtm_src_len and rtm_dst_len must be 128 for IPv6");
+		return -EINVAL;
+	}
+
+	for (i = 0; i <= RTA_MAX; i++) {
+		if (!tb[i])
+			continue;
+
+		switch (i) {
+		case RTA_SRC:
+		case RTA_DST:
+		case RTA_IIF:
+		case RTA_OIF:
+		case RTA_MARK:
+		case RTA_UID:
+		case RTA_SPORT:
+		case RTA_DPORT:
+		case RTA_IP_PROTO:
+			break;
+		default:
+			NL_SET_ERR_MSG_MOD(extack, "Unsupported attribute in get route request");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 			      struct netlink_ext_ack *extack)
 {
@@ -4826,8 +4893,7 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 	struct flowi6 fl6 = {};
 	bool fibmatch;
 
-	err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv6_policy,
-			  extack);
+	err = inet6_rtm_valid_getroute_req(in_skb, nlh, tb, extack);
 	if (err < 0)
 		goto errout;
 
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 12/13] net: mpls: route: perform strict checks also for doit handlers
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (10 preceding siblings ...)
  2019-01-17 22:52 ` [PATCH net-next 11/13] net: ipv6: route: " Jakub Kicinski
@ 2019-01-17 22:52 ` Jakub Kicinski
  2019-01-17 22:53 ` [PATCH net-next 13/13] net: mpls: netconf: " Jakub Kicinski
  2019-01-18 14:45 ` [PATCH net-next 00/13] net: use strict checks in " David Ahern
  13 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:52 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Make RTM_GETROUTE's doit handler use strict checks when
NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/mpls/af_mpls.c | 61 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 7d55d4c04088..733c86db551b 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -2236,6 +2236,64 @@ static void rtmsg_lfib(int event, u32 label, struct mpls_route *rt,
 		rtnl_set_sk_err(net, RTNLGRP_MPLS_ROUTE, err);
 }
 
+static int mpls_valid_getroute_req(struct sk_buff *skb,
+				   const struct nlmsghdr *nlh,
+				   struct nlattr **tb,
+				   struct netlink_ext_ack *extack)
+{
+	struct rtmsg *rtm;
+	int i, err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Invalid header for get route request");
+		return -EINVAL;
+	}
+
+	if (!netlink_strict_get_check(skb))
+		return nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX,
+				   rtm_mpls_policy, extack);
+
+	rtm = nlmsg_data(nlh);
+	if ((rtm->rtm_dst_len && rtm->rtm_dst_len != 20) ||
+	    rtm->rtm_src_len || rtm->rtm_tos || rtm->rtm_table ||
+	    rtm->rtm_protocol || rtm->rtm_scope || rtm->rtm_type) {
+		NL_SET_ERR_MSG_MOD(extack, "Invalid values in header for get route request");
+		return -EINVAL;
+	}
+	if (rtm->rtm_flags & ~RTM_F_FIB_MATCH) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Invalid flags for get route request");
+		return -EINVAL;
+	}
+
+	err = nlmsg_parse_strict(nlh, sizeof(*rtm), tb, RTA_MAX,
+				 rtm_mpls_policy, extack);
+	if (err)
+		return err;
+
+	if ((tb[RTA_DST] || tb[RTA_NEWDST]) && !rtm->rtm_dst_len) {
+		NL_SET_ERR_MSG_MOD(extack, "rtm_dst_len must be 20 for MPLS");
+		return -EINVAL;
+	}
+
+	for (i = 0; i <= RTA_MAX; i++) {
+		if (!tb[i])
+			continue;
+
+		switch (i) {
+		case RTA_DST:
+		case RTA_NEWDST:
+			break;
+		default:
+			NL_SET_ERR_MSG_MOD(extack, "Unsupported attribute in get route request");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int mpls_getroute(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
 			 struct netlink_ext_ack *extack)
 {
@@ -2255,8 +2313,7 @@ static int mpls_getroute(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
 	u8 n_labels;
 	int err;
 
-	err = nlmsg_parse(in_nlh, sizeof(*rtm), tb, RTA_MAX,
-			  rtm_mpls_policy, extack);
+	err = mpls_valid_getroute_req(in_skb, in_nlh, tb, extack);
 	if (err < 0)
 		goto errout;
 
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 13/13] net: mpls: netconf: perform strict checks also for doit handlers
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (11 preceding siblings ...)
  2019-01-17 22:52 ` [PATCH net-next 12/13] net: mpls: " Jakub Kicinski
@ 2019-01-17 22:53 ` Jakub Kicinski
  2019-01-18 14:45 ` [PATCH net-next 00/13] net: use strict checks in " David Ahern
  13 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-17 22:53 UTC (permalink / raw)
  To: davem, dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski

Make RTM_GETNETCONF's doit handler use strict checks when
NETLINK_F_STRICT_CHK is set.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/mpls/af_mpls.c | 42 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 733c86db551b..2662a23c658e 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -1209,21 +1209,57 @@ static const struct nla_policy devconf_mpls_policy[NETCONFA_MAX + 1] = {
 	[NETCONFA_IFINDEX]	= { .len = sizeof(int) },
 };
 
+static int mpls_netconf_valid_get_req(struct sk_buff *skb,
+				      const struct nlmsghdr *nlh,
+				      struct nlattr **tb,
+				      struct netlink_ext_ack *extack)
+{
+	int i, err;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(struct netconfmsg))) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Invalid header for netconf get request");
+		return -EINVAL;
+	}
+
+	if (!netlink_strict_get_check(skb))
+		return nlmsg_parse(nlh, sizeof(struct netconfmsg), tb,
+				   NETCONFA_MAX, devconf_mpls_policy, extack);
+
+	err = nlmsg_parse_strict(nlh, sizeof(struct netconfmsg), tb,
+				 NETCONFA_MAX, devconf_mpls_policy, extack);
+	if (err)
+		return err;
+
+	for (i = 0; i <= NETCONFA_MAX; i++) {
+		if (!tb[i])
+			continue;
+
+		switch (i) {
+		case NETCONFA_IFINDEX:
+			break;
+		default:
+			NL_SET_ERR_MSG_MOD(extack, "Unsupported attribute in netconf get request");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int mpls_netconf_get_devconf(struct sk_buff *in_skb,
 				    struct nlmsghdr *nlh,
 				    struct netlink_ext_ack *extack)
 {
 	struct net *net = sock_net(in_skb->sk);
 	struct nlattr *tb[NETCONFA_MAX + 1];
-	struct netconfmsg *ncm;
 	struct net_device *dev;
 	struct mpls_dev *mdev;
 	struct sk_buff *skb;
 	int ifindex;
 	int err;
 
-	err = nlmsg_parse(nlh, sizeof(*ncm), tb, NETCONFA_MAX,
-			  devconf_mpls_policy, extack);
+	err = mpls_netconf_valid_get_req(in_skb, nlh, tb, extack);
 	if (err < 0)
 		goto errout;
 
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 06/13] net: namespace: perform strict checks also for doit handlers
  2019-01-17 22:52 ` [PATCH net-next 06/13] net: namespace: " Jakub Kicinski
@ 2019-01-18  8:17   ` Nicolas Dichtel
  2019-01-18 18:00     ` Jakub Kicinski
  0 siblings, 1 reply; 20+ messages in thread
From: Nicolas Dichtel @ 2019-01-18  8:17 UTC (permalink / raw)
  To: Jakub Kicinski, davem, dsahern; +Cc: netdev, oss-drivers, ktkhai

Le 17/01/2019 à 23:52, Jakub Kicinski a écrit :
> Make RTM_GETNSID's doit handler use strict checks when
> NETLINK_F_STRICT_CHK is set.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> CC: ktkhai@virtuozzo.com
> CC: nicolas.dichtel@6wind.com
> ---
>  net/core/net_namespace.c | 43 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index b02fb19df2cc..1b45e3ab2b65 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -778,6 +778,46 @@ static int rtnl_net_fill(struct sk_buff *skb, struct net_fill_args *args)
>  	return -EMSGSIZE;
>  }
>  
> +static int rtnl_net_valid_getid_req(struct sk_buff *skb,
> +				    const struct nlmsghdr *nlh,
> +				    struct nlattr **tb,
> +				    struct netlink_ext_ack *extack)
> +{
> +	int i, err;
> +
> +	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(struct rtgenmsg))) {
This is not possible, the check is already done in rtnetlink_rcv_msg().


Regards,
Nicolas

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 07/13] net: ipv4: ipmr: perform strict checks also for doit handlers
  2019-01-17 22:52 ` [PATCH net-next 07/13] net: ipv4: ipmr: " Jakub Kicinski
@ 2019-01-18 14:24   ` David Ahern
  0 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2019-01-18 14:24 UTC (permalink / raw)
  To: Jakub Kicinski, davem; +Cc: netdev, oss-drivers

On 1/17/19 3:52 PM, Jakub Kicinski wrote:
> +	rtm = nlmsg_data(nlh);
> +	if ((rtm->rtm_src_len && rtm->rtm_src_len != 32) ||
> +	    (rtm->rtm_dst_len && rtm->rtm_dst_len != 32) ||
> +	    rtm->rtm_tos || rtm->rtm_table || rtm->rtm_protocol ||
> +	    rtm->rtm_scope || rtm->rtm_type || rtm->rtm_flags) {
> +		NL_SET_ERR_MSG(extack, "ipv4: MR invalid values in header for route get request");

The MR at the beginning makes that read awkwardly. Perhaps:

ipv4: Invalid values in header for multicast route get request


> +		return -EINVAL;
> +	}
> +
> +	err = nlmsg_parse_strict(nlh, sizeof(*rtm), tb, RTA_MAX,
> +				 rtm_ipv4_policy, extack);
> +	if (err)
> +		return err;
> +
> +	if ((tb[RTA_SRC] && !rtm->rtm_src_len) ||
> +	    (tb[RTA_DST] && !rtm->rtm_dst_len)) {
> +		NL_SET_ERR_MSG(extack, "ipv4: MR rtm_src_len and rtm_dst_len must be 32 for IPv4");

and here. Also ipv4 is listed twice.

> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i <= RTA_MAX; i++) {
> +		if (!tb[i])
> +			continue;
> +
> +		switch (i) {
> +		case RTA_SRC:
> +		case RTA_DST:
> +		case RTA_TABLE:
> +			break;
> +		default:
> +			NL_SET_ERR_MSG(extack, "ipv4: MR unsupported attribute in route get request");

and here


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 05/13] net: ipv4: perform strict checks also for doit handlers
  2019-01-17 22:52 ` [PATCH net-next 05/13] net: ipv4: perform strict checks also for doit handlers Jakub Kicinski
@ 2019-01-18 14:41   ` David Ahern
  0 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2019-01-18 14:41 UTC (permalink / raw)
  To: Jakub Kicinski, davem; +Cc: netdev, oss-drivers

can you add netconf to the patch subject since only it is converted.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 00/13] net: use strict checks in doit handlers
  2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
                   ` (12 preceding siblings ...)
  2019-01-17 22:53 ` [PATCH net-next 13/13] net: mpls: netconf: " Jakub Kicinski
@ 2019-01-18 14:45 ` David Ahern
  2019-01-18 18:01   ` Jakub Kicinski
  13 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2019-01-18 14:45 UTC (permalink / raw)
  To: Jakub Kicinski, davem; +Cc: netdev, oss-drivers

On 1/17/19 3:52 PM, Jakub Kicinski wrote:
> Hi!
> 
> This series extends strict argument checking to doit handlers
> of the GET* nature.  This is a bit tricky since strict checking
> flag has already been released..
> 
> iproute2 did not have a release with strick checks enabled,
> and it will only need a minor one-liner to pass strick checks
> after all the work that DaveA has already done.
> 
> Big thanks to Dave Ahern for help and guidence.
> DaveA, does this look good to you?
> 
> Jakub Kicinski (13):
>   net: netlink: add helper to retrieve NETLINK_F_STRICT_CHK
>   rtnetlink: stats: validate attributes in get as well as dumps
>   rtnetlink: stats: reject requests for unknown stats
>   rtnetlink: ifinfo: perform strict checks also for doit handler
>   net: ipv4: perform strict checks also for doit handlers
>   net: namespace: perform strict checks also for doit handlers
>   net: ipv4: ipmr: perform strict checks also for doit handlers
>   net: ipv6: addr: perform strict checks also for doit handlers
>   net: ipv6: netconf: perform strict checks also for doit handlers
>   net: ipv6: addrlabel: perform strict checks also for doit handlers
>   net: ipv6: route: perform strict checks also for doit handlers
>   net: mpls: route: perform strict checks also for doit handlers
>   net: mpls: netconf: perform strict checks also for doit handlers

Thanks for working on this. Besides the few nits, all of the changes
look good to me.

Looking at RTM_GET handlers I noticed inet_rtm_getroute and
rtnl_net_getid are not included in this set. Any reason?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 06/13] net: namespace: perform strict checks also for doit handlers
  2019-01-18  8:17   ` Nicolas Dichtel
@ 2019-01-18 18:00     ` Jakub Kicinski
  0 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-18 18:00 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: davem, dsahern, netdev, oss-drivers, ktkhai

On Fri, 18 Jan 2019 09:17:46 +0100, Nicolas Dichtel wrote:
> > +static int rtnl_net_valid_getid_req(struct sk_buff *skb,
> > +				    const struct nlmsghdr *nlh,
> > +				    struct nlattr **tb,
> > +				    struct netlink_ext_ack *extack)
> > +{
> > +	int i, err;
> > +
> > +	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(struct rtgenmsg))) {  
> This is not possible, the check is already done in rtnetlink_rcv_msg().

Good point, thanks!

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 00/13] net: use strict checks in doit handlers
  2019-01-18 14:45 ` [PATCH net-next 00/13] net: use strict checks in " David Ahern
@ 2019-01-18 18:01   ` Jakub Kicinski
  0 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2019-01-18 18:01 UTC (permalink / raw)
  To: David Ahern; +Cc: davem, netdev, oss-drivers

On Fri, 18 Jan 2019 07:45:50 -0700, David Ahern wrote:
> On 1/17/19 3:52 PM, Jakub Kicinski wrote:
> > Hi!
> > 
> > This series extends strict argument checking to doit handlers
> > of the GET* nature.  This is a bit tricky since strict checking
> > flag has already been released..
> > 
> > iproute2 did not have a release with strick checks enabled,
> > and it will only need a minor one-liner to pass strick checks
> > after all the work that DaveA has already done.
> > 
> > Big thanks to Dave Ahern for help and guidence.
> > DaveA, does this look good to you?
> > 
> > Jakub Kicinski (13):
> >   net: netlink: add helper to retrieve NETLINK_F_STRICT_CHK
> >   rtnetlink: stats: validate attributes in get as well as dumps
> >   rtnetlink: stats: reject requests for unknown stats
> >   rtnetlink: ifinfo: perform strict checks also for doit handler
> >   net: ipv4: perform strict checks also for doit handlers
> >   net: namespace: perform strict checks also for doit handlers
> >   net: ipv4: ipmr: perform strict checks also for doit handlers
> >   net: ipv6: addr: perform strict checks also for doit handlers
> >   net: ipv6: netconf: perform strict checks also for doit handlers
> >   net: ipv6: addrlabel: perform strict checks also for doit handlers
> >   net: ipv6: route: perform strict checks also for doit handlers
> >   net: mpls: route: perform strict checks also for doit handlers
> >   net: mpls: netconf: perform strict checks also for doit handlers  
> 
> Thanks for working on this. Besides the few nits, all of the changes
> look good to me.

Thanks for those, will fix!

> Looking at RTM_GET handlers I noticed inet_rtm_getroute and
> rtnl_net_getid are not included in this set. Any reason?

I must have misread doit-only as dumpit-only.  Will add.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-01-18 18:01 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-17 22:52 [PATCH net-next 00/13] net: use strict checks in doit handlers Jakub Kicinski
2019-01-17 22:52 ` [PATCH net-next 01/13] net: netlink: add helper to retrieve NETLINK_F_STRICT_CHK Jakub Kicinski
2019-01-17 22:52 ` [PATCH net-next 02/13] rtnetlink: stats: validate attributes in get as well as dumps Jakub Kicinski
2019-01-17 22:52 ` [PATCH net-next 03/13] rtnetlink: stats: reject requests for unknown stats Jakub Kicinski
2019-01-17 22:52 ` [PATCH net-next 04/13] rtnetlink: ifinfo: perform strict checks also for doit handler Jakub Kicinski
2019-01-17 22:52 ` [PATCH net-next 05/13] net: ipv4: perform strict checks also for doit handlers Jakub Kicinski
2019-01-18 14:41   ` David Ahern
2019-01-17 22:52 ` [PATCH net-next 06/13] net: namespace: " Jakub Kicinski
2019-01-18  8:17   ` Nicolas Dichtel
2019-01-18 18:00     ` Jakub Kicinski
2019-01-17 22:52 ` [PATCH net-next 07/13] net: ipv4: ipmr: " Jakub Kicinski
2019-01-18 14:24   ` David Ahern
2019-01-17 22:52 ` [PATCH net-next 08/13] net: ipv6: addr: " Jakub Kicinski
2019-01-17 22:52 ` [PATCH net-next 09/13] net: ipv6: netconf: " Jakub Kicinski
2019-01-17 22:52 ` [PATCH net-next 10/13] net: ipv6: addrlabel: " Jakub Kicinski
2019-01-17 22:52 ` [PATCH net-next 11/13] net: ipv6: route: " Jakub Kicinski
2019-01-17 22:52 ` [PATCH net-next 12/13] net: mpls: " Jakub Kicinski
2019-01-17 22:53 ` [PATCH net-next 13/13] net: mpls: netconf: " Jakub Kicinski
2019-01-18 14:45 ` [PATCH net-next 00/13] net: use strict checks in " David Ahern
2019-01-18 18:01   ` Jakub Kicinski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.