All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request
@ 2018-10-02  0:28 David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 01/25] net/netlink: Pass extack to dump callbacks David Ahern
                   ` (25 more replies)
  0 siblings, 26 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

There are many use cases where a user wants to influence what is
returned in a dump for some rtnetlink command: one is wanting data
for a different namespace than the one the request is received and
another is limiting the amount of data returned in the dump to a
specific set of interest to userspace, reducing the cpu overhead of
both kernel and userspace. Unfortunately, the kernel has historically
not been strict with checking for the proper header or checking the
values passed in the header. This lenient implementation has allowed
iproute2 and other packages to pass any struct or data in the dump
request as long as the family is the first byte. For example, ifinfomsg
struct is used by iproute2 for all generic dump requests - links,
addresses, routes and rules when it is really only valid for link
requests.

There is 1 is example where the kernel deals with the wrong struct: link
dumps after VF support was added. Older iproute2 was sending rtgenmsg as
the header instead of ifinfomsg so a patch was added to try and detect
old userspace vs new:
e5eca6d41f53 ("rtnetlink: fix userspace API breakage for iproute2 < v3.9.0")

The latest example is Christian's patch set wanting to return addresses for
a target namespace. It guesses the header struct is an ifaddrmsg and if it
guesses wrong a netlink warning is generated in the kernel log on every
address dump which is unacceptable.

Another example where the kernel is a bit lenient is route dumps: iproute2
can send either a request with either ifinfomsg or a rtmsg as the header
struct, yet the kernel always treats the header as an rtmsg (see
inet_dump_fib and rtm_flags check). The header inconsistency impacts the
ability to add kernel side filters for route dumps - a necessary feature
for scale setups with 100k+ routes.

How to resolve the problem of not breaking old userspace yet be able to
move forward with new features such as kernel side filtering which are
crucial for efficient operation at high scale?

This patch set addresses the problem by adding a new netlink flag,
NLM_F_DUMP_PROPER_HDR, that userspace can set to say "I have a clue, and
I am sending the right header struct" and that the struct fields and any
attributes after it should be used for filtering the data returned in the
dump.

Kernel side, the dump handlers are updated to verify the message contains
at least the expected header struct:
    RTM_GETLINK:       ifinfomsg
    RTM_GETADDR:       ifaddrmsg
    RTM_GETMULTICAST:  ifaddrmsg
    RTM_GETANYCAST:    ifaddrmsg
    RTM_GETADDRLABEL:  ifaddrlblmsg
    RTM_GETROUTE:      rtmsg
    RTM_GETSTATS:      if_stats_msg
    RTM_GETNEIGH:      ndmsg
    RTM_GETNEIGHTBL:   ndtmsg
    RTM_GETNSID:       rtnl_net_dumpid
    RTM_GETRULE:       fib_rule_hdr
    RTM_GETNETCONF:    netconfmsg
    RTM_GETMDB:        br_port_msg

And then every field in the header struct should be 0 with the exception
of the family. There are a few exceptions to this rule where the kernel
already influences the data returned by values in the struct. Next the
message should not contain attributes unless the kernel implements
filtering for it. Any unexpected data causes the dump to fail with EINVAL.
If the new flag is honored by the kernel and the dump contents adjusted
by any data passed in the request, the dump handler can set the
NLM_F_DUMP_FILTERED flag in the netlink message header.

As an example of how this new NLM_F_DUMP_PROPER_HDR can be leveraged,
the last 6 patch add filtering of route dumps based on table id, protocol,
tos, flags, scope, and egress device.

David Ahern (25):
  net/netlink: Pass extack to dump callbacks
  net/ipv6: Refactor address dump to push inet6_fill_args to
    in6_dump_addrs
  netlink: introduce NLM_F_DUMP_PROPER_HDR flag
  net/ipv4: Update inet_dump_ifaddr to support NLM_F_DUMP_PROPER_HDR
  net/ipv6: Update inet6_dump_addr to support NLM_F_DUMP_PROPER_HDR
  rtnetlink: Update rtnl_dump_ifinfo to support NLM_F_DUMP_PROPER_HDR
  rtnetlink: Update rtnl_bridge_getlink to support NLM_F_DUMP_PROPER_HDR
  rtnetlink: Update rtnl_stats_dump to support NLM_F_DUMP_PROPER_HDR
  rtnetlink: Update inet6_dump_ifinfo to support NLM_F_DUMP_PROPER_HDR
  rtnetlink: Update ipmr_rtm_dumplink to support NLM_F_DUMP_PROPER_HDR
  rtnetlink: Update fib dumps to support NLM_F_DUMP_PROPER_HDR
  net/neigh: Refactor dump filter handling
  net/neighbor: Update neigh_dump_info to support NLM_F_DUMP_PROPER_HDR
  net/neighbor: Update neightbl_dump_info to support
    NLM_F_DUMP_PROPER_HDR
  net/namespace: Update rtnl_net_dumpid to support NLM_F_DUMP_PROPER_HDR
  net/fib_rules: Update fib_nl_dumprule to support NLM_F_DUMP_PROPER_HDR
  net/ipv6: Update ip6addrlbl_dump to support NLM_F_DUMP_PROPER_HDR
  net: Update netconf dump handlers to support NLM_F_DUMP_PROPER_HDR
  net/bridge: Update br_mdb_dump to support NLM_F_DUMP_PROPER_HDR
  net: Add struct for fib dump filter
  net/ipv4: Plumb support for filtering route dumps
  net/ipv6: Plumb support for filtering route dumps
  net/mpls: Plumb support for filtering route dumps
  net: Plumb support for filtering ipv4 and ipv6 multicast route dumps
  net: Enable kernel side filtering of route dumps

 include/linux/mroute_base.h  |   5 +-
 include/linux/netlink.h      |   2 +
 include/net/ip6_route.h      |   1 +
 include/net/ip_fib.h         |  16 +++-
 include/uapi/linux/netlink.h |   1 +
 net/bridge/br_mdb.c          |  20 ++++-
 net/core/fib_rules.c         |  25 +++++-
 net/core/neighbour.c         | 136 +++++++++++++++++++++++--------
 net/core/net_namespace.c     |   8 ++
 net/core/rtnetlink.c         | 185 +++++++++++++++++++++++++++++++++----------
 net/ipv4/devinet.c           |  74 +++++++++++++----
 net/ipv4/fib_frontend.c      |  76 +++++++++++++++++-
 net/ipv4/fib_trie.c          |  33 +++++---
 net/ipv4/ipmr.c              |  44 +++++++++-
 net/ipv4/ipmr_base.c         |  42 +++++++++-
 net/ipv6/addrconf.c          | 160 ++++++++++++++++++++++++++++---------
 net/ipv6/addrlabel.c         |  25 +++++-
 net/ipv6/ip6_fib.c           |  23 +++++-
 net/ipv6/ip6mr.c             |  12 ++-
 net/ipv6/route.c             |  36 ++++++---
 net/mpls/af_mpls.c           |  82 ++++++++++++++++++-
 net/netlink/af_netlink.c     |  20 +++--
 22 files changed, 860 insertions(+), 166 deletions(-)

-- 
2.11.0

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

* [PATCH RFC v2 net-next 01/25] net/netlink: Pass extack to dump callbacks
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02 11:07   ` Christian Brauner
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs David Ahern
                   ` (24 subsequent siblings)
  25 siblings, 1 reply; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Pass extack to dump callbacks by adding extack to netlink_dump_control,
transferring to netlink_callback and adding to the netlink_dump. Update
rtnetlink as the first user. Update netlink_dump to add any message after
the dump_done_errno.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/linux/netlink.h  |  2 ++
 net/core/rtnetlink.c     |  1 +
 net/netlink/af_netlink.c | 20 +++++++++++++++-----
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 71f121b66ca8..8fc90308a653 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -176,6 +176,7 @@ struct netlink_callback {
 	void			*data;
 	/* the module that dump function belong to */
 	struct module		*module;
+	struct netlink_ext_ack	*extack;
 	u16			family;
 	u16			min_dump_alloc;
 	unsigned int		prev_seq, seq;
@@ -197,6 +198,7 @@ struct netlink_dump_control {
 	int (*done)(struct netlink_callback *);
 	void *data;
 	struct module *module;
+	struct netlink_ext_ack *extack;
 	u16 min_dump_alloc;
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 35162e1b06ad..da91b38297d3 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4689,6 +4689,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 				.dump		= dumpit,
 				.min_dump_alloc	= min_dump_alloc,
 				.module		= owner,
+				.extack		= extack
 			};
 			err = netlink_dump_start(rtnl, skb, nlh, &c);
 			/* netlink_dump_start() will keep a reference on
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index e3a0538ec0be..7094156c94f0 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -129,7 +129,7 @@ static const char *const nlk_cb_mutex_key_strings[MAX_LINKS + 1] = {
 	"nlk_cb_mutex-MAX_LINKS"
 };
 
-static int netlink_dump(struct sock *sk);
+static int netlink_dump(struct sock *sk, struct netlink_ext_ack *extack);
 
 /* nl_table locking explained:
  * Lookup and traversal are protected with an RCU read-side lock. Insertion
@@ -1981,7 +1981,7 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 
 	if (nlk->cb_running &&
 	    atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf / 2) {
-		ret = netlink_dump(sk);
+		ret = netlink_dump(sk, NULL);
 		if (ret) {
 			sk->sk_err = -ret;
 			sk->sk_error_report(sk);
@@ -2168,7 +2168,7 @@ EXPORT_SYMBOL(__nlmsg_put);
  * It would be better to create kernel thread.
  */
 
-static int netlink_dump(struct sock *sk)
+static int netlink_dump(struct sock *sk, struct netlink_ext_ack *extack)
 {
 	struct netlink_sock *nlk = nlk_sk(sk);
 	struct netlink_callback *cb;
@@ -2222,8 +2222,11 @@ static int netlink_dump(struct sock *sk)
 	skb_reserve(skb, skb_tailroom(skb) - alloc_size);
 	netlink_skb_set_owner_r(skb, sk);
 
-	if (nlk->dump_done_errno > 0)
+	if (nlk->dump_done_errno > 0) {
+		cb->extack = extack;
 		nlk->dump_done_errno = cb->dump(skb, cb);
+		cb->extack = NULL;
+	}
 
 	if (nlk->dump_done_errno > 0 ||
 	    skb_tailroom(skb) < nlmsg_total_size(sizeof(nlk->dump_done_errno))) {
@@ -2246,6 +2249,12 @@ static int netlink_dump(struct sock *sk)
 	memcpy(nlmsg_data(nlh), &nlk->dump_done_errno,
 	       sizeof(nlk->dump_done_errno));
 
+	if (extack && extack->_msg && nlk->flags & NETLINK_F_EXT_ACK) {
+		nlh->nlmsg_flags |= NLM_F_ACK_TLVS;
+		if (!nla_put_string(skb, NLMSGERR_ATTR_MSG, extack->_msg))
+			nlmsg_end(skb, nlh);
+	}
+
 	if (sk_filter(sk, skb))
 		kfree_skb(skb);
 	else
@@ -2307,6 +2316,7 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
 	cb->module = control->module;
 	cb->min_dump_alloc = control->min_dump_alloc;
 	cb->skb = skb;
+	cb->extack = control->extack;
 
 	if (control->start) {
 		ret = control->start(cb);
@@ -2319,7 +2329,7 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
 
 	mutex_unlock(nlk->cb_mutex);
 
-	ret = netlink_dump(sk);
+	ret = netlink_dump(sk, cb->extack);
 
 	sock_put(sk);
 
-- 
2.11.0

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

* [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 01/25] net/netlink: Pass extack to dump callbacks David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02 10:54   ` Jiri Benc
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag David Ahern
                   ` (23 subsequent siblings)
  25 siblings, 1 reply; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Pull the inet6_fill_args arg up to in6_dump_addrs and move netnsid
into it. Since IFA_TARGET_NETNSID is a kernel side filter add the
NLM_F_DUMP_FILTERED flag so userspace knows the request was honored.

Signed-off-by: David Ahern <dsahern@gmail.com>
Acked-by: Christian Brauner <christian@brauner.io>
---
 net/ipv6/addrconf.c | 59 +++++++++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index a9a317322388..375ea9d9869b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4793,12 +4793,19 @@ static inline int inet6_ifaddr_msgsize(void)
 	       + nla_total_size(4)  /* IFA_RT_PRIORITY */;
 }
 
+enum addr_type_t {
+	UNICAST_ADDR,
+	MULTICAST_ADDR,
+	ANYCAST_ADDR,
+};
+
 struct inet6_fill_args {
 	u32 portid;
 	u32 seq;
 	int event;
 	unsigned int flags;
 	int netnsid;
+	enum addr_type_t type;
 };
 
 static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
@@ -4930,39 +4937,28 @@ static int inet6_fill_ifacaddr(struct sk_buff *skb, struct ifacaddr6 *ifaca,
 	return 0;
 }
 
-enum addr_type_t {
-	UNICAST_ADDR,
-	MULTICAST_ADDR,
-	ANYCAST_ADDR,
-};
-
 /* called with rcu_read_lock() */
 static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
-			  struct netlink_callback *cb, enum addr_type_t type,
-			  int s_ip_idx, int *p_ip_idx, int netnsid)
+			  struct netlink_callback *cb,
+			  int s_ip_idx, int *p_ip_idx,
+			  struct inet6_fill_args *fillargs)
 {
-	struct inet6_fill_args fillargs = {
-		.portid = NETLINK_CB(cb->skb).portid,
-		.seq = cb->nlh->nlmsg_seq,
-		.flags = NLM_F_MULTI,
-		.netnsid = netnsid,
-	};
 	struct ifmcaddr6 *ifmca;
 	struct ifacaddr6 *ifaca;
 	int err = 1;
 	int ip_idx = *p_ip_idx;
 
 	read_lock_bh(&idev->lock);
-	switch (type) {
+	switch (fillargs->type) {
 	case UNICAST_ADDR: {
 		struct inet6_ifaddr *ifa;
-		fillargs.event = RTM_NEWADDR;
+		fillargs->event = RTM_NEWADDR;
 
 		/* unicast address incl. temp addr */
 		list_for_each_entry(ifa, &idev->addr_list, if_list) {
 			if (++ip_idx < s_ip_idx)
 				continue;
-			err = inet6_fill_ifaddr(skb, ifa, &fillargs);
+			err = inet6_fill_ifaddr(skb, ifa, fillargs);
 			if (err < 0)
 				break;
 			nl_dump_check_consistent(cb, nlmsg_hdr(skb));
@@ -4970,26 +4966,26 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
 		break;
 	}
 	case MULTICAST_ADDR:
-		fillargs.event = RTM_GETMULTICAST;
+		fillargs->event = RTM_GETMULTICAST;
 
 		/* multicast address */
 		for (ifmca = idev->mc_list; ifmca;
 		     ifmca = ifmca->next, ip_idx++) {
 			if (ip_idx < s_ip_idx)
 				continue;
-			err = inet6_fill_ifmcaddr(skb, ifmca, &fillargs);
+			err = inet6_fill_ifmcaddr(skb, ifmca, fillargs);
 			if (err < 0)
 				break;
 		}
 		break;
 	case ANYCAST_ADDR:
-		fillargs.event = RTM_GETANYCAST;
+		fillargs->event = RTM_GETANYCAST;
 		/* anycast address */
 		for (ifaca = idev->ac_list; ifaca;
 		     ifaca = ifaca->aca_next, ip_idx++) {
 			if (ip_idx < s_ip_idx)
 				continue;
-			err = inet6_fill_ifacaddr(skb, ifaca, &fillargs);
+			err = inet6_fill_ifacaddr(skb, ifaca, fillargs);
 			if (err < 0)
 				break;
 		}
@@ -5005,10 +5001,16 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
 static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 			   enum addr_type_t type)
 {
+	struct inet6_fill_args fillargs = {
+		.portid = NETLINK_CB(cb->skb).portid,
+		.seq = cb->nlh->nlmsg_seq,
+		.flags = NLM_F_MULTI,
+		.netnsid = -1,
+		.type = type,
+	};
 	struct net *net = sock_net(skb->sk);
 	struct nlattr *tb[IFA_MAX+1];
 	struct net *tgt_net = net;
-	int netnsid = -1;
 	int h, s_h;
 	int idx, ip_idx;
 	int s_idx, s_ip_idx;
@@ -5023,11 +5025,14 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 	if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
 			ifa_ipv6_policy, NULL) >= 0) {
 		if (tb[IFA_TARGET_NETNSID]) {
-			netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
+			fillargs.netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
 
-			tgt_net = rtnl_get_net_ns_capable(skb->sk, netnsid);
+			tgt_net = rtnl_get_net_ns_capable(skb->sk,
+							  fillargs.netnsid);
 			if (IS_ERR(tgt_net))
 				return PTR_ERR(tgt_net);
+
+			fillargs.flags |= NLM_F_DUMP_FILTERED;
 		}
 	}
 
@@ -5046,8 +5051,8 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 			if (!idev)
 				goto cont;
 
-			if (in6_dump_addrs(idev, skb, cb, type,
-					   s_ip_idx, &ip_idx, netnsid) < 0)
+			if (in6_dump_addrs(idev, skb, cb, s_ip_idx, &ip_idx,
+					   &fillargs) < 0)
 				goto done;
 cont:
 			idx++;
@@ -5058,7 +5063,7 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 	cb->args[0] = h;
 	cb->args[1] = idx;
 	cb->args[2] = ip_idx;
-	if (netnsid >= 0)
+	if (fillargs.netnsid >= 0)
 		put_net(tgt_net);
 
 	return skb->len;
-- 
2.11.0

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

* [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 01/25] net/netlink: Pass extack to dump callbacks David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02 11:06   ` Jiri Benc
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 04/25] net/ipv4: Update inet_dump_ifaddr to support NLM_F_DUMP_PROPER_HDR David Ahern
                   ` (22 subsequent siblings)
  25 siblings, 1 reply; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Add a new flag, NLM_F_DUMP_PROPER_HDR, for userspace to indicate to the
kernel that it believes it is sending the right header struct for the
dump message type (ifinfomsg, ifaddrmsg, rtmsg, fib_rule_hdr, ...).

Setting the flag in the netlink message header indicates to the kernel
it should do rigid checking on all data passed in the dump request and
filter the data returned based on data passed in.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/uapi/linux/netlink.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index 776bc92e9118..e722bed88dee 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -57,6 +57,7 @@ struct nlmsghdr {
 #define NLM_F_ECHO		0x08	/* Echo this request 		*/
 #define NLM_F_DUMP_INTR		0x10	/* Dump was inconsistent due to sequence change */
 #define NLM_F_DUMP_FILTERED	0x20	/* Dump was filtered as requested */
+#define NLM_F_DUMP_PROPER_HDR	0x40	/* Dump request has the proper header for type */
 
 /* Modifiers to GET request */
 #define NLM_F_ROOT	0x100	/* specify tree	root	*/
-- 
2.11.0

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

* [PATCH RFC v2 net-next 04/25] net/ipv4: Update inet_dump_ifaddr to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (2 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 05/25] net/ipv6: Update inet6_dump_addr " David Ahern
                   ` (21 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update inet_dump_ifaddr to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ifaddrmsg struct as the header potentially followed by one or more
attributes. Any data passed in the header or as an attribute is taken as
a request to influence the data returned. Only values suppored by the
dump handler are allowed to be non-0 or set in the request. At the moment
only the IFA_TARGET_NETNSID attribute is supported. Follow on patches
will support for other fields (e.g., honor ifa_index and only return data
for the given device index).

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/devinet.c | 52 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 44d931a3cd50..c27537f568f0 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1661,15 +1661,15 @@ static int inet_fill_ifaddr(struct sk_buff *skb, struct in_ifaddr *ifa,
 
 static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct netlink_ext_ack *extack = cb->extack;
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct inet_fill_args fillargs = {
 		.portid = NETLINK_CB(cb->skb).portid,
-		.seq = cb->nlh->nlmsg_seq,
+		.seq = nlh->nlmsg_seq,
 		.event = RTM_NEWADDR,
-		.flags = NLM_F_MULTI,
 		.netnsid = -1,
 	};
 	struct net *net = sock_net(skb->sk);
-	struct nlattr *tb[IFA_MAX+1];
 	struct net *tgt_net = net;
 	int h, s_h;
 	int idx, s_idx;
@@ -1683,15 +1683,45 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 	s_idx = idx = cb->args[1];
 	s_ip_idx = ip_idx = cb->args[2];
 
-	if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
-			ifa_ipv4_policy, NULL) >= 0) {
-		if (tb[IFA_TARGET_NETNSID]) {
-			fillargs.netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		struct nlattr *tb[IFA_MAX+1];
+		struct ifaddrmsg *ifm;
+		int err, i;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		ifm = nlmsg_data(nlh);
+		if (ifm->ifa_prefixlen || ifm->ifa_flags || ifm->ifa_scope) {
+			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+			return -EINVAL;
+		}
+		if (ifm->ifa_index) {
+			NL_SET_ERR_MSG(extack, "Filter by device index not supported");
+			return -EINVAL;
+		}
 
-			tgt_net = rtnl_get_net_ns_capable(skb->sk,
-							  fillargs.netnsid);
-			if (IS_ERR(tgt_net))
-				return PTR_ERR(tgt_net);
+		err = nlmsg_parse(nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
+				  ifa_ipv4_policy, extack);
+		if (err < 0)
+			return err;
+
+		for (i = 0; i <= IFA_MAX; ++i) {
+			if (i == IFA_TARGET_NETNSID) {
+				fillargs.netnsid = nla_get_s32(tb[i]);
+
+				tgt_net = rtnl_get_net_ns_capable(skb->sk,
+								  fillargs.netnsid);
+				if (IS_ERR(tgt_net))
+					return PTR_ERR(tgt_net);
+
+				fillargs.flags |= NLM_F_DUMP_FILTERED;
+			} else if (tb[i]) {
+				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
+				return -EINVAL;
+			}
 		}
 	}
 
-- 
2.11.0

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

* [PATCH RFC v2 net-next 05/25] net/ipv6: Update inet6_dump_addr to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (3 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 04/25] net/ipv4: Update inet_dump_ifaddr to support NLM_F_DUMP_PROPER_HDR David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 06/25] rtnetlink: Update rtnl_dump_ifinfo " David Ahern
                   ` (20 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update inet6_dump_addr to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ifaddrmsg struct as the header potentially followed by one or more
attributes. Any data passed in the header or as an attribute is taken as
a request to influence the data returned. Only values suppored by the
dump handler are allowed to be non-0 or set in the request. At the moment
only the IFA_TARGET_NETNSID attribute is supported. Follow on patches
will support for other fields (e.g., honor ifa_index and only return data
for the given device index).

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv6/addrconf.c | 49 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 375ea9d9869b..3382737df2a8 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5001,6 +5001,8 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
 static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 			   enum addr_type_t type)
 {
+	struct netlink_ext_ack *extack = cb->extack;
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct inet6_fill_args fillargs = {
 		.portid = NETLINK_CB(cb->skb).portid,
 		.seq = cb->nlh->nlmsg_seq,
@@ -5009,7 +5011,6 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 		.type = type,
 	};
 	struct net *net = sock_net(skb->sk);
-	struct nlattr *tb[IFA_MAX+1];
 	struct net *tgt_net = net;
 	int h, s_h;
 	int idx, ip_idx;
@@ -5022,17 +5023,45 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 	s_idx = idx = cb->args[1];
 	s_ip_idx = ip_idx = cb->args[2];
 
-	if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
-			ifa_ipv6_policy, NULL) >= 0) {
-		if (tb[IFA_TARGET_NETNSID]) {
-			fillargs.netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		struct nlattr *tb[IFA_MAX+1];
+		struct ifaddrmsg *ifm;
+		int err, i;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		ifm = nlmsg_data(nlh);
+		if (ifm->ifa_prefixlen || ifm->ifa_flags || ifm->ifa_scope) {
+			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+			return -EINVAL;
+		}
+		if (ifm->ifa_index) {
+			NL_SET_ERR_MSG(extack, "Filter by device index not supported");
+			return -EINVAL;
+		}
+
+		err = nlmsg_parse(nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
+				  ifa_ipv6_policy, extack);
+		if (err < 0)
+			return err;
+
+		for (i = 0; i <= IFA_MAX; ++i) {
+			if (i == IFA_TARGET_NETNSID) {
+				fillargs.netnsid = nla_get_s32(tb[i]);
 
-			tgt_net = rtnl_get_net_ns_capable(skb->sk,
-							  fillargs.netnsid);
-			if (IS_ERR(tgt_net))
-				return PTR_ERR(tgt_net);
+				tgt_net = rtnl_get_net_ns_capable(skb->sk,
+								  fillargs.netnsid);
+				if (IS_ERR(tgt_net))
+					return PTR_ERR(tgt_net);
 
-			fillargs.flags |= NLM_F_DUMP_FILTERED;
+				fillargs.flags |= NLM_F_DUMP_FILTERED;
+			} else if (tb[i]) {
+				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
+				return -EINVAL;
+			}
 		}
 	}
 
-- 
2.11.0

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

* [PATCH RFC v2 net-next 06/25] rtnetlink: Update rtnl_dump_ifinfo to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (4 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 05/25] net/ipv6: Update inet6_dump_addr " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 07/25] rtnetlink: Update rtnl_bridge_getlink " David Ahern
                   ` (19 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update rtnl_dump_ifinfo to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ifinfomsg struct as the header potentially followed by one or more
attributes. Any data passed in the header or as an attribute is taken as
a request to influence the data returned. Only values supported by the
dump handler are allowed to be non-0 or set in the request. At the moment
only the IFA_TARGET_NETNSID, IFLA_EXT_MASK, IFLA_MASTER, and IFLA_LINKINFO
attributes are supported.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/rtnetlink.c | 106 ++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 75 insertions(+), 31 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index da91b38297d3..2bf4b9916ca2 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1880,6 +1880,9 @@ EXPORT_SYMBOL_GPL(rtnl_get_net_ns_capable);
 
 static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct netlink_ext_ack *extack = cb->extack;
+	const struct nlmsghdr *nlh = cb->nlh;
+	bool proper_hdr = !!(nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR);
 	struct net *net = sock_net(skb->sk);
 	struct net *tgt_net = net;
 	int h, s_h;
@@ -1892,46 +1895,88 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
 	unsigned int flags = NLM_F_MULTI;
 	int master_idx = 0;
 	int netnsid = -1;
-	int err;
+	int err, i;
 	int hdrlen;
 
 	s_h = cb->args[0];
 	s_idx = cb->args[1];
 
-	/* A hack to preserve kernel<->userspace interface.
-	 * The correct header is ifinfomsg. It is consistent with rtnl_getlink.
-	 * However, before Linux v3.9 the code here assumed rtgenmsg and that's
-	 * what iproute2 < v3.9.0 used.
-	 * We can detect the old iproute2. Even including the IFLA_EXT_MASK
-	 * attribute, its netlink message is shorter than struct ifinfomsg.
-	 */
-	hdrlen = nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg) ?
-		 sizeof(struct rtgenmsg) : sizeof(struct ifinfomsg);
+	if (proper_hdr) {
+		struct ifinfomsg *ifm;
 
-	if (nlmsg_parse(cb->nlh, hdrlen, tb, IFLA_MAX,
-			ifla_policy, NULL) >= 0) {
-		if (tb[IFLA_TARGET_NETNSID]) {
-			netnsid = nla_get_s32(tb[IFLA_TARGET_NETNSID]);
-			tgt_net = rtnl_get_net_ns_capable(skb->sk, netnsid);
-			if (IS_ERR(tgt_net)) {
-				tgt_net = net;
-				netnsid = -1;
-			}
+		hdrlen = sizeof(*ifm);
+		if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen)) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
 		}
 
-		if (tb[IFLA_EXT_MASK])
-			ext_filter_mask = nla_get_u32(tb[IFLA_EXT_MASK]);
-
-		if (tb[IFLA_MASTER])
-			master_idx = nla_get_u32(tb[IFLA_MASTER]);
-
-		if (tb[IFLA_LINKINFO])
-			kind_ops = linkinfo_to_kind_ops(tb[IFLA_LINKINFO]);
+		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 dump request");
+			return -EINVAL;
+		}
+		if (ifm->ifi_index) {
+			NL_SET_ERR_MSG(extack, "Filter by device index not supported");
+			return -EINVAL;
+		}
+	} else {
+		/* A hack to preserve kernel<->userspace interface.
+		 * The correct header is ifinfomsg. It is consistent with
+		 * rtnl_getlink. However, before Linux v3.9 the code here
+		 * assumed rtgenmsg and that's what iproute2 < v3.9.0 used.
+		 * We can detect the old iproute2. Even including the
+		 * IFLA_EXT_MASK attribute, its netlink message is shorter
+		 * than struct ifinfomsg.
+		 */
+		hdrlen = nlmsg_len(nlh) < sizeof(struct ifinfomsg) ?
+			 sizeof(struct rtgenmsg) : sizeof(struct ifinfomsg);
+	}
 
-		if (master_idx || kind_ops)
-			flags |= NLM_F_DUMP_FILTERED;
+	err = nlmsg_parse(nlh, hdrlen, tb, IFLA_MAX, ifla_policy, extack);
+	if (err < 0) {
+		if (proper_hdr) {
+			NL_SET_ERR_MSG(extack, "Failed to parse link attributes");
+			return -EINVAL;
+		}
+		goto walk_entries;
+	}
+
+	for (i = 0; i <= IFLA_MAX; ++i) {
+		switch (i) {
+		case IFLA_TARGET_NETNSID:
+			if (tb[i]) {
+				netnsid = nla_get_s32(tb[i]);
+				tgt_net = rtnl_get_net_ns_capable(skb->sk,
+								  netnsid);
+				if (IS_ERR(tgt_net))
+					return PTR_ERR(tgt_net);
+			}
+			break;
+		case IFLA_EXT_MASK:
+			if (tb[i])
+				ext_filter_mask = nla_get_u32(tb[i]);
+			break;
+		case IFLA_MASTER:
+			if (tb[i])
+				master_idx = nla_get_u32(tb[i]);
+			break;
+		case IFLA_LINKINFO:
+			if (tb[i])
+				kind_ops = linkinfo_to_kind_ops(tb[i]);
+			break;
+		default:
+			if (proper_hdr && tb[i]) {
+				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
+				return -EINVAL;
+			}
+		}
 	}
 
+	if (master_idx || kind_ops || netnsid >= 0)
+		flags |= NLM_F_DUMP_FILTERED;
+
+walk_entries:
 	for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
 		idx = 0;
 		head = &tgt_net->dev_index_head[h];
@@ -1943,8 +1988,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
 			err = rtnl_fill_ifinfo(skb, dev, net,
 					       RTM_NEWLINK,
 					       NETLINK_CB(cb->skb).portid,
-					       cb->nlh->nlmsg_seq, 0,
-					       flags,
+					       nlh->nlmsg_seq, 0, flags,
 					       ext_filter_mask, 0, NULL, 0,
 					       netnsid);
 
-- 
2.11.0

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

* [PATCH RFC v2 net-next 07/25] rtnetlink: Update rtnl_bridge_getlink to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (5 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 06/25] rtnetlink: Update rtnl_dump_ifinfo " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 08/25] rtnetlink: Update rtnl_stats_dump " David Ahern
                   ` (18 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update rtnl_bridge_getlink to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ifinfomsg struct as the header potentially followed by one or more
attributes. Any data passed in the header or as an attribute is taken as
a request to influence the data returned. Only values supported by the
dump handler are allowed to be non-0 or set in the request. At the moment
only the IFLA_EXT_MASK attribute is supported.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/rtnetlink.c | 56 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 2bf4b9916ca2..51a653b810be 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3999,27 +3999,63 @@ EXPORT_SYMBOL_GPL(ndo_dflt_bridge_getlink);
 
 static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct netlink_ext_ack *extack = cb->extack;
+	const struct nlmsghdr *nlh = cb->nlh;
+	bool proper_hdr = !!(nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR);
 	struct net *net = sock_net(skb->sk);
+	struct nlattr *tb[IFLA_MAX+1];
 	struct net_device *dev;
 	int idx = 0;
 	u32 portid = NETLINK_CB(cb->skb).portid;
-	u32 seq = cb->nlh->nlmsg_seq;
+	u32 seq = nlh->nlmsg_seq;
 	u32 filter_mask = 0;
-	int err;
+	int err, i;
 
-	if (nlmsg_len(cb->nlh) > sizeof(struct ifinfomsg)) {
-		struct nlattr *extfilt;
+	if (proper_hdr) {
+		struct ifinfomsg *ifm;
 
-		extfilt = nlmsg_find_attr(cb->nlh, sizeof(struct ifinfomsg),
-					  IFLA_EXT_MASK);
-		if (extfilt) {
-			if (nla_len(extfilt) < sizeof(filter_mask))
-				return -EINVAL;
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
 
-			filter_mask = nla_get_u32(extfilt);
+		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 dump request");
+			return -EINVAL;
+		}
+		if (ifm->ifi_index) {
+			NL_SET_ERR_MSG(extack, "Filter by device index not supported");
+			return -EINVAL;
 		}
 	}
 
+	err = nlmsg_parse(nlh, sizeof(struct ifinfomsg), tb, IFLA_MAX,
+			  ifla_policy, extack);
+	if (err < 0) {
+		if (proper_hdr) {
+			NL_SET_ERR_MSG(extack, "Failed to parse link attributes");
+			return -EINVAL;
+		}
+		goto walk_entries;
+	}
+
+	for (i = 0; i <= IFLA_MAX; ++i) {
+		switch (i) {
+		case IFLA_EXT_MASK:
+			if (tb[i])
+				filter_mask = nla_get_u32(tb[i]);
+			break;
+		default:
+			if (proper_hdr && tb[i]) {
+				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
+				return -EINVAL;
+			}
+		}
+	}
+
+walk_entries:
 	rcu_read_lock();
 	for_each_netdev_rcu(net, dev) {
 		const struct net_device_ops *ops = dev->netdev_ops;
-- 
2.11.0

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

* [PATCH RFC v2 net-next 08/25] rtnetlink: Update rtnl_stats_dump to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (6 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 07/25] rtnetlink: Update rtnl_bridge_getlink " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 09/25] rtnetlink: Update inet6_dump_ifinfo " David Ahern
                   ` (17 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update rtnl_stats_dump to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an if_stats_msg struct as the header. All elements of the struct are
expected to be 0 except filter_mask which must be non-0 (legacy behavior).
No attributes are supported.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/rtnetlink.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 51a653b810be..1751baf0c823 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4648,6 +4648,9 @@ static int rtnl_stats_get(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct netlink_ext_ack *extack = cb->extack;
+	const struct nlmsghdr *nlh = cb->nlh;
+	bool proper_hdr = !!(nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR);
 	int h, s_h, err, s_idx, s_idxattr, s_prividx;
 	struct net *net = sock_net(skb->sk);
 	unsigned int flags = NLM_F_MULTI;
@@ -4668,9 +4671,26 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
 		return -EINVAL;
 
 	ifsm = nlmsg_data(cb->nlh);
+
+	/* only requests using NLM_F_DUMP_PROPER_HDR can pass data to
+	 * influence the dump. The legacy exception is filter_mask.
+	 */
+	if (proper_hdr) {
+		if (ifsm->pad1 || ifsm->pad2 || ifsm->ifindex) {
+			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+			return -EINVAL;
+		}
+		if (nlmsg_len(cb->nlh) != nlmsg_msg_size(sizeof(*ifsm))) {
+			NL_SET_ERR_MSG(extack, "Invalid attributes after header");
+			return -EINVAL;
+		}
+	}
+
 	filter_mask = ifsm->filter_mask;
-	if (!filter_mask)
+	if (!filter_mask) {
+		NL_SET_ERR_MSG(extack, "Invalid filter_mask in header");
 		return -EINVAL;
+	}
 
 	for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
 		idx = 0;
-- 
2.11.0

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

* [PATCH RFC v2 net-next 09/25] rtnetlink: Update inet6_dump_ifinfo to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (7 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 08/25] rtnetlink: Update rtnl_stats_dump " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 10/25] rtnetlink: Update ipmr_rtm_dumplink " David Ahern
                   ` (16 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update inet6_dump_ifinfo to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ifinfomsg struct as the header. All elements of the struct are
expected to be 0 and no attributes can be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv6/addrconf.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3382737df2a8..eb6fd5fbac80 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5626,8 +5626,34 @@ static int inet6_fill_ifinfo(struct sk_buff *skb, struct inet6_dev *idev,
 	return -EMSGSIZE;
 }
 
+static int inet6_valid_dump_ifinfo(const struct nlmsghdr *nlh,
+				   struct netlink_ext_ack *extack)
+{
+	struct ifinfomsg *ifm;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
+		NL_SET_ERR_MSG(extack, "Invalid header");
+		return -EINVAL;
+	}
+
+	if (nlh->nlmsg_len > nlmsg_msg_size(sizeof(*ifm))) {
+		NL_SET_ERR_MSG(extack, "Invalid data after header");
+		return -EINVAL;
+	}
+
+	ifm = nlmsg_data(nlh);
+	if (ifm->__ifi_pad || ifm->ifi_type || ifm->ifi_flags ||
+	    ifm->ifi_change || ifm->ifi_index) {
+		NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int inet6_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	int h, s_h;
 	int idx = 0, s_idx;
@@ -5635,6 +5661,16 @@ static int inet6_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
 	struct inet6_dev *idev;
 	struct hlist_head *head;
 
+	/* only requests using NLM_F_DUMP_PROPER_HDR can pass data to
+	 * influence the dump
+	 */
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		int err = inet6_valid_dump_ifinfo(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	s_h = cb->args[0];
 	s_idx = cb->args[1];
 
@@ -5650,7 +5686,7 @@ static int inet6_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
 				goto cont;
 			if (inet6_fill_ifinfo(skb, idev,
 					      NETLINK_CB(cb->skb).portid,
-					      cb->nlh->nlmsg_seq,
+					      nlh->nlmsg_seq,
 					      RTM_NEWLINK, NLM_F_MULTI) < 0)
 				goto out;
 cont:
-- 
2.11.0

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

* [PATCH RFC v2 net-next 10/25] rtnetlink: Update ipmr_rtm_dumplink to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (8 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 09/25] rtnetlink: Update inet6_dump_ifinfo " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 11/25] rtnetlink: Update fib dumps " David Ahern
                   ` (15 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update ipmr_rtm_dumplink to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ifinfomsg struct as the header. All elements of the struct are
expected to be 0 and no attributes can be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/ipmr.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 5660adcf7a04..a706e9269e8c 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2710,6 +2710,31 @@ static bool ipmr_fill_vif(struct mr_table *mrt, u32 vifid, struct sk_buff *skb)
 	return true;
 }
 
+static int ipmr_valid_dumplink(const struct nlmsghdr *nlh,
+			       struct netlink_ext_ack *extack)
+{
+	struct ifinfomsg *ifm;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
+		NL_SET_ERR_MSG(extack, "Invalid header");
+		return -EINVAL;
+	}
+
+	if (nlh->nlmsg_len > nlmsg_msg_size(sizeof(*ifm))) {
+		NL_SET_ERR_MSG(extack, "Invalid data after header");
+		return -EINVAL;
+	}
+
+	ifm = nlmsg_data(nlh);
+	if (ifm->__ifi_pad || ifm->ifi_type || ifm->ifi_flags ||
+	    ifm->ifi_change || ifm->ifi_index) {
+		NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int ipmr_rtm_dumplink(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct net *net = sock_net(skb->sk);
@@ -2718,6 +2743,13 @@ static int ipmr_rtm_dumplink(struct sk_buff *skb, struct netlink_callback *cb)
 	unsigned int e = 0, s_e;
 	struct mr_table *mrt;
 
+	if (cb->nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		int err = ipmr_valid_dumplink(cb->nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	s_t = cb->args[0];
 	s_e = cb->args[1];
 
-- 
2.11.0

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

* [PATCH RFC v2 net-next 11/25] rtnetlink: Update fib dumps to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (9 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 10/25] rtnetlink: Update ipmr_rtm_dumplink " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 12/25] net/neigh: Refactor dump filter handling David Ahern
                   ` (14 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Add helper to check netlink message for route dumps. The dump request
is expected to have an rtmsg struct as the header. All elements of the
struct are expected to be 0 with the exception of rtm_flags (which is
used by both ipv4 and ipv6 dumps) and with no attributes can be appended.

Update inet_dump_fib, inet6_dump_fib, mpls_dump_routes, ipmr_rtm_dumproute,
and ip6mr_rtm_dumproute to call this helper if NLM_F_DUMP_PROPER_HDR is
set in the netlink message header.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/ip_fib.h    |  2 ++
 net/ipv4/fib_frontend.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 net/ipv4/ipmr.c         |  9 +++++++++
 net/ipv6/ip6_fib.c      |  8 ++++++++
 net/ipv6/ip6mr.c        |  9 +++++++++
 net/mpls/af_mpls.c      |  8 ++++++++
 6 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index f7c109e37298..9846b79c9ee1 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -452,4 +452,6 @@ static inline void fib_proc_exit(struct net *net)
 
 u32 ip_mtu_from_fib_result(struct fib_result *res, __be32 daddr);
 
+int ip_valid_fib_dump_req(const struct nlmsghdr *nlh,
+			  struct netlink_ext_ack *extack);
 #endif  /* _NET_FIB_H */
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 30e2bcc3ef2a..c608b393ae49 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -802,8 +802,41 @@ static int inet_rtm_newroute(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return err;
 }
 
+int ip_valid_fib_dump_req(const struct nlmsghdr *nlh,
+			  struct netlink_ext_ack *extack)
+{
+	struct rtmsg *rtm;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) {
+		NL_SET_ERR_MSG(extack, "Invalid header");
+		return -EINVAL;
+	}
+
+	rtm = nlmsg_data(nlh);
+	if (rtm->rtm_dst_len || rtm->rtm_src_len  || rtm->rtm_tos   ||
+	    rtm->rtm_table   || rtm->rtm_protocol || rtm->rtm_scope ||
+	    rtm->rtm_type) {
+		NL_SET_ERR_MSG(extack,
+			       "Invalid values in header for dump request");
+		return -EINVAL;
+	}
+
+	if (rtm->rtm_flags & ~(RTM_F_CLONED | RTM_F_PREFIX)) {
+		NL_SET_ERR_MSG(extack, "Invalid flags for dump request");
+		return -EINVAL;
+	}
+	if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*rtm))) {
+		NL_SET_ERR_MSG(extack, "Invalid data after header");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ip_valid_fib_dump_req);
+
 static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	unsigned int h, s_h;
 	unsigned int e = 0, s_e;
@@ -811,8 +844,14 @@ static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 	struct hlist_head *head;
 	int dumped = 0, err;
 
-	if (nlmsg_len(cb->nlh) >= sizeof(struct rtmsg) &&
-	    ((struct rtmsg *) nlmsg_data(cb->nlh))->rtm_flags & RTM_F_CLONED)
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		err = ip_valid_fib_dump_req(nlh, cb->extack);
+		if (err)
+			return err;
+	}
+
+	if (nlmsg_len(nlh) >= sizeof(struct rtmsg) &&
+	    ((struct rtmsg *)nlmsg_data(nlh))->rtm_flags & RTM_F_CLONED)
 		return skb->len;
 
 	s_h = cb->args[0];
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index a706e9269e8c..91b5991ed536 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2527,6 +2527,15 @@ static int ipmr_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 
 static int ipmr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
+
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	return mr_rtm_dumproute(skb, cb, ipmr_mr_table_iter,
 				_ipmr_fill_mroute, &mfc_unres_lock);
 }
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 5516f55e214b..fc14733fbad8 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -568,6 +568,7 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
 
 static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	unsigned int h, s_h;
 	unsigned int e = 0, s_e;
@@ -577,6 +578,13 @@ static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 	struct hlist_head *head;
 	int res = 0;
 
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	s_h = cb->args[0];
 	s_e = cb->args[1];
 
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index d0b7e0249c13..aa668214edc2 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -2433,6 +2433,15 @@ static void mrt6msg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt)
 
 static int ip6mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
+
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	return mr_rtm_dumproute(skb, cb, ip6mr_mr_table_iter,
 				_ip6mr_fill_mroute, &mfc_unres_lock);
 }
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 8fbe6cdbe255..812100a5a906 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -2017,6 +2017,7 @@ static int mpls_dump_route(struct sk_buff *skb, u32 portid, u32 seq, int event,
 
 static int mpls_dump_routes(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct mpls_route __rcu **platform_label;
 	size_t platform_labels;
@@ -2024,6 +2025,13 @@ static int mpls_dump_routes(struct sk_buff *skb, struct netlink_callback *cb)
 
 	ASSERT_RTNL();
 
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	index = cb->args[0];
 	if (index < MPLS_LABEL_FIRST_UNRESERVED)
 		index = MPLS_LABEL_FIRST_UNRESERVED;
-- 
2.11.0

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

* [PATCH RFC v2 net-next 12/25] net/neigh: Refactor dump filter handling
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (10 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 11/25] rtnetlink: Update fib dumps " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 13/25] net/neighbor: Update neigh_dump_info to support NLM_F_DUMP_PROPER_HDR David Ahern
                   ` (13 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Move the attribute parsing from neigh_dump_table to neigh_dump_info, and
pass the filter arguments down to neigh_dump_table in a new struct. Add
the filter option to proxy neigh dumps as well to make the dumps consistent.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/neighbour.c | 65 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 28 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 20e0d3308148..9bab9ae9c98e 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2329,35 +2329,24 @@ static bool neigh_ifindex_filtered(struct net_device *dev, int filter_idx)
 	return false;
 }
 
+struct neigh_dump_filter {
+	int master_idx;
+	int dev_idx;
+};
+
 static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
-			    struct netlink_callback *cb)
+			    struct netlink_callback *cb,
+			    struct neigh_dump_filter *filter)
 {
 	struct net *net = sock_net(skb->sk);
-	const struct nlmsghdr *nlh = cb->nlh;
-	struct nlattr *tb[NDA_MAX + 1];
 	struct neighbour *n;
 	int rc, h, s_h = cb->args[1];
 	int idx, s_idx = idx = cb->args[2];
 	struct neigh_hash_table *nht;
-	int filter_master_idx = 0, filter_idx = 0;
 	unsigned int flags = NLM_F_MULTI;
-	int err;
 
-	err = nlmsg_parse(nlh, sizeof(struct ndmsg), tb, NDA_MAX, NULL, NULL);
-	if (!err) {
-		if (tb[NDA_IFINDEX]) {
-			if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
-				return -EINVAL;
-			filter_idx = nla_get_u32(tb[NDA_IFINDEX]);
-		}
-		if (tb[NDA_MASTER]) {
-			if (nla_len(tb[NDA_MASTER]) != sizeof(u32))
-				return -EINVAL;
-			filter_master_idx = nla_get_u32(tb[NDA_MASTER]);
-		}
-		if (filter_idx || filter_master_idx)
-			flags |= NLM_F_DUMP_FILTERED;
-	}
+	if (filter->dev_idx || filter->master_idx)
+		flags |= NLM_F_DUMP_FILTERED;
 
 	rcu_read_lock_bh();
 	nht = rcu_dereference_bh(tbl->nht);
@@ -2370,8 +2359,8 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
 		     n = rcu_dereference_bh(n->next)) {
 			if (idx < s_idx || !net_eq(dev_net(n->dev), net))
 				goto next;
-			if (neigh_ifindex_filtered(n->dev, filter_idx) ||
-			    neigh_master_filtered(n->dev, filter_master_idx))
+			if (neigh_ifindex_filtered(n->dev, filter->dev_idx) ||
+			    neigh_master_filtered(n->dev, filter->master_idx))
 				goto next;
 			if (neigh_fill_info(skb, n, NETLINK_CB(cb->skb).portid,
 					    cb->nlh->nlmsg_seq,
@@ -2393,7 +2382,8 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
 }
 
 static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
-			     struct netlink_callback *cb)
+			     struct netlink_callback *cb,
+			     struct neigh_dump_filter *filter)
 {
 	struct pneigh_entry *n;
 	struct net *net = sock_net(skb->sk);
@@ -2408,6 +2398,9 @@ static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
 		for (n = tbl->phash_buckets[h], idx = 0; n; n = n->next) {
 			if (idx < s_idx || pneigh_net(n) != net)
 				goto next;
+			if (neigh_ifindex_filtered(n->dev, filter->dev_idx) ||
+			    neigh_master_filtered(n->dev, filter->master_idx))
+				goto next;
 			if (pneigh_fill_info(skb, n, NETLINK_CB(cb->skb).portid,
 					    cb->nlh->nlmsg_seq,
 					    RTM_NEWNEIGH,
@@ -2432,20 +2425,36 @@ static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
 
 static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
+	struct neigh_dump_filter filter = {};
+	struct nlattr *tb[NDA_MAX + 1];
 	struct neigh_table *tbl;
 	int t, family, s_t;
 	int proxy = 0;
 	int err;
 
-	family = ((struct rtgenmsg *) nlmsg_data(cb->nlh))->rtgen_family;
+	family = ((struct rtgenmsg *)nlmsg_data(nlh))->rtgen_family;
 
 	/* check for full ndmsg structure presence, family member is
 	 * the same for both structures
 	 */
-	if (nlmsg_len(cb->nlh) >= sizeof(struct ndmsg) &&
-	    ((struct ndmsg *) nlmsg_data(cb->nlh))->ndm_flags == NTF_PROXY)
+	if (nlmsg_len(nlh) >= sizeof(struct ndmsg) &&
+	    ((struct ndmsg *)nlmsg_data(nlh))->ndm_flags == NTF_PROXY)
 		proxy = 1;
 
+	err = nlmsg_parse(nlh, sizeof(struct ndmsg), tb, NDA_MAX, NULL, NULL);
+	if (!err) {
+		if (tb[NDA_IFINDEX]) {
+			if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
+				return -EINVAL;
+			filter.dev_idx = nla_get_u32(tb[NDA_IFINDEX]);
+		}
+		if (tb[NDA_MASTER]) {
+			if (nla_len(tb[NDA_MASTER]) != sizeof(u32))
+				return -EINVAL;
+			filter.master_idx = nla_get_u32(tb[NDA_MASTER]);
+		}
+	}
 	s_t = cb->args[0];
 
 	for (t = 0; t < NEIGH_NR_TABLES; t++) {
@@ -2459,9 +2468,9 @@ static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 			memset(&cb->args[1], 0, sizeof(cb->args) -
 						sizeof(cb->args[0]));
 		if (proxy)
-			err = pneigh_dump_table(tbl, skb, cb);
+			err = pneigh_dump_table(tbl, skb, cb, &filter);
 		else
-			err = neigh_dump_table(tbl, skb, cb);
+			err = neigh_dump_table(tbl, skb, cb, &filter);
 		if (err < 0)
 			break;
 	}
-- 
2.11.0

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

* [PATCH RFC v2 net-next 13/25] net/neighbor: Update neigh_dump_info to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (11 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 12/25] net/neigh: Refactor dump filter handling David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 14/25] net/neighbor: Update neightbl_dump_info " David Ahern
                   ` (12 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update neigh_dump_info to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ndmsg struct as the header potentially followed by one or more
attributes. Any data passed in the header or as an attribute is taken as
a request to influence the data returned. Only values supported by the
dump handler are allowed to be non-0 or set in the request. At the moment
only the NDA_IFINDEX and NDA_MASTER attributes are supported.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/neighbour.c | 61 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 10 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 9bab9ae9c98e..aaf2526e5da4 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2425,13 +2425,15 @@ static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
 
 static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct netlink_ext_ack *extack = cb->extack;
 	const struct nlmsghdr *nlh = cb->nlh;
+	bool proper_hdr = !!(nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR);
 	struct neigh_dump_filter filter = {};
 	struct nlattr *tb[NDA_MAX + 1];
 	struct neigh_table *tbl;
 	int t, family, s_t;
 	int proxy = 0;
-	int err;
+	int err, i;
 
 	family = ((struct rtgenmsg *)nlmsg_data(nlh))->rtgen_family;
 
@@ -2442,19 +2444,58 @@ static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 	    ((struct ndmsg *)nlmsg_data(nlh))->ndm_flags == NTF_PROXY)
 		proxy = 1;
 
-	err = nlmsg_parse(nlh, sizeof(struct ndmsg), tb, NDA_MAX, NULL, NULL);
-	if (!err) {
-		if (tb[NDA_IFINDEX]) {
-			if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
-				return -EINVAL;
-			filter.dev_idx = nla_get_u32(tb[NDA_IFINDEX]);
+	if (proper_hdr) {
+		struct ndmsg *ndm;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ndm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		ndm = nlmsg_data(nlh);
+		if (ndm->ndm_pad1  || ndm->ndm_pad2  || ndm->ndm_ifindex ||
+		    ndm->ndm_state || ndm->ndm_flags || ndm->ndm_type) {
+			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+			return -EINVAL;
 		}
-		if (tb[NDA_MASTER]) {
-			if (nla_len(tb[NDA_MASTER]) != sizeof(u32))
+	}
+
+	err = nlmsg_parse(nlh, sizeof(struct ndmsg), tb, NDA_MAX, NULL, extack);
+	if (err < 0) {
+		if (proper_hdr) {
+			NL_SET_ERR_MSG(extack, "Failed to parse link attributes");
+			return -EINVAL;
+		}
+		goto walk_entries;
+	}
+
+	for (i = 0; i <= NDA_MAX; ++i) {
+		if (!tb[i])
+			continue;
+		switch (i) {
+		case NDA_IFINDEX:
+			if (nla_len(tb[i]) != sizeof(u32)) {
+				NL_SET_ERR_MSG(extack, "Invalid IFINDEX attribute");
+				return -EINVAL;
+			}
+			filter.dev_idx = nla_get_u32(tb[i]);
+			break;
+		case NDA_MASTER:
+			if (nla_len(tb[i]) != sizeof(u32)) {
+				NL_SET_ERR_MSG(extack, "Invalid MASTER attribute");
 				return -EINVAL;
-			filter.master_idx = nla_get_u32(tb[NDA_MASTER]);
+			}
+			filter.master_idx = nla_get_u32(tb[i]);
+			break;
+		default:
+			if (proper_hdr) {
+				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
+				return -EINVAL;
+			}
 		}
 	}
+
+walk_entries:
 	s_t = cb->args[0];
 
 	for (t = 0; t < NEIGH_NR_TABLES; t++) {
-- 
2.11.0

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

* [PATCH RFC v2 net-next 14/25] net/neighbor: Update neightbl_dump_info to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (12 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 13/25] net/neighbor: Update neigh_dump_info to support NLM_F_DUMP_PROPER_HDR David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 15/25] net/namespace: Update rtnl_net_dumpid " David Ahern
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update neightbl_dump_info to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ndtmsg struct as the header. All elements of the struct are expected to
be 0 and no attributes can be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/neighbour.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index aaf2526e5da4..8488f2e2c865 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2166,13 +2166,35 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	int family, tidx, nidx = 0;
 	int tbl_skip = cb->args[0];
 	int neigh_skip = cb->args[1];
 	struct neigh_table *tbl;
 
-	family = ((struct rtgenmsg *) nlmsg_data(cb->nlh))->rtgen_family;
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		struct netlink_ext_ack *extack = cb->extack;
+		struct ndtmsg *ndtm;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ndtm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		ndtm = nlmsg_data(nlh);
+		if (ndtm->ndtm_pad1  || ndtm->ndtm_pad2) {
+			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+			return -EINVAL;
+		}
+
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ndtm))) {
+			NL_SET_ERR_MSG(extack, "Invalid data after header");
+			return -EINVAL;
+		}
+	}
+
+	family = ((struct rtgenmsg *)nlmsg_data(nlh))->rtgen_family;
 
 	for (tidx = 0; tidx < NEIGH_NR_TABLES; tidx++) {
 		struct neigh_parms *p;
@@ -2185,7 +2207,7 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 			continue;
 
 		if (neightbl_fill_info(skb, tbl, NETLINK_CB(cb->skb).portid,
-				       cb->nlh->nlmsg_seq, RTM_NEWNEIGHTBL,
+				       nlh->nlmsg_seq, RTM_NEWNEIGHTBL,
 				       NLM_F_MULTI) < 0)
 			break;
 
@@ -2200,7 +2222,7 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 
 			if (neightbl_fill_param_info(skb, tbl, p,
 						     NETLINK_CB(cb->skb).portid,
-						     cb->nlh->nlmsg_seq,
+						     nlh->nlmsg_seq,
 						     RTM_NEWNEIGHTBL,
 						     NLM_F_MULTI) < 0)
 				goto out;
-- 
2.11.0

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

* [PATCH RFC v2 net-next 15/25] net/namespace: Update rtnl_net_dumpid to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (13 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 14/25] net/neighbor: Update neightbl_dump_info " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 16/25] net/fib_rules: Update fib_nl_dumprule " David Ahern
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update rtnl_net_dumpid to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. The dump request is expected to have an rtgenmsg struct
which has the family as the only element. No data may be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/net_namespace.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 670c84b1bfc2..0c3a978efc57 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -844,6 +844,7 @@ static int rtnl_net_dumpid_one(int id, void *peer, void *data)
 
 static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct rtnl_net_dump_cb net_cb = {
 		.net = net,
@@ -853,6 +854,13 @@ static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
 		.s_idx = cb->args[0],
 	};
 
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(struct rtgenmsg))) {
+			NL_SET_ERR_MSG(cb->extack, "Unknown data in dump request");
+			return -EINVAL;
+		}
+	}
+
 	spin_lock_bh(&net->nsid_lock);
 	idr_for_each(&net->netns_ids, rtnl_net_dumpid_one, &net_cb);
 	spin_unlock_bh(&net->nsid_lock);
-- 
2.11.0

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

* [PATCH RFC v2 net-next 16/25] net/fib_rules: Update fib_nl_dumprule to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (14 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 15/25] net/namespace: Update rtnl_net_dumpid " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 17/25] net/ipv6: Update ip6addrlbl_dump " David Ahern
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update fib_nl_dumprule to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
fib_rule_hdr struct as the header. All elements of the struct are
expected to be 0 and no attributes can be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/fib_rules.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 0ff3953f64aa..e9f9dc501b2e 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -1065,11 +1065,34 @@ static int dump_rules(struct sk_buff *skb, struct netlink_callback *cb,
 
 static int fib_nl_dumprule(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct fib_rules_ops *ops;
 	int idx = 0, family;
 
-	family = rtnl_msg_family(cb->nlh);
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		struct netlink_ext_ack *extack = cb->extack;
+		struct fib_rule_hdr *frh;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*frh))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		frh = nlmsg_data(nlh);
+		if (frh->dst_len || frh->src_len || frh->tos || frh->table ||
+		    frh->res1 || frh->res2 || frh->action || frh->flags) {
+			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+			return -EINVAL;
+		}
+
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*frh))) {
+			NL_SET_ERR_MSG(extack, "Invalid data after header");
+			return -EINVAL;
+		}
+	}
+
+	family = rtnl_msg_family(nlh);
 	if (family != AF_UNSPEC) {
 		/* Protocol specific dump request */
 		ops = lookup_rules_ops(net, family);
-- 
2.11.0

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

* [PATCH RFC v2 net-next 17/25] net/ipv6: Update ip6addrlbl_dump to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (15 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 16/25] net/fib_rules: Update fib_nl_dumprule " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 18/25] net: Update netconf dump handlers " David Ahern
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update ip6addrlbl_dump to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ifaddrlblmsg struct as the header. All elements of the struct are
expected to be 0 and no attributes can be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv6/addrlabel.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
index 1d6ced37ad71..89e15ed78c60 100644
--- a/net/ipv6/addrlabel.c
+++ b/net/ipv6/addrlabel.c
@@ -460,18 +460,41 @@ static int ip6addrlbl_fill(struct sk_buff *skb,
 
 static int ip6addrlbl_dump(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct ip6addrlbl_entry *p;
 	int idx = 0, s_idx = cb->args[0];
 	int err;
 
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		struct netlink_ext_ack *extack = cb->extack;
+		struct ifaddrlblmsg *ifal;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifal))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		ifal = nlmsg_data(nlh);
+		if (ifal->__ifal_reserved || ifal->ifal_prefixlen ||
+		    ifal->ifal_flags || ifal->ifal_index || ifal->ifal_seq) {
+			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+			return -EINVAL;
+		}
+
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ifal))) {
+			NL_SET_ERR_MSG(extack, "Invalid data after header");
+			return -EINVAL;
+		}
+	}
+
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(p, &net->ipv6.ip6addrlbl_table.head, list) {
 		if (idx >= s_idx) {
 			err = ip6addrlbl_fill(skb, p,
 					      net->ipv6.ip6addrlbl_table.seq,
 					      NETLINK_CB(cb->skb).portid,
-					      cb->nlh->nlmsg_seq,
+					      nlh->nlmsg_seq,
 					      RTM_NEWADDRLABEL,
 					      NLM_F_MULTI);
 			if (err < 0)
-- 
2.11.0

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

* [PATCH RFC v2 net-next 18/25] net: Update netconf dump handlers to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (16 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 17/25] net/ipv6: Update ip6addrlbl_dump " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 19/25] net/bridge: Update br_mdb_dump " David Ahern
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update inet_netconf_dump_devconf, inet6_netconf_dump_devconf, and
mpls_netconf_dump_devconf to check for NLM_F_DUMP_PROPER_HDR in the
netlink message header. If the flag is set, the dump request is
expected to have an netconfmsg struct as the header. The struct
only has the family member and no attributes can be appended, so
the request should only have the header.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/devinet.c  | 22 +++++++++++++++++++---
 net/ipv6/addrconf.c | 22 +++++++++++++++++++---
 net/mpls/af_mpls.c  | 18 +++++++++++++++++-
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index c27537f568f0..d7859b358cc6 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2065,6 +2065,7 @@ static int inet_netconf_get_devconf(struct sk_buff *in_skb,
 static int inet_netconf_dump_devconf(struct sk_buff *skb,
 				     struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	int h, s_h;
 	int idx, s_idx;
@@ -2072,6 +2073,21 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
 	struct in_device *in_dev;
 	struct hlist_head *head;
 
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		struct netlink_ext_ack *extack = cb->extack;
+		struct netconfmsg *ncm;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid data after header");
+			return -EINVAL;
+		}
+	}
+
 	s_h = cb->args[0];
 	s_idx = idx = cb->args[1];
 
@@ -2091,7 +2107,7 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
 			if (inet_netconf_fill_devconf(skb, dev->ifindex,
 						      &in_dev->cnf,
 						      NETLINK_CB(cb->skb).portid,
-						      cb->nlh->nlmsg_seq,
+						      nlh->nlmsg_seq,
 						      RTM_NEWNETCONF,
 						      NLM_F_MULTI,
 						      NETCONFA_ALL) < 0) {
@@ -2108,7 +2124,7 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
 		if (inet_netconf_fill_devconf(skb, NETCONFA_IFINDEX_ALL,
 					      net->ipv4.devconf_all,
 					      NETLINK_CB(cb->skb).portid,
-					      cb->nlh->nlmsg_seq,
+					      nlh->nlmsg_seq,
 					      RTM_NEWNETCONF, NLM_F_MULTI,
 					      NETCONFA_ALL) < 0)
 			goto done;
@@ -2119,7 +2135,7 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
 		if (inet_netconf_fill_devconf(skb, NETCONFA_IFINDEX_DEFAULT,
 					      net->ipv4.devconf_dflt,
 					      NETLINK_CB(cb->skb).portid,
-					      cb->nlh->nlmsg_seq,
+					      nlh->nlmsg_seq,
 					      RTM_NEWNETCONF, NLM_F_MULTI,
 					      NETCONFA_ALL) < 0)
 			goto done;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index eb6fd5fbac80..34b5daa9e977 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -666,6 +666,7 @@ static int inet6_netconf_get_devconf(struct sk_buff *in_skb,
 static int inet6_netconf_dump_devconf(struct sk_buff *skb,
 				      struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	int h, s_h;
 	int idx, s_idx;
@@ -673,6 +674,21 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
 	struct inet6_dev *idev;
 	struct hlist_head *head;
 
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		struct netlink_ext_ack *extack = cb->extack;
+		struct netconfmsg *ncm;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid data after header");
+			return -EINVAL;
+		}
+	}
+
 	s_h = cb->args[0];
 	s_idx = idx = cb->args[1];
 
@@ -692,7 +708,7 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
 			if (inet6_netconf_fill_devconf(skb, dev->ifindex,
 						       &idev->cnf,
 						       NETLINK_CB(cb->skb).portid,
-						       cb->nlh->nlmsg_seq,
+						       nlh->nlmsg_seq,
 						       RTM_NEWNETCONF,
 						       NLM_F_MULTI,
 						       NETCONFA_ALL) < 0) {
@@ -709,7 +725,7 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
 		if (inet6_netconf_fill_devconf(skb, NETCONFA_IFINDEX_ALL,
 					       net->ipv6.devconf_all,
 					       NETLINK_CB(cb->skb).portid,
-					       cb->nlh->nlmsg_seq,
+					       nlh->nlmsg_seq,
 					       RTM_NEWNETCONF, NLM_F_MULTI,
 					       NETCONFA_ALL) < 0)
 			goto done;
@@ -720,7 +736,7 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
 		if (inet6_netconf_fill_devconf(skb, NETCONFA_IFINDEX_DEFAULT,
 					       net->ipv6.devconf_dflt,
 					       NETLINK_CB(cb->skb).portid,
-					       cb->nlh->nlmsg_seq,
+					       nlh->nlmsg_seq,
 					       RTM_NEWNETCONF, NLM_F_MULTI,
 					       NETCONFA_ALL) < 0)
 			goto done;
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 812100a5a906..64de2f8c3847 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -1263,6 +1263,7 @@ static int mpls_netconf_get_devconf(struct sk_buff *in_skb,
 static int mpls_netconf_dump_devconf(struct sk_buff *skb,
 				     struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct hlist_head *head;
 	struct net_device *dev;
@@ -1270,6 +1271,21 @@ static int mpls_netconf_dump_devconf(struct sk_buff *skb,
 	int idx, s_idx;
 	int h, s_h;
 
+	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		struct netlink_ext_ack *extack = cb->extack;
+		struct netconfmsg *ncm;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid data after header");
+			return -EINVAL;
+		}
+	}
+
 	s_h = cb->args[0];
 	s_idx = idx = cb->args[1];
 
@@ -1286,7 +1302,7 @@ static int mpls_netconf_dump_devconf(struct sk_buff *skb,
 				goto cont;
 			if (mpls_netconf_fill_devconf(skb, mdev,
 						      NETLINK_CB(cb->skb).portid,
-						      cb->nlh->nlmsg_seq,
+						      nlh->nlmsg_seq,
 						      RTM_NEWNETCONF,
 						      NLM_F_MULTI,
 						      NETCONFA_ALL) < 0) {
-- 
2.11.0

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

* [PATCH RFC v2 net-next 19/25] net/bridge: Update br_mdb_dump to support NLM_F_DUMP_PROPER_HDR
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (17 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 18/25] net: Update netconf dump handlers " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 20/25] net: Add struct for fib dump filter David Ahern
                   ` (6 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update br_mdb_dump to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
a br_port_msg struct as the header. All elements of the struct are
expected to be 0 and no attributes can be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/bridge/br_mdb.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index a4a848bf827b..57c43c1b1e71 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -167,8 +167,26 @@ static int br_mdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	struct net_device *dev;
 	struct net *net = sock_net(skb->sk);
 	struct nlmsghdr *nlh = NULL;
+	struct br_port_msg *bpm;
 	int idx = 0, s_idx;
 
+	if (cb->nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+		struct netlink_ext_ack *extack = cb->extack;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*bpm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+		if (bpm->ifindex) {
+			NL_SET_ERR_MSG(extack, "Filtering by device index is not supported");
+			return -EINVAL;
+		}
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*bpm))) {
+			NL_SET_ERR_MSG(extack, "Invalid data after header");
+			return -EINVAL;
+		}
+	}
+
 	s_idx = cb->args[0];
 
 	rcu_read_lock();
@@ -178,8 +196,6 @@ static int br_mdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 
 	for_each_netdev_rcu(net, dev) {
 		if (dev->priv_flags & IFF_EBRIDGE) {
-			struct br_port_msg *bpm;
-
 			if (idx < s_idx)
 				goto skip;
 
-- 
2.11.0

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

* [PATCH RFC v2 net-next 20/25] net: Add struct for fib dump filter
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (18 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 19/25] net/bridge: Update br_mdb_dump " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 21/25] net/ipv4: Plumb support for filtering route dumps David Ahern
                   ` (5 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Add struct fib_dump_filter for options on limiting which routes are
dumped. The current list is table id, tos, protocol, scope, route type,
flags and nexthop device index.

This patch adds the struct and argument to ip_valid_fib_dump_req so
that per-protocol patches can be done followed by actually parsing any
data from userspace.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/ip6_route.h |  1 +
 include/net/ip_fib.h    | 12 ++++++++++++
 net/ipv4/fib_frontend.c |  4 +++-
 net/ipv4/ipmr.c         |  3 ++-
 net/ipv6/ip6_fib.c      |  4 ++--
 net/ipv6/ip6mr.c        |  3 ++-
 net/mpls/af_mpls.c      |  3 ++-
 7 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 7b9c82de11cc..ecaba26b3399 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -175,6 +175,7 @@ struct rt6_rtnl_dump_arg {
 	struct sk_buff *skb;
 	struct netlink_callback *cb;
 	struct net *net;
+	struct fib_dump_filter filter;
 };
 
 int rt6_dump_route(struct fib6_info *f6i, void *p_arg);
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 9846b79c9ee1..d0cd838ca00c 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -222,6 +222,17 @@ struct fib_table {
 	unsigned long		__data[0];
 };
 
+struct fib_dump_filter {
+	u32			table_id;
+	unsigned char		tos;
+	unsigned char		protocol;
+	unsigned char		scope;
+	unsigned char		rt_type;
+	unsigned int		flags;
+	int			ifindex;
+	struct net_device	*dev;
+};
+
 int fib_table_lookup(struct fib_table *tb, const struct flowi4 *flp,
 		     struct fib_result *res, int fib_flags);
 int fib_table_insert(struct net *, struct fib_table *, struct fib_config *,
@@ -453,5 +464,6 @@ static inline void fib_proc_exit(struct net *net)
 u32 ip_mtu_from_fib_result(struct fib_result *res, __be32 daddr);
 
 int ip_valid_fib_dump_req(const struct nlmsghdr *nlh,
+			  struct fib_dump_filter *filter,
 			  struct netlink_ext_ack *extack);
 #endif  /* _NET_FIB_H */
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index c608b393ae49..9d872a4900cd 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -803,6 +803,7 @@ static int inet_rtm_newroute(struct sk_buff *skb, struct nlmsghdr *nlh,
 }
 
 int ip_valid_fib_dump_req(const struct nlmsghdr *nlh,
+			  struct fib_dump_filter *filter,
 			  struct netlink_ext_ack *extack)
 {
 	struct rtmsg *rtm;
@@ -838,6 +839,7 @@ static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
+	struct fib_dump_filter filter = {};
 	unsigned int h, s_h;
 	unsigned int e = 0, s_e;
 	struct fib_table *tb;
@@ -845,7 +847,7 @@ static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 	int dumped = 0, err;
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
-		err = ip_valid_fib_dump_req(nlh, cb->extack);
+		err = ip_valid_fib_dump_req(nlh, &filter, cb->extack);
 		if (err)
 			return err;
 	}
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 91b5991ed536..9e9ad60dff6b 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2528,9 +2528,10 @@ static int ipmr_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 static int ipmr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	const struct nlmsghdr *nlh = cb->nlh;
+	struct fib_dump_filter filter = {};
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
-		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+		int err = ip_valid_fib_dump_req(nlh, &filter, cb->extack);
 
 		if (err)
 			return err;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index fc14733fbad8..e0362a21737f 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -570,16 +570,16 @@ static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
+	struct rt6_rtnl_dump_arg arg = {};
 	unsigned int h, s_h;
 	unsigned int e = 0, s_e;
-	struct rt6_rtnl_dump_arg arg;
 	struct fib6_walker *w;
 	struct fib6_table *tb;
 	struct hlist_head *head;
 	int res = 0;
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
-		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+		int err = ip_valid_fib_dump_req(nlh, &arg.filter, cb->extack);
 
 		if (err)
 			return err;
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index aa668214edc2..b3084b2c8f88 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -2434,9 +2434,10 @@ static void mrt6msg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt)
 static int ip6mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	const struct nlmsghdr *nlh = cb->nlh;
+	struct fib_dump_filter filter = {};
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
-		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+		int err = ip_valid_fib_dump_req(nlh, &filter, cb->extack);
 
 		if (err)
 			return err;
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 64de2f8c3847..f94d1db63eb5 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -2036,13 +2036,14 @@ static int mpls_dump_routes(struct sk_buff *skb, struct netlink_callback *cb)
 	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct mpls_route __rcu **platform_label;
+	struct fib_dump_filter filter = {};
 	size_t platform_labels;
 	unsigned int index;
 
 	ASSERT_RTNL();
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
-		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+		int err = ip_valid_fib_dump_req(nlh, &filter, cb->extack);
 
 		if (err)
 			return err;
-- 
2.11.0

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

* [PATCH RFC v2 net-next 21/25] net/ipv4: Plumb support for filtering route dumps
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (19 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 20/25] net: Add struct for fib dump filter David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 22/25] net/ipv6: " David Ahern
                   ` (4 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Implement kernel side filtering of routes by table id, egress device index,
protocol, tos, scope, and route type.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/ip_fib.h    |  2 +-
 net/ipv4/fib_frontend.c | 13 ++++++++++++-
 net/ipv4/fib_trie.c     | 33 ++++++++++++++++++++++-----------
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index d0cd838ca00c..e064c37a2a9f 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -240,7 +240,7 @@ int fib_table_insert(struct net *, struct fib_table *, struct fib_config *,
 int fib_table_delete(struct net *, struct fib_table *, struct fib_config *,
 		     struct netlink_ext_ack *extack);
 int fib_table_dump(struct fib_table *table, struct sk_buff *skb,
-		   struct netlink_callback *cb);
+		   struct netlink_callback *cb, struct fib_dump_filter *filter);
 int fib_table_flush(struct net *net, struct fib_table *table);
 struct fib_table *fib_trie_unmerge(struct fib_table *main_tb);
 void fib_table_flush_external(struct fib_table *table);
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 9d872a4900cd..a3f4073e509a 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -861,16 +861,27 @@ static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 
 	rcu_read_lock();
 
+	if (filter.ifindex) {
+		filter.dev = dev_get_by_index_rcu(net, filter.ifindex);
+		if (!filter.dev) {
+			err = -ENODEV;
+			goto out_err;
+		}
+	}
+
 	for (h = s_h; h < FIB_TABLE_HASHSZ; h++, s_e = 0) {
 		e = 0;
 		head = &net->ipv4.fib_table_hash[h];
 		hlist_for_each_entry_rcu(tb, head, tb_hlist) {
 			if (e < s_e)
 				goto next;
+			if (filter.table_id && filter.table_id != tb->tb_id)
+				goto next;
+
 			if (dumped)
 				memset(&cb->args[2], 0, sizeof(cb->args) -
 						 2 * sizeof(cb->args[0]));
-			err = fib_table_dump(tb, skb, cb);
+			err = fib_table_dump(tb, skb, cb, &filter);
 			if (err < 0) {
 				if (likely(skb->len))
 					goto out;
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 5bc0c89e81e4..0e7b4233851a 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -2003,7 +2003,8 @@ void fib_free_table(struct fib_table *tb)
 }
 
 static int fn_trie_dump_leaf(struct key_vector *l, struct fib_table *tb,
-			     struct sk_buff *skb, struct netlink_callback *cb)
+			     struct sk_buff *skb, struct netlink_callback *cb,
+			     struct fib_dump_filter *filter)
 {
 	__be32 xkey = htonl(l->key);
 	struct fib_alias *fa;
@@ -2016,15 +2017,24 @@ static int fn_trie_dump_leaf(struct key_vector *l, struct fib_table *tb,
 	hlist_for_each_entry_rcu(fa, &l->leaf, fa_list) {
 		int err;
 
-		if (i < s_i) {
-			i++;
-			continue;
-		}
+		if (i < s_i)
+			goto next;
 
-		if (tb->tb_id != fa->tb_id) {
-			i++;
-			continue;
-		}
+		if (tb->tb_id != fa->tb_id)
+			goto next;
+
+		if ((filter->tos && fa->fa_tos != filter->tos) ||
+		    (filter->rt_type && fa->fa_type != filter->rt_type))
+			goto next;
+
+		if ((filter->protocol &&
+		     fa->fa_info->fib_protocol != filter->protocol) ||
+		    (filter->scope && fa->fa_info->fib_scope != filter->scope))
+			goto next;
+
+		if (filter->dev &&
+		    !fib_info_nh_uses_dev(fa->fa_info, filter->dev))
+			goto next;
 
 		err = fib_dump_info(skb, NETLINK_CB(cb->skb).portid,
 				    cb->nlh->nlmsg_seq, RTM_NEWROUTE,
@@ -2035,6 +2045,7 @@ static int fn_trie_dump_leaf(struct key_vector *l, struct fib_table *tb,
 			cb->args[4] = i;
 			return err;
 		}
+next:
 		i++;
 	}
 
@@ -2044,7 +2055,7 @@ static int fn_trie_dump_leaf(struct key_vector *l, struct fib_table *tb,
 
 /* rcu_read_lock needs to be hold by caller from readside */
 int fib_table_dump(struct fib_table *tb, struct sk_buff *skb,
-		   struct netlink_callback *cb)
+		   struct netlink_callback *cb, struct fib_dump_filter *filter)
 {
 	struct trie *t = (struct trie *)tb->tb_data;
 	struct key_vector *l, *tp = t->kv;
@@ -2057,7 +2068,7 @@ int fib_table_dump(struct fib_table *tb, struct sk_buff *skb,
 	while ((l = leaf_walk_rcu(&tp, key)) != NULL) {
 		int err;
 
-		err = fn_trie_dump_leaf(l, tb, skb, cb);
+		err = fn_trie_dump_leaf(l, tb, skb, cb, filter);
 		if (err < 0) {
 			cb->args[3] = key;
 			cb->args[2] = count;
-- 
2.11.0

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

* [PATCH RFC v2 net-next 22/25] net/ipv6: Plumb support for filtering route dumps
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (20 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 21/25] net/ipv4: Plumb support for filtering route dumps David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 23/25] net/mpls: " David Ahern
                   ` (3 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Implement kernel side filtering of routes by table id, egress device index,
protocol, and route type. Move the existing route flags check
for prefix only routes to the new filter.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv6/ip6_fib.c | 13 +++++++++++++
 net/ipv6/route.c   | 36 +++++++++++++++++++++++++++---------
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index e0362a21737f..15b9806270c1 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -613,12 +613,25 @@ static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 	w->args = &arg;
 
 	rcu_read_lock();
+
+	if (arg.filter.ifindex) {
+		arg.filter.dev = dev_get_by_index_rcu(net, arg.filter.ifindex);
+		if (!arg.filter.dev) {
+			res = -ENODEV;
+			goto out;
+		}
+	}
+
 	for (h = s_h; h < FIB6_TABLE_HASHSZ; h++, s_e = 0) {
 		e = 0;
 		head = &net->ipv6.fib_table_hash[h];
 		hlist_for_each_entry_rcu(tb, head, tb6_hlist) {
 			if (e < s_e)
 				goto next;
+			if (arg.filter.table_id &&
+			    arg.filter.table_id != tb->tb6_id)
+				goto next;
+
 			res = fib6_dump_table(tb, skb, cb);
 			if (res != 0)
 				goto out;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d28f83e01593..99ba2313c380 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4792,24 +4792,42 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb,
 	return -EMSGSIZE;
 }
 
+static bool fib6_info_uses_dev(const struct fib6_info *f6i,
+			       const struct net_device *dev)
+{
+	if (f6i->fib6_nh.nh_dev == dev)
+		return true;
+
+	if (f6i->fib6_nsiblings) {
+		struct fib6_info *sibling, *next_sibling;
+
+		list_for_each_entry_safe(sibling, next_sibling,
+					 &f6i->fib6_siblings, fib6_siblings) {
+			if (sibling->fib6_nh.nh_dev == dev)
+				return true;
+		}
+	}
+	return false;
+}
+
 int rt6_dump_route(struct fib6_info *rt, void *p_arg)
 {
 	struct rt6_rtnl_dump_arg *arg = (struct rt6_rtnl_dump_arg *) p_arg;
+	struct fib_dump_filter *filter = &arg->filter;
 	struct net *net = arg->net;
 
 	if (rt == net->ipv6.fib6_null_entry)
 		return 0;
 
-	if (nlmsg_len(arg->cb->nlh) >= sizeof(struct rtmsg)) {
-		struct rtmsg *rtm = nlmsg_data(arg->cb->nlh);
-
-		/* user wants prefix routes only */
-		if (rtm->rtm_flags & RTM_F_PREFIX &&
-		    !(rt->fib6_flags & RTF_PREFIX_RT)) {
-			/* success since this is not a prefix route */
-			return 1;
-		}
+	if ((filter->flags & RTM_F_PREFIX) &&
+	    !(rt->fib6_flags & RTF_PREFIX_RT)) {
+		/* success since this is not a prefix route */
+		return 1;
 	}
+	if ((filter->protocol && rt->fib6_protocol != filter->protocol) ||
+	    (filter->rt_type && rt->fib6_type != filter->rt_type) ||
+	    (filter->dev && !fib6_info_uses_dev(rt, filter->dev)))
+		return 1;
 
 	return rt6_fill_node(net, arg->skb, rt, NULL, NULL, NULL, 0,
 			     RTM_NEWROUTE, NETLINK_CB(arg->cb->skb).portid,
-- 
2.11.0

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

* [PATCH RFC v2 net-next 23/25] net/mpls: Plumb support for filtering route dumps
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (21 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 22/25] net/ipv6: " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 24/25] net: Plumb support for filtering ipv4 and ipv6 multicast " David Ahern
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Implement kernel side filtering of routes by egress device index and
protocol. MPLS uses only a single table and route type.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/mpls/af_mpls.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index f94d1db63eb5..4dd8a2a026e7 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -2031,6 +2031,28 @@ static int mpls_dump_route(struct sk_buff *skb, u32 portid, u32 seq, int event,
 	return -EMSGSIZE;
 }
 
+static bool mpls_rt_uses_dev(struct mpls_route *rt,
+			     const struct net_device *dev)
+{
+	struct net_device *nh_dev;
+
+	if (rt->rt_nhn == 1) {
+		struct mpls_nh *nh = rt->rt_nh;
+
+		nh_dev = rtnl_dereference(nh->nh_dev);
+		if (dev == nh_dev)
+			return true;
+	} else {
+		for_nexthops(rt) {
+			nh_dev = rtnl_dereference(nh->nh_dev);
+			if (nh_dev == dev)
+				return true;
+		} endfor_nexthops(rt);
+	}
+
+	return false;
+}
+
 static int mpls_dump_routes(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	const struct nlmsghdr *nlh = cb->nlh;
@@ -2039,6 +2061,7 @@ static int mpls_dump_routes(struct sk_buff *skb, struct netlink_callback *cb)
 	struct fib_dump_filter filter = {};
 	size_t platform_labels;
 	unsigned int index;
+	int err;
 
 	ASSERT_RTNL();
 
@@ -2047,6 +2070,15 @@ static int mpls_dump_routes(struct sk_buff *skb, struct netlink_callback *cb)
 
 		if (err)
 			return err;
+
+		/* for MPLS, there is only 1 table with fixed type, scope
+		 * tos and flags. If any of these are set in the filter then
+		 * return nothing
+		 */
+		if ((filter.table_id && filter.table_id != RT_TABLE_MAIN) ||
+		    (filter.rt_type && filter.rt_type != RTN_UNICAST) ||
+		     filter.scope || filter.tos || filter.flags)
+			return 0;
 	}
 
 	index = cb->args[0];
@@ -2055,20 +2087,41 @@ static int mpls_dump_routes(struct sk_buff *skb, struct netlink_callback *cb)
 
 	platform_label = rtnl_dereference(net->mpls.platform_label);
 	platform_labels = net->mpls.platform_labels;
+
+	rcu_read_lock();
+
+	if (filter.ifindex) {
+		filter.dev = dev_get_by_index_rcu(net, filter.ifindex);
+		if (!filter.dev) {
+			err = -ENODEV;
+			goto out_err;
+		}
+	}
+
 	for (; index < platform_labels; index++) {
 		struct mpls_route *rt;
+
 		rt = rtnl_dereference(platform_label[index]);
 		if (!rt)
 			continue;
 
+		if (filter.protocol && rt->rt_protocol != filter.protocol)
+			continue;
+
+		if (filter.dev && !mpls_rt_uses_dev(rt, filter.dev))
+			continue;
+
 		if (mpls_dump_route(skb, NETLINK_CB(cb->skb).portid,
 				    cb->nlh->nlmsg_seq, RTM_NEWROUTE,
 				    index, rt, NLM_F_MULTI) < 0)
 			break;
 	}
 	cb->args[0] = index;
+	err = skb->len;
 
-	return skb->len;
+out_err:
+	rcu_read_unlock();
+	return err;
 }
 
 static inline size_t lfib_nlmsg_size(struct mpls_route *rt)
-- 
2.11.0

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

* [PATCH RFC v2 net-next 24/25] net: Plumb support for filtering ipv4 and ipv6 multicast route dumps
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (22 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 23/25] net/mpls: " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 25/25] net: Enable kernel side filtering of " David Ahern
  2018-10-03 14:59 ` [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request Stephen Hemminger
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Implement kernel side filtering of routes by egress device index and
table id.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/linux/mroute_base.h |  5 +++--
 net/ipv4/ipmr.c             |  2 +-
 net/ipv4/ipmr_base.c        | 42 +++++++++++++++++++++++++++++++++++++++++-
 net/ipv6/ip6mr.c            |  2 +-
 4 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/include/linux/mroute_base.h b/include/linux/mroute_base.h
index 6675b9f81979..8fc516c47a64 100644
--- a/include/linux/mroute_base.h
+++ b/include/linux/mroute_base.h
@@ -7,6 +7,7 @@
 #include <net/net_namespace.h>
 #include <net/sock.h>
 #include <net/fib_notifier.h>
+#include <net/ip_fib.h>
 
 /**
  * struct vif_device - interface representor for multicast routing
@@ -290,7 +291,7 @@ int mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb,
 				 struct sk_buff *skb,
 				 u32 portid, u32 seq, struct mr_mfc *c,
 				 int cmd, int flags),
-		     spinlock_t *lock);
+		     spinlock_t *lock, struct fib_dump_filter *filter);
 
 int mr_dump(struct net *net, struct notifier_block *nb, unsigned short family,
 	    int (*rules_dump)(struct net *net,
@@ -340,7 +341,7 @@ mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb,
 			     struct sk_buff *skb,
 			     u32 portid, u32 seq, struct mr_mfc *c,
 			     int cmd, int flags),
-		 spinlock_t *lock)
+		 spinlock_t *lock, struct fib_dump_filter *filter)
 {
 	return -EINVAL;
 }
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 9e9ad60dff6b..2fe24009439a 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2538,7 +2538,7 @@ static int ipmr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb)
 	}
 
 	return mr_rtm_dumproute(skb, cb, ipmr_mr_table_iter,
-				_ipmr_fill_mroute, &mfc_unres_lock);
+				_ipmr_fill_mroute, &mfc_unres_lock, &filter);
 }
 
 static const struct nla_policy rtm_ipmr_policy[RTA_MAX + 1] = {
diff --git a/net/ipv4/ipmr_base.c b/net/ipv4/ipmr_base.c
index 1ad9aa62a97b..a4f83cbf033d 100644
--- a/net/ipv4/ipmr_base.c
+++ b/net/ipv4/ipmr_base.c
@@ -268,6 +268,24 @@ int mr_fill_mroute(struct mr_table *mrt, struct sk_buff *skb,
 }
 EXPORT_SYMBOL(mr_fill_mroute);
 
+static bool mr_mfc_uses_dev(const struct mr_table *mrt,
+			    const struct mr_mfc *c,
+			    const struct net_device *dev)
+{
+	int ct;
+
+	for (ct = c->mfc_un.res.minvif; ct < c->mfc_un.res.maxvif; ct++) {
+		if (VIF_EXISTS(mrt, ct) && c->mfc_un.res.ttls[ct] < 255) {
+			const struct vif_device *vif;
+
+			vif = &mrt->vif_table[ct];
+			if (vif->dev == dev)
+				return true;
+		}
+	}
+	return false;
+}
+
 int mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb,
 		     struct mr_table *(*iter)(struct net *net,
 					      struct mr_table *mrt),
@@ -275,17 +293,35 @@ int mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb,
 				 struct sk_buff *skb,
 				 u32 portid, u32 seq, struct mr_mfc *c,
 				 int cmd, int flags),
-		     spinlock_t *lock)
+		     spinlock_t *lock, struct fib_dump_filter *filter)
 {
 	unsigned int t = 0, e = 0, s_t = cb->args[0], s_e = cb->args[1];
 	struct net *net = sock_net(skb->sk);
 	struct mr_table *mrt;
 	struct mr_mfc *mfc;
 
+	/* multicast does not use tos or scope, track protocol or have
+	 * route type other than RTN_MULTICAST
+	 */
+	if (filter->tos || filter->protocol || filter->scope || filter->flags ||
+	    (filter->rt_type && filter->rt_type != RTN_MULTICAST))
+		return 0;
+
 	rcu_read_lock();
+
+	if (filter->ifindex) {
+		filter->dev = dev_get_by_index_rcu(net, filter->ifindex);
+		if (!filter->dev) {
+			rcu_read_unlock();
+			return -ENODEV;
+		}
+	}
+
 	for (mrt = iter(net, NULL); mrt; mrt = iter(net, mrt)) {
 		if (t < s_t)
 			goto next_table;
+		if (filter->table_id && filter->table_id != mrt->id)
+			goto next_table;
 		list_for_each_entry_rcu(mfc, &mrt->mfc_cache_list, list) {
 			if (e < s_e)
 				goto next_entry;
@@ -303,6 +339,10 @@ int mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb,
 		list_for_each_entry(mfc, &mrt->mfc_unres_queue, list) {
 			if (e < s_e)
 				goto next_entry2;
+			if (filter->dev &&
+			    !mr_mfc_uses_dev(mrt, mfc, filter->dev))
+				goto next_entry2;
+
 			if (fill(mrt, skb, NETLINK_CB(cb->skb).portid,
 				 cb->nlh->nlmsg_seq, mfc,
 				 RTM_NEWROUTE, NLM_F_MULTI) < 0) {
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index b3084b2c8f88..08e2443ca0cc 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -2444,5 +2444,5 @@ static int ip6mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb)
 	}
 
 	return mr_rtm_dumproute(skb, cb, ip6mr_mr_table_iter,
-				_ip6mr_fill_mroute, &mfc_unres_lock);
+				_ip6mr_fill_mroute, &mfc_unres_lock, &filter);
 }
-- 
2.11.0

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

* [PATCH RFC v2 net-next 25/25] net: Enable kernel side filtering of route dumps
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (23 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 24/25] net: Plumb support for filtering ipv4 and ipv6 multicast " David Ahern
@ 2018-10-02  0:28 ` David Ahern
  2018-10-03 14:59 ` [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request Stephen Hemminger
  25 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02  0:28 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Update parsing of route dump request to enable kernel side of filtering.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/fib_frontend.c | 42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index a3f4073e509a..d1ef1cb98139 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -806,7 +806,9 @@ int ip_valid_fib_dump_req(const struct nlmsghdr *nlh,
 			  struct fib_dump_filter *filter,
 			  struct netlink_ext_ack *extack)
 {
+	struct nlattr *tb[RTA_MAX + 1];
 	struct rtmsg *rtm;
+	int err, i;
 
 	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) {
 		NL_SET_ERR_MSG(extack, "Invalid header");
@@ -814,21 +816,37 @@ int ip_valid_fib_dump_req(const struct nlmsghdr *nlh,
 	}
 
 	rtm = nlmsg_data(nlh);
-	if (rtm->rtm_dst_len || rtm->rtm_src_len  || rtm->rtm_tos   ||
-	    rtm->rtm_table   || rtm->rtm_protocol || rtm->rtm_scope ||
-	    rtm->rtm_type) {
-		NL_SET_ERR_MSG(extack,
-			       "Invalid values in header for dump request");
+	if (rtm->rtm_dst_len || rtm->rtm_src_len) {
+		NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
 		return -EINVAL;
 	}
 
-	if (rtm->rtm_flags & ~(RTM_F_CLONED | RTM_F_PREFIX)) {
-		NL_SET_ERR_MSG(extack, "Invalid flags for dump request");
-		return -EINVAL;
-	}
-	if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*rtm))) {
-		NL_SET_ERR_MSG(extack, "Invalid data after header");
-		return -EINVAL;
+	filter->flags    = rtm->rtm_flags;
+	filter->tos      = rtm->rtm_tos;
+	filter->protocol = rtm->rtm_protocol;
+	filter->scope    = rtm->rtm_scope;
+	filter->rt_type  = rtm->rtm_type;
+	filter->table_id = rtm->rtm_table;
+
+	err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX,
+			  rtm_ipv4_policy, extack);
+	if (err < 0)
+		return err;
+
+	for (i = 0; i <= RTA_MAX; ++i) {
+		if (!tb[i])
+			continue;
+		switch (i) {
+		case RTA_TABLE:
+			filter->table_id = nla_get_u32(tb[i]);
+			break;
+		case RTA_OIF:
+			filter->ifindex = nla_get_u32(tb[i]);
+			break;
+		default:
+			NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
+			return -EINVAL;
+		}
 	}
 
 	return 0;
-- 
2.11.0

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

* Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs David Ahern
@ 2018-10-02 10:54   ` Jiri Benc
  2018-10-02 11:03     ` Christian Brauner
  2018-10-02 15:11     ` David Ahern
  0 siblings, 2 replies; 43+ messages in thread
From: Jiri Benc @ 2018-10-02 10:54 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, christian, stephen, David Ahern

On Mon,  1 Oct 2018 17:28:28 -0700, David Ahern wrote:
> Pull the inet6_fill_args arg up to in6_dump_addrs and move netnsid
> into it. Since IFA_TARGET_NETNSID is a kernel side filter add the
> NLM_F_DUMP_FILTERED flag so userspace knows the request was honored.

IFA_TARGET_NETNSID is not a filter.

"Filter" returns a subset of the results. It's kind of optimization
when one is interested only in some data but not all of them. Instead
of dumping everything, going through the results and picking only the
data one is interested in, it's better to pass a filter and get only
the relevant data. But you're not really required to: you can filter in
your app.

By contrast, IFA_TARGET_NETNSID returns a completely different set of
data. It's impossible to not set it and filter the results in your app.

As the consequence, IFA_TARGET_NETNSID must not set NLM_F_DUMP_FILTERED
(if not complemented by a real filter).

I understand that you want to differentiate between data dumped without
and with IFA_TARGET_NETNSID present. But we already have that: the
IFA_TARGET_NETNSID attribute is returned back in the latter case.

Nacked-by: Jiri Benc <jbenc@redhat.com>

 Jiri

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

* Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs
  2018-10-02 10:54   ` Jiri Benc
@ 2018-10-02 11:03     ` Christian Brauner
  2018-10-02 11:07       ` Jiri Benc
  2018-10-02 15:11     ` David Ahern
  1 sibling, 1 reply; 43+ messages in thread
From: Christian Brauner @ 2018-10-02 11:03 UTC (permalink / raw)
  To: Jiri Benc; +Cc: David Ahern, netdev, davem, stephen, David Ahern

On Tue, Oct 02, 2018 at 12:54:25PM +0200, Jiri Benc wrote:
> On Mon,  1 Oct 2018 17:28:28 -0700, David Ahern wrote:
> > Pull the inet6_fill_args arg up to in6_dump_addrs and move netnsid
> > into it. Since IFA_TARGET_NETNSID is a kernel side filter add the
> > NLM_F_DUMP_FILTERED flag so userspace knows the request was honored.
> 
> IFA_TARGET_NETNSID is not a filter.

Well, it's a namespace filter that's how I saw it.

> 
> "Filter" returns a subset of the results. It's kind of optimization

That's an argument I can buy.

> when one is interested only in some data but not all of them. Instead
> of dumping everything, going through the results and picking only the
> data one is interested in, it's better to pass a filter and get only
> the relevant data. But you're not really required to: you can filter in
> your app.
> 
> By contrast, IFA_TARGET_NETNSID returns a completely different set of
> data. It's impossible to not set it and filter the results in your app.
> 
> As the consequence, IFA_TARGET_NETNSID must not set NLM_F_DUMP_FILTERED
> (if not complemented by a real filter).
> 
> I understand that you want to differentiate between data dumped without
> and with IFA_TARGET_NETNSID present. But we already have that: the
> IFA_TARGET_NETNSID attribute is returned back in the latter case.
> 
> Nacked-by: Jiri Benc <jbenc@redhat.com>
> 
>  Jiri

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

* Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag David Ahern
@ 2018-10-02 11:06   ` Jiri Benc
  2018-10-02 11:18     ` Christian Brauner
  2018-10-02 14:55     ` David Ahern
  0 siblings, 2 replies; 43+ messages in thread
From: Jiri Benc @ 2018-10-02 11:06 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, christian, stephen, David Ahern

On Mon,  1 Oct 2018 17:28:29 -0700, David Ahern wrote:
> Add a new flag, NLM_F_DUMP_PROPER_HDR, for userspace to indicate to the
> kernel that it believes it is sending the right header struct for the
> dump message type (ifinfomsg, ifaddrmsg, rtmsg, fib_rule_hdr, ...).

Why is this limited to dumps? Other kind of netlink messages contain
the common struct, too. When introducing such mechanism, please make it
generic.

Last time when we were discussing strict checking in netlink, it was
suggested to add a socket option instead of adding NLM flags[1].
It makes a lot of sense: the number of flags is very limited and we'd
run out of them pretty fast. It's not just the header structure that
is currently checked sloppily. It's also attributes, flags in
attributes, etc. We can't assign a flag to all of them.

You should also consider a different name for the flag: it should
reflect what the effect of the flag is. "Proper header" is not an
effect, it's a requirement for the message to pass. The effect is
enforced strict checking of the header.

 Jiri

[1] https://marc.info/?l=linux-netdev&m=144492718118955

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

* Re: [PATCH RFC v2 net-next 01/25] net/netlink: Pass extack to dump callbacks
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 01/25] net/netlink: Pass extack to dump callbacks David Ahern
@ 2018-10-02 11:07   ` Christian Brauner
  0 siblings, 0 replies; 43+ messages in thread
From: Christian Brauner @ 2018-10-02 11:07 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jbenc, stephen, David Ahern

On Mon, Oct 01, 2018 at 05:28:27PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Pass extack to dump callbacks by adding extack to netlink_dump_control,
> transferring to netlink_callback and adding to the netlink_dump. Update
> rtnetlink as the first user. Update netlink_dump to add any message after
> the dump_done_errno.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

This makes sense to me as it would allow us to report back more
meaningful errors to userspace.

> ---
>  include/linux/netlink.h  |  2 ++
>  net/core/rtnetlink.c     |  1 +
>  net/netlink/af_netlink.c | 20 +++++++++++++++-----
>  3 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/netlink.h b/include/linux/netlink.h
> index 71f121b66ca8..8fc90308a653 100644
> --- a/include/linux/netlink.h
> +++ b/include/linux/netlink.h
> @@ -176,6 +176,7 @@ struct netlink_callback {
>  	void			*data;
>  	/* the module that dump function belong to */
>  	struct module		*module;
> +	struct netlink_ext_ack	*extack;
>  	u16			family;
>  	u16			min_dump_alloc;
>  	unsigned int		prev_seq, seq;
> @@ -197,6 +198,7 @@ struct netlink_dump_control {
>  	int (*done)(struct netlink_callback *);
>  	void *data;
>  	struct module *module;
> +	struct netlink_ext_ack *extack;
>  	u16 min_dump_alloc;
>  };
>  
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 35162e1b06ad..da91b38297d3 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -4689,6 +4689,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
>  				.dump		= dumpit,
>  				.min_dump_alloc	= min_dump_alloc,
>  				.module		= owner,
> +				.extack		= extack
>  			};
>  			err = netlink_dump_start(rtnl, skb, nlh, &c);
>  			/* netlink_dump_start() will keep a reference on
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index e3a0538ec0be..7094156c94f0 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -129,7 +129,7 @@ static const char *const nlk_cb_mutex_key_strings[MAX_LINKS + 1] = {
>  	"nlk_cb_mutex-MAX_LINKS"
>  };
>  
> -static int netlink_dump(struct sock *sk);
> +static int netlink_dump(struct sock *sk, struct netlink_ext_ack *extack);
>  
>  /* nl_table locking explained:
>   * Lookup and traversal are protected with an RCU read-side lock. Insertion
> @@ -1981,7 +1981,7 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>  
>  	if (nlk->cb_running &&
>  	    atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf / 2) {
> -		ret = netlink_dump(sk);
> +		ret = netlink_dump(sk, NULL);
>  		if (ret) {
>  			sk->sk_err = -ret;
>  			sk->sk_error_report(sk);
> @@ -2168,7 +2168,7 @@ EXPORT_SYMBOL(__nlmsg_put);
>   * It would be better to create kernel thread.
>   */
>  
> -static int netlink_dump(struct sock *sk)
> +static int netlink_dump(struct sock *sk, struct netlink_ext_ack *extack)
>  {
>  	struct netlink_sock *nlk = nlk_sk(sk);
>  	struct netlink_callback *cb;
> @@ -2222,8 +2222,11 @@ static int netlink_dump(struct sock *sk)
>  	skb_reserve(skb, skb_tailroom(skb) - alloc_size);
>  	netlink_skb_set_owner_r(skb, sk);
>  
> -	if (nlk->dump_done_errno > 0)
> +	if (nlk->dump_done_errno > 0) {
> +		cb->extack = extack;
>  		nlk->dump_done_errno = cb->dump(skb, cb);
> +		cb->extack = NULL;
> +	}
>  
>  	if (nlk->dump_done_errno > 0 ||
>  	    skb_tailroom(skb) < nlmsg_total_size(sizeof(nlk->dump_done_errno))) {
> @@ -2246,6 +2249,12 @@ static int netlink_dump(struct sock *sk)
>  	memcpy(nlmsg_data(nlh), &nlk->dump_done_errno,
>  	       sizeof(nlk->dump_done_errno));
>  
> +	if (extack && extack->_msg && nlk->flags & NETLINK_F_EXT_ACK) {
> +		nlh->nlmsg_flags |= NLM_F_ACK_TLVS;
> +		if (!nla_put_string(skb, NLMSGERR_ATTR_MSG, extack->_msg))
> +			nlmsg_end(skb, nlh);
> +	}
> +
>  	if (sk_filter(sk, skb))
>  		kfree_skb(skb);
>  	else
> @@ -2307,6 +2316,7 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
>  	cb->module = control->module;
>  	cb->min_dump_alloc = control->min_dump_alloc;
>  	cb->skb = skb;
> +	cb->extack = control->extack;
>  
>  	if (control->start) {
>  		ret = control->start(cb);
> @@ -2319,7 +2329,7 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
>  
>  	mutex_unlock(nlk->cb_mutex);
>  
> -	ret = netlink_dump(sk);
> +	ret = netlink_dump(sk, cb->extack);
>  
>  	sock_put(sk);
>  
> -- 
> 2.11.0
> 

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

* Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs
  2018-10-02 11:03     ` Christian Brauner
@ 2018-10-02 11:07       ` Jiri Benc
  2018-10-02 11:09         ` Christian Brauner
  0 siblings, 1 reply; 43+ messages in thread
From: Jiri Benc @ 2018-10-02 11:07 UTC (permalink / raw)
  To: Christian Brauner; +Cc: David Ahern, netdev, davem, stephen, David Ahern

On Tue, 2 Oct 2018 13:03:00 +0200, Christian Brauner wrote:
> Well, it's a namespace filter that's how I saw it.

That would imply that without it, you get data from all name spaces
(= unfiltered by name space), which is not what's happening :-)

 Jiri

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

* Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs
  2018-10-02 11:07       ` Jiri Benc
@ 2018-10-02 11:09         ` Christian Brauner
  0 siblings, 0 replies; 43+ messages in thread
From: Christian Brauner @ 2018-10-02 11:09 UTC (permalink / raw)
  To: Jiri Benc; +Cc: David Ahern, netdev, davem, stephen, David Ahern

On Tue, Oct 02, 2018 at 01:07:49PM +0200, Jiri Benc wrote:
> On Tue, 2 Oct 2018 13:03:00 +0200, Christian Brauner wrote:
> > Well, it's a namespace filter that's how I saw it.
> 
> That would imply that without it, you get data from all name spaces
> (= unfiltered by name space), which is not what's happening :-)

Yeah, you convinced me already. I'm not one to argue for the sake of
winning an argument (At least I hope so.) :)

Christian

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

* Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag
  2018-10-02 11:06   ` Jiri Benc
@ 2018-10-02 11:18     ` Christian Brauner
  2018-10-02 11:27       ` Jiri Benc
  2018-10-02 14:55     ` David Ahern
  1 sibling, 1 reply; 43+ messages in thread
From: Christian Brauner @ 2018-10-02 11:18 UTC (permalink / raw)
  To: Jiri Benc; +Cc: David Ahern, netdev, davem, stephen, David Ahern

On Tue, Oct 02, 2018 at 01:06:14PM +0200, Jiri Benc wrote:
> On Mon,  1 Oct 2018 17:28:29 -0700, David Ahern wrote:
> > Add a new flag, NLM_F_DUMP_PROPER_HDR, for userspace to indicate to the
> > kernel that it believes it is sending the right header struct for the
> > dump message type (ifinfomsg, ifaddrmsg, rtmsg, fib_rule_hdr, ...).
> 
> Why is this limited to dumps? Other kind of netlink messages contain
> the common struct, too. When introducing such mechanism, please make it
> generic.
> 
> Last time when we were discussing strict checking in netlink, it was
> suggested to add a socket option instead of adding NLM flags[1].

I didn't find this in the linked thread. What I find interesting and
convincing is one of Dave's points:

"I'm beginning to wonder if we can just change this unilaterally to
not ignore unrecognized attributes.

I am increasingly certain that things that would "break" we wouldn't
want to succeed anyways." [1]

:)

But a socket option or this header flag both sound acceptable to me. Was
there any more detail on how a socket option would look like, i.e. an
api proposal or something?

[1]: https://marc.info/?l=linux-netdev&m=144522081220166&w=2

> It makes a lot of sense: the number of flags is very limited and we'd
> run out of them pretty fast. It's not just the header structure that
> is currently checked sloppily. It's also attributes, flags in
> attributes, etc. We can't assign a flag to all of them.
> 
> You should also consider a different name for the flag: it should
> reflect what the effect of the flag is. "Proper header" is not an
> effect, it's a requirement for the message to pass. The effect is
> enforced strict checking of the header.
> 
>  Jiri
> 
> [1] https://marc.info/?l=linux-netdev&m=144492718118955

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

* Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag
  2018-10-02 11:18     ` Christian Brauner
@ 2018-10-02 11:27       ` Jiri Benc
  2018-10-02 14:57         ` David Ahern
  0 siblings, 1 reply; 43+ messages in thread
From: Jiri Benc @ 2018-10-02 11:27 UTC (permalink / raw)
  To: Christian Brauner; +Cc: David Ahern, netdev, davem, stephen, David Ahern

On Tue, 2 Oct 2018 13:18:32 +0200, Christian Brauner wrote:
> I didn't find this in the linked thread.

Maybe it was suggested in another thread or in person on a conference,
I can't remember, it's too long ago, sorry.

> What I find interesting and convincing is one of Dave's points:
> 
> "I'm beginning to wonder if we can just change this unilaterally to
> not ignore unrecognized attributes.
> 
> I am increasingly certain that things that would "break" we wouldn't
> want to succeed anyways." [1]

It's unfortunate we can't do that. I'd like it.

> But a socket option or this header flag both sound acceptable to me. Was
> there any more detail on how a socket option would look like, i.e. an
> api proposal or something?

Look at how NETLINK_CAP_ACK and NETLINK_EXT_ACK is implemented.

 Jiri

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

* Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag
  2018-10-02 11:06   ` Jiri Benc
  2018-10-02 11:18     ` Christian Brauner
@ 2018-10-02 14:55     ` David Ahern
  2018-10-02 16:33       ` Jiri Benc
  1 sibling, 1 reply; 43+ messages in thread
From: David Ahern @ 2018-10-02 14:55 UTC (permalink / raw)
  To: Jiri Benc, David Ahern; +Cc: netdev, davem, christian, stephen

On 10/2/18 5:06 AM, Jiri Benc wrote:
> On Mon,  1 Oct 2018 17:28:29 -0700, David Ahern wrote:
>> Add a new flag, NLM_F_DUMP_PROPER_HDR, for userspace to indicate to the
>> kernel that it believes it is sending the right header struct for the
>> dump message type (ifinfomsg, ifaddrmsg, rtmsg, fib_rule_hdr, ...).
> 
> Why is this limited to dumps? Other kind of netlink messages contain
> the common struct, too. When introducing such mechanism, please make it
> generic.

Because all of the other requests -- NEW, DEL, and GET -- all seem to
use and expect the right header. Dumps are the ones where the header is
not looked at in most cases and for years iproute2 got away with sending
the wrong one.

> 
> Last time when we were discussing strict checking in netlink, it was
> suggested to add a socket option instead of adding NLM flags[1].
> It makes a lot of sense: the number of flags is very limited and we'd
> run out of them pretty fast. It's not just the header structure that
> is currently checked sloppily. It's also attributes, flags in
> attributes, etc. We can't assign a flag to all of them.


> 
> You should also consider a different name for the flag: it should
> reflect what the effect of the flag is. "Proper header" is not an
> effect, it's a requirement for the message to pass. The effect is
> enforced strict checking of the header.

Proper means the correct header for the dump type is sent.

You want take issue with a name suggest a different one.

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

* Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag
  2018-10-02 11:27       ` Jiri Benc
@ 2018-10-02 14:57         ` David Ahern
  2018-10-02 16:30           ` Jiri Benc
  0 siblings, 1 reply; 43+ messages in thread
From: David Ahern @ 2018-10-02 14:57 UTC (permalink / raw)
  To: Jiri Benc, Christian Brauner; +Cc: David Ahern, netdev, davem, stephen

On 10/2/18 5:27 AM, Jiri Benc wrote:
> On Tue, 2 Oct 2018 13:18:32 +0200, Christian Brauner wrote:
>> I didn't find this in the linked thread.
> 
> Maybe it was suggested in another thread or in person on a conference,
> I can't remember, it's too long ago, sorry.
> 
>> What I find interesting and convincing is one of Dave's points:
>>
>> "I'm beginning to wonder if we can just change this unilaterally to
>> not ignore unrecognized attributes.
>>
>> I am increasingly certain that things that would "break" we wouldn't
>> want to succeed anyways." [1]
> 
> It's unfortunate we can't do that. I'd like it.

You can when you introduce a new option or a new flag that is required
to get new behavior like kernel side filtering.

> 
>> But a socket option or this header flag both sound acceptable to me. Was
>> there any more detail on how a socket option would look like, i.e. an
>> api proposal or something?
> 
> Look at how NETLINK_CAP_ACK and NETLINK_EXT_ACK is implemented.

I chose a netlink flag for consistency with NLM_F_DUMP_INTR and
NLM_F_DUMP_FILTERED. Both are netlink flags. This patch set fixes only
what is broken -- dumps.

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

* Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs
  2018-10-02 10:54   ` Jiri Benc
  2018-10-02 11:03     ` Christian Brauner
@ 2018-10-02 15:11     ` David Ahern
  2018-10-02 16:24       ` Jiri Benc
  1 sibling, 1 reply; 43+ messages in thread
From: David Ahern @ 2018-10-02 15:11 UTC (permalink / raw)
  To: Jiri Benc, David Ahern; +Cc: netdev, davem, christian, stephen

On 10/2/18 4:54 AM, Jiri Benc wrote:
> On Mon,  1 Oct 2018 17:28:28 -0700, David Ahern wrote:
>> Pull the inet6_fill_args arg up to in6_dump_addrs and move netnsid
>> into it. Since IFA_TARGET_NETNSID is a kernel side filter add the
>> NLM_F_DUMP_FILTERED flag so userspace knows the request was honored.
> 
> IFA_TARGET_NETNSID is not a filter.
> 
> "Filter" returns a subset of the results. It's kind of optimization
> when one is interested only in some data but not all of them. Instead
> of dumping everything, going through the results and picking only the
> data one is interested in, it's better to pass a filter and get only
> the relevant data. But you're not really required to: you can filter in
> your app.

Generically speaking a filter modifies the output based on the input.
Specifying a target namespace is an input to the dump that modifies the
output.

Yes, you can do it in userspace which is what iproute2 has done to this
point, but it is grossly inefficient and that inefficiency has
implications at scale.

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

* Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs
  2018-10-02 15:11     ` David Ahern
@ 2018-10-02 16:24       ` Jiri Benc
  0 siblings, 0 replies; 43+ messages in thread
From: Jiri Benc @ 2018-10-02 16:24 UTC (permalink / raw)
  To: David Ahern; +Cc: David Ahern, netdev, davem, christian, stephen

On Tue, 2 Oct 2018 09:11:17 -0600, David Ahern wrote:
> Generically speaking a filter modifies the output based on the input.
> Specifying a target namespace is an input to the dump that modifies the
> output.

That's conventionally called "algorithm" :-)

Let's just say we have a different understanding of what "filter" is.
Perhaps we should look at it from a different side. What is the use
case of having NLM_F_DUMP_FILTERED set for IFA_TARGET_NETNSID? How is
this going to be used by applications?

> Yes, you can do it in userspace which is what iproute2 has done to this
> point, but it is grossly inefficient and that inefficiency has
> implications at scale.

You can't do that with IFA_TARGET_NETNSID. Which is my point. Without
the flag, you don't get a list of all interfaces in all net name spaces.

 Jiri

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

* Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag
  2018-10-02 14:57         ` David Ahern
@ 2018-10-02 16:30           ` Jiri Benc
  2018-10-02 19:20             ` David Ahern
  0 siblings, 1 reply; 43+ messages in thread
From: Jiri Benc @ 2018-10-02 16:30 UTC (permalink / raw)
  To: David Ahern; +Cc: Christian Brauner, David Ahern, netdev, davem, stephen

On Tue, 2 Oct 2018 08:57:24 -0600, David Ahern wrote:
> You can when you introduce a new option or a new flag that is required
> to get new behavior like kernel side filtering.

Yes. That was what I tried with the patchset a few years back. It would
be nice to revive the effort.

> I chose a netlink flag for consistency with NLM_F_DUMP_INTR and
> NLM_F_DUMP_FILTERED. Both are netlink flags. This patch set fixes only
> what is broken -- dumps.

When we're introducing better input checking in netlink (which is a
good thing!), it would be good to do it consistently and have it
generic across all operations.

 Jiri

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

* Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag
  2018-10-02 14:55     ` David Ahern
@ 2018-10-02 16:33       ` Jiri Benc
  0 siblings, 0 replies; 43+ messages in thread
From: Jiri Benc @ 2018-10-02 16:33 UTC (permalink / raw)
  To: David Ahern; +Cc: David Ahern, netdev, davem, christian, stephen

On Tue, 2 Oct 2018 08:55:19 -0600, David Ahern wrote:
> You want take issue with a name suggest a different one.

I don't have a strong opinion on this. STRICT_HDR? HDR_CHECKED?
VERIFY_HDR? If you feel that your proposal is better, I don't mind.

Thanks,

 Jiri

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

* Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag
  2018-10-02 16:30           ` Jiri Benc
@ 2018-10-02 19:20             ` David Ahern
  0 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-02 19:20 UTC (permalink / raw)
  To: Jiri Benc; +Cc: Christian Brauner, netdev, davem, stephen

On 10/2/18 10:30 AM, Jiri Benc wrote:
> On Tue, 2 Oct 2018 08:57:24 -0600, David Ahern wrote:
>> You can when you introduce a new option or a new flag that is required
>> to get new behavior like kernel side filtering.
> 
> Yes. That was what I tried with the patchset a few years back. It would
> be nice to revive the effort.
> 
>> I chose a netlink flag for consistency with NLM_F_DUMP_INTR and
>> NLM_F_DUMP_FILTERED. Both are netlink flags. This patch set fixes only
>> what is broken -- dumps.
> 
> When we're introducing better input checking in netlink (which is a
> good thing!), it would be good to do it consistently and have it
> generic across all operations.
> 

Thinking about this some more... a setsockopt to enable the new checks
would provide a definitive way for userspace to know if the feature is
supported or not.

As for expanding the scope to NEW (and maybe DEL) commands (which was
the point of your patch set 3 years ago), I think is an idealistic goal
that is not practical to implement. Trying to go through all commands
and cover all options and valid combinations to report back errors is a
herculean effort with little return on the time investment. New commands
and new features should certainly add rigid checks for valid
combinations, but a retrospective audit on existing commands is a time
sink. Perhaps new attributes can be checked (new command on old kernel);
seems like that can be covered by a simple change to nla_parse to handle
type > maxtype based on a new input flag.

Once the flag goes in, all of the commands to be affected by it have to
be done in the same release. I have done that for the dump commands
which is fairly easy considering the existing code.

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

* Re: [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request
  2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
                   ` (24 preceding siblings ...)
  2018-10-02  0:28 ` [PATCH RFC v2 net-next 25/25] net: Enable kernel side filtering of " David Ahern
@ 2018-10-03 14:59 ` Stephen Hemminger
  2018-10-03 15:21   ` David Ahern
  25 siblings, 1 reply; 43+ messages in thread
From: Stephen Hemminger @ 2018-10-03 14:59 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, christian, jbenc, David Ahern

On Mon,  1 Oct 2018 17:28:26 -0700
David Ahern <dsahern@kernel.org> wrote:

> How to resolve the problem of not breaking old userspace yet be able to
> move forward with new features such as kernel side filtering which are
> crucial for efficient operation at high scale?

What about forward compatibility? How would this work when running new iproute2
command on older kernels?

I expect the new command would set the "I am smart flag" and the older
kernel would ignore it. The if the header for the message type had
changed, the dump would be broken.

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

* Re: [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request
  2018-10-03 14:59 ` [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request Stephen Hemminger
@ 2018-10-03 15:21   ` David Ahern
  0 siblings, 0 replies; 43+ messages in thread
From: David Ahern @ 2018-10-03 15:21 UTC (permalink / raw)
  To: Stephen Hemminger, David Ahern; +Cc: netdev, davem, christian, jbenc

On 10/3/18 8:59 AM, Stephen Hemminger wrote:
> On Mon,  1 Oct 2018 17:28:26 -0700
> David Ahern <dsahern@kernel.org> wrote:
> 
>> How to resolve the problem of not breaking old userspace yet be able to
>> move forward with new features such as kernel side filtering which are
>> crucial for efficient operation at high scale?
> 
> What about forward compatibility? How would this work when running new iproute2
> command on older kernels?
> 
> I expect the new command would set the "I am smart flag" and the older
> kernel would ignore it. The if the header for the message type had
> changed, the dump would be broken.
> 

The kernel today happily ignores garbage in the request it does not
understand. If the new iproute2 sends a dump request with attributes or
fields in the header set the kernel ignores it.

With the setsockopt option for setting the flag, userspace knows the
kernel does not support attribute checking and kernel side filtering.

As far as changing the header (new iproute2 on old kernel), there are 3
dumps that look at the header beyond the family:
1. link dumps - but it has the expected ifinfomsg header

2. neighbor dumps (expects the right ndmsg header)

3. fdb dumps - wrongly expect ifinfomsg header but there is patch to
detect when the ndmsg header is sent (ip neigh vs bridge fdb)

The 4th dump that looks at the header is addresses. Those patches were
added in this development cycle. Those dumps need to be wrapped in the
'userspace has a clue' setting or reverted until this is figured out.

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

end of thread, other threads:[~2018-10-03 22:10 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-02  0:28 [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 01/25] net/netlink: Pass extack to dump callbacks David Ahern
2018-10-02 11:07   ` Christian Brauner
2018-10-02  0:28 ` [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs David Ahern
2018-10-02 10:54   ` Jiri Benc
2018-10-02 11:03     ` Christian Brauner
2018-10-02 11:07       ` Jiri Benc
2018-10-02 11:09         ` Christian Brauner
2018-10-02 15:11     ` David Ahern
2018-10-02 16:24       ` Jiri Benc
2018-10-02  0:28 ` [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag David Ahern
2018-10-02 11:06   ` Jiri Benc
2018-10-02 11:18     ` Christian Brauner
2018-10-02 11:27       ` Jiri Benc
2018-10-02 14:57         ` David Ahern
2018-10-02 16:30           ` Jiri Benc
2018-10-02 19:20             ` David Ahern
2018-10-02 14:55     ` David Ahern
2018-10-02 16:33       ` Jiri Benc
2018-10-02  0:28 ` [PATCH RFC v2 net-next 04/25] net/ipv4: Update inet_dump_ifaddr to support NLM_F_DUMP_PROPER_HDR David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 05/25] net/ipv6: Update inet6_dump_addr " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 06/25] rtnetlink: Update rtnl_dump_ifinfo " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 07/25] rtnetlink: Update rtnl_bridge_getlink " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 08/25] rtnetlink: Update rtnl_stats_dump " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 09/25] rtnetlink: Update inet6_dump_ifinfo " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 10/25] rtnetlink: Update ipmr_rtm_dumplink " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 11/25] rtnetlink: Update fib dumps " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 12/25] net/neigh: Refactor dump filter handling David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 13/25] net/neighbor: Update neigh_dump_info to support NLM_F_DUMP_PROPER_HDR David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 14/25] net/neighbor: Update neightbl_dump_info " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 15/25] net/namespace: Update rtnl_net_dumpid " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 16/25] net/fib_rules: Update fib_nl_dumprule " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 17/25] net/ipv6: Update ip6addrlbl_dump " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 18/25] net: Update netconf dump handlers " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 19/25] net/bridge: Update br_mdb_dump " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 20/25] net: Add struct for fib dump filter David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 21/25] net/ipv4: Plumb support for filtering route dumps David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 22/25] net/ipv6: " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 23/25] net/mpls: " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 24/25] net: Plumb support for filtering ipv4 and ipv6 multicast " David Ahern
2018-10-02  0:28 ` [PATCH RFC v2 net-next 25/25] net: Enable kernel side filtering of " David Ahern
2018-10-03 14:59 ` [PATCH RFC v2 net-next 00/25] rtnetlink: Add support for rigid checking of data in dump request Stephen Hemminger
2018-10-03 15:21   ` David Ahern

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.