Skip to contentSkip to conversation

rtnetlink: Add support for rigid checking of data in dump request

RFC PATCH Superseded David Ahern 5 participants 43 messages Oct 2, 2018 – Oct 3, 2018

Subsystems (4)
lore
Share & tools
lore
author maintainer
#10 lore raw

RFC PATCH v2 3/25 netlink: introduce NLM_F_DUMP_PROPER_HDR flag

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 include/uapi/linux/netlink.h | 1 +
 1 file changed, 1 insertion(+)
include/uapi/linux/netlink.h +1 -0
index 776bc92e9118..e722bed88dee 100644
@@ -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	*/
Signature · 1 line
2.11.0

David Ahern

author maintainer
#0 lore raw

v2 cover rtnetlink: Add support for rigid checking of data in dump request

From: David Ahern <[email protected]>

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(-)
Signature · 1 line
2.11.0
author maintainer
#1 lore raw

RFC PATCH v2 1/25 net/netlink: Pass extack to dump callbacks

From: David Ahern <[email protected]>

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.

---
 include/linux/netlink.h  |  2 ++
 net/core/rtnetlink.c     |  1 +
 net/netlink/af_netlink.c | 20 +++++++++++++++-----
 3 files changed, 18 insertions(+), 5 deletions(-)
include/linux/netlink.h +2 -0
index 71f121b66ca8..8fc90308a653 100644
@@ -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;
 };
 
net/core/rtnetlink.c +1 -0
index 35162e1b06ad..da91b38297d3 100644
@@ -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
net/netlink/af_netlink.c +15 -5
index e3a0538ec0be..7094156c94f0 100644
@@ -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);
 
Signature · 1 line
2.11.0
author maintainer
#3 lore raw

RFC PATCH v2 2/25 net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/ipv6/addrconf.c | 59 +++++++++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 27 deletions(-)
net/ipv6/addrconf.c +32 -27
index a9a317322388..375ea9d9869b 100644
@@ -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;
Signature · 1 line
2.11.0
author maintainer
#19 lore raw

RFC PATCH v2 4/25 net/ipv4: Update inet_dump_ifaddr to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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).

---
 net/ipv4/devinet.c | 52 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 11 deletions(-)
net/ipv4/devinet.c +41 -11
index 44d931a3cd50..c27537f568f0 100644
@@ -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;
+			}
 		}
 	}
 
Signature · 1 line
2.11.0
author maintainer
#20 lore raw

RFC PATCH v2 5/25 net/ipv6: Update inet6_dump_addr to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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).

---
 net/ipv6/addrconf.c | 49 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 10 deletions(-)
net/ipv6/addrconf.c +39 -10
index 375ea9d9869b..3382737df2a8 100644
@@ -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;
+			}
 		}
 	}
 
Signature · 1 line
2.11.0
author maintainer
#21 lore raw

RFC PATCH v2 6/25 rtnetlink: Update rtnl_dump_ifinfo to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/core/rtnetlink.c | 106 ++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 75 insertions(+), 31 deletions(-)
net/core/rtnetlink.c +75 -31
index da91b38297d3..2bf4b9916ca2 100644
@@ -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);
 
Signature · 1 line
2.11.0
author maintainer
#22 lore raw

RFC PATCH v2 7/25 rtnetlink: Update rtnl_bridge_getlink to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/core/rtnetlink.c | 56 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 10 deletions(-)
net/core/rtnetlink.c +46 -10
index 2bf4b9916ca2..51a653b810be 100644
@@ -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;
Signature · 1 line
2.11.0
author maintainer
#23 lore raw

RFC PATCH v2 8/25 rtnetlink: Update rtnl_stats_dump to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/core/rtnetlink.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)
net/core/rtnetlink.c +21 -1
index 51a653b810be..1751baf0c823 100644
@@ -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;
Signature · 1 line
2.11.0
author maintainer
#24 lore raw

RFC PATCH v2 9/25 rtnetlink: Update inet6_dump_ifinfo to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/ipv6/addrconf.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)
net/ipv6/addrconf.c +37 -1
index 3382737df2a8..eb6fd5fbac80 100644
@@ -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:
Signature · 1 line
2.11.0
author maintainer
#25 lore raw

RFC PATCH v2 10/25 rtnetlink: Update ipmr_rtm_dumplink to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/ipv4/ipmr.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
net/ipv4/ipmr.c +32 -0
index 5660adcf7a04..a706e9269e8c 100644
@@ -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];
 
Signature · 1 line
2.11.0
author maintainer
#26 lore raw

RFC PATCH v2 11/25 rtnetlink: Update fib dumps to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 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(-)
include/net/ip_fib.h +2 -0
index f7c109e37298..9846b79c9ee1 100644
@@ -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 */
net/ipv4/fib_frontend.c +41 -2

57 lines not on this page · raw message

net/ipv4/ipmr.c +9 -0
index a706e9269e8c..91b5991ed536 100644
@@ -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);
 }
net/ipv6/ip6_fib.c +8 -0
index 5516f55e214b..fc14733fbad8 100644
@@ -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];
 
net/ipv6/ip6mr.c +9 -0
index d0b7e0249c13..aa668214edc2 100644
@@ -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);
 }
net/mpls/af_mpls.c +8 -0
index 8fbe6cdbe255..812100a5a906 100644
@@ -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;
Signature · 1 line
2.11.0
author maintainer
#27 lore raw

RFC PATCH v2 12/25 net/neigh: Refactor dump filter handling

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/core/neighbour.c | 65 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 28 deletions(-)
net/core/neighbour.c +37 -28
index 20e0d3308148..9bab9ae9c98e 100644
@@ -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;
 	}
Signature · 1 line
2.11.0
author maintainer
#28 lore raw

RFC PATCH v2 13/25 net/neighbor: Update neigh_dump_info to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/core/neighbour.c | 61 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 10 deletions(-)
net/core/neighbour.c +51 -10
index 9bab9ae9c98e..aaf2526e5da4 100644
@@ -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++) {
Signature · 1 line
2.11.0
author maintainer
#29 lore raw

RFC PATCH v2 14/25 net/neighbor: Update neightbl_dump_info to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/core/neighbour.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)
net/core/neighbour.c +25 -3
index aaf2526e5da4..8488f2e2c865 100644
@@ -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;
Signature · 1 line
2.11.0
author maintainer
#30 lore raw

RFC PATCH v2 15/25 net/namespace: Update rtnl_net_dumpid to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/core/net_namespace.c | 8 ++++++++
 1 file changed, 8 insertions(+)
net/core/net_namespace.c +8 -0
index 670c84b1bfc2..0c3a978efc57 100644
@@ -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);
Signature · 1 line
2.11.0
author maintainer
#31 lore raw

RFC PATCH v2 16/25 net/fib_rules: Update fib_nl_dumprule to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/core/fib_rules.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)
net/core/fib_rules.c +24 -1
index 0ff3953f64aa..e9f9dc501b2e 100644
@@ -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);
Signature · 1 line
2.11.0
author maintainer
#33 lore raw

RFC PATCH v2 18/25 net: Update netconf dump handlers to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/ipv4/devinet.c  | 22 +++++++++++++++++++---
 net/ipv6/addrconf.c | 22 +++++++++++++++++++---
 net/mpls/af_mpls.c  | 18 +++++++++++++++++-
 3 files changed, 55 insertions(+), 7 deletions(-)
net/ipv4/devinet.c +19 -3
index c27537f568f0..d7859b358cc6 100644
@@ -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;
net/ipv6/addrconf.c +19 -3
index eb6fd5fbac80..34b5daa9e977 100644
@@ -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;
net/mpls/af_mpls.c +17 -1
index 812100a5a906..64de2f8c3847 100644
@@ -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) {
Signature · 1 line
2.11.0
author maintainer
#32 lore raw

RFC PATCH v2 17/25 net/ipv6: Update ip6addrlbl_dump to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/ipv6/addrlabel.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)
net/ipv6/addrlabel.c +24 -1
index 1d6ced37ad71..89e15ed78c60 100644
@@ -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)
Signature · 1 line
2.11.0
author maintainer
#34 lore raw

RFC PATCH v2 19/25 net/bridge: Update br_mdb_dump to support NLM_F_DUMP_PROPER_HDR

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/bridge/br_mdb.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
net/bridge/br_mdb.c +18 -2
index a4a848bf827b..57c43c1b1e71 100644
@@ -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;
 
Signature · 1 line
2.11.0
author maintainer
#35 lore raw

RFC PATCH v2 20/25 net: Add struct for fib dump filter

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 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(-)
include/net/ip6_route.h +1 -0
index 7b9c82de11cc..ecaba26b3399 100644
@@ -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);
include/net/ip_fib.h +12 -0
index 9846b79c9ee1..d0cd838ca00c 100644
@@ -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 */
net/ipv4/fib_frontend.c +3 -1
index c608b393ae49..9d872a4900cd 100644
@@ -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;
 	}
net/ipv4/ipmr.c +2 -1
index 91b5991ed536..9e9ad60dff6b 100644
@@ -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;
net/ipv6/ip6_fib.c +2 -2
index fc14733fbad8..e0362a21737f 100644
@@ -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;
net/ipv6/ip6mr.c +2 -1
index aa668214edc2..b3084b2c8f88 100644
@@ -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;
net/mpls/af_mpls.c +2 -1
index 64de2f8c3847..f94d1db63eb5 100644
@@ -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;
Signature · 1 line
2.11.0
author maintainer
#36 lore raw

RFC PATCH v2 21/25 net/ipv4: Plumb support for filtering route dumps

In reply to David Ahern

From: David Ahern <[email protected]>

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

---
 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(-)
include/net/ip_fib.h +1 -1
index d0cd838ca00c..e064c37a2a9f 100644
@@ -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);
net/ipv4/fib_frontend.c +12 -1
index 9d872a4900cd..a3f4073e509a 100644
@@ -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;
net/ipv4/fib_trie.c +22 -11
index 5bc0c89e81e4..0e7b4233851a 100644
@@ -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;
Signature · 1 line
2.11.0
author maintainer
#37 lore raw

RFC PATCH v2 22/25 net/ipv6: Plumb support for filtering route dumps

In reply to David Ahern

From: David Ahern <[email protected]>

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.

---
 net/ipv6/ip6_fib.c | 13 +++++++++++++
 net/ipv6/route.c   | 36 +++++++++++++++++++++++++++---------
 2 files changed, 40 insertions(+), 9 deletions(-)
net/ipv6/ip6_fib.c +13 -0
index e0362a21737f..15b9806270c1 100644
@@ -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;
net/ipv6/route.c +27 -9
index d28f83e01593..99ba2313c380 100644
@@ -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,
Signature · 1 line
2.11.0
author maintainer
#38 lore raw

RFC PATCH v2 23/25 net/mpls: Plumb support for filtering route dumps

In reply to David Ahern

From: David Ahern <[email protected]>

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

---
 net/mpls/af_mpls.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)
net/mpls/af_mpls.c +54 -1
index f94d1db63eb5..4dd8a2a026e7 100644
@@ -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)
Signature · 1 line
2.11.0
author maintainer
#39 lore raw

RFC PATCH v2 24/25 net: Plumb support for filtering ipv4 and ipv6 multicast route dumps

In reply to David Ahern

From: David Ahern <[email protected]>

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

---
 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(-)
include/linux/mroute_base.h +3 -2
index 6675b9f81979..8fc516c47a64 100644
@@ -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;
 }
net/ipv4/ipmr.c +1 -1
index 9e9ad60dff6b..2fe24009439a 100644
@@ -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] = {
net/ipv4/ipmr_base.c +41 -1

70 lines not on this page · raw message

net/ipv6/ip6mr.c +1 -1
index b3084b2c8f88..08e2443ca0cc 100644
@@ -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);
 }
Signature · 1 line
2.11.0
author maintainer
#40 lore raw

RFC PATCH v2 25/25 net: Enable kernel side filtering of route dumps

In reply to David Ahern

From: David Ahern <[email protected]>

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

---
 net/ipv4/fib_frontend.c | 42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)
net/ipv4/fib_frontend.c +30 -12
index a3f4073e509a..d1ef1cb98139 100644
@@ -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;
Signature · 1 line
2.11.0
Nacked-by on v2 2/25 NAK
#4 lore raw

Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs

In reply to David Ahern
Quoted 3 lines · David Ahern

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.

Jiri

#5 lore raw

Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs

Quoted 6 lines · Jiri Benc
Quoted 3 lines · David Ahern

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.

Quoted 2 lines

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

That's an argument I can buy.

Quoted 19 lines

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.

Jiri

#11 lore raw

Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag

In reply to David Ahern
Quoted 3 lines · David Ahern

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

#2 lore raw

Re: [PATCH RFC v2 net-next 01/25] net/netlink: Pass extack to dump callbacks

In reply to David Ahern
Quoted 8 lines · David Ahern

From: David Ahern <[email protected]>

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.

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

Quoted 115 lines
---
 include/linux/netlink.h  |  2 ++
 net/core/rtnetlink.c     |  1 +
 net/netlink/af_netlink.c | 20 +++++++++++++++-----
 3 files changed, 18 insertions(+), 5 deletions(-)
include/linux/netlink.h +2 -0
index 71f121b66ca8..8fc90308a653 100644
@@ -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;
 };
 
net/core/rtnetlink.c +1 -0
index 35162e1b06ad..da91b38297d3 100644
@@ -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
net/netlink/af_netlink.c +15 -5
index e3a0538ec0be..7094156c94f0 100644
@@ -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);
 
Signature · 1 line
2.11.0
#6 lore raw

Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs

In reply to Christian Brauner
Quoted 1 line · Christian Brauner

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

#7 lore raw

Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs

Quoted 5 lines · Jiri Benc
Quoted 1 line · Christian Brauner

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

#12 lore raw

Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag

In reply to Jiri Benc
Quoted 11 lines · Jiri Benc
Quoted 3 lines · David Ahern

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

Quoted 13 lines

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

#13 lore raw

Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag

Quoted 1 line · Christian Brauner

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.

Quoted 7 lines

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.

Quoted 3 lines

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

#17 lore raw

Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag

In reply to Jiri Benc
Quoted 8 lines · Jiri Benc
Quoted 3 lines · David Ahern

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.

Quoted 10 lines

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.

Quoted 4 lines

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.

#14 lore raw

Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag

In reply to Jiri Benc
Quoted 8 lines · Jiri Benc
Quoted 1 line · Christian Brauner

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.

Quoted 2 lines

What I find interesting and convincing is one of Dave's points:

Quoted 7 lines
Quoted 5 lines

"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.

Quoted 6 lines
Quoted 4 lines

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.

#8 lore raw

Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs

In reply to Jiri Benc
Quoted 5 lines · Jiri Benc
Quoted 3 lines · David Ahern

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.

Quoted 8 lines

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.

#9 lore raw

Re: [PATCH RFC v2 net-next 02/25] net/ipv6: Refactor address dump to push inet6_fill_args to in6_dump_addrs

Quoted 3 lines · David Ahern

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?

Quoted 3 lines

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

#15 lore raw

Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag

In reply to David Ahern
Quoted 2 lines · David Ahern

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.

Quoted 3 lines

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

#18 lore raw

Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag

In reply to David Ahern
Quoted 1 line · David Ahern

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

#16 lore raw

Re: [PATCH RFC v2 net-next 03/25] netlink: introduce NLM_F_DUMP_PROPER_HDR flag

In reply to Jiri Benc
Quoted 7 lines · Jiri Benc
Quoted 2 lines · David Ahern

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.

Quoted 8 lines
Quoted 3 lines

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.

#41 lore raw
In reply to David Ahern
Quoted 3 lines · On Mon, 1 Oct 2018 17:28:26 -0700 David Ahern <[email protected]> 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.

#42 lore raw
Quoted 7 lines · Stephen Hemminger
Quoted 3 lines · On Mon, 1 Oct 2018 17:28:26 -0700 David Ahern <[email protected]> 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?

Quoted 7 lines

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.