[PATCH net 0/4] net: Fixups for recent dump filtering changes

[PATCH net 0/4] net: Fixups for recent dump filtering changes

[David Ahern]

From: David Ahern <dsahern@gmail.com>

Li RongQing noted that tgt_net is leaked in ipv4 due to the recent change
to handle address dumps for a specific device. The report also applies to
ipv6 and other error paths. Patches 1 and 2 fix those leaks.

Patch 3 stops route dumps from erroring out when dumping across address
families and a table id is given. This is needed in preparation for
patch 4.

Patch 4 updates the rtnl_dump_all to handle a failure in one of the dumpit
functions. At the moment, if an address dump returns an error the dump all
loop breaks but the error is dropped. The result can be no data is returned
and no error either leaving the user wondering about the addresses.

Patches were tested with a modified iproute2 to add invalid data to the
dump request causing each specific failure path to be hit in addition
to positive testing that it works as it should when given valid data.

David Ahern (4):
  net/ipv4: Put target net when address dump fails due to bad attributes
  net/ipv6: Put target net when address dump fails due to bad attributes
  net: Don't return invalid table id error when dumping all families
  net: rtnl_dump_all needs to propagate error from dumpit function

 include/net/ip_fib.h    |  1 +
 net/core/rtnetlink.c    |  6 ++++--
 net/ipv4/devinet.c      | 13 ++++++++-----
 net/ipv4/fib_frontend.c |  4 ++++
 net/ipv4/ipmr.c         |  3 +++
 net/ipv6/addrconf.c     | 14 ++++++++------
 net/ipv6/ip6_fib.c      |  3 +++
 net/ipv6/ip6mr.c        |  3 +++
 8 files changed, 34 insertions(+), 13 deletions(-)

-- 
2.11.0

[PATCH net 1/4] net/ipv4: Put target net when address dump fails due to bad attributes

[David Ahern]

From: David Ahern <dsahern@gmail.com>

If tgt_net is set based on IFA_TARGET_NETNSID attribute in the dump
request, make sure all error paths call put_net.

Fixes: 5fcd266a9f64 ("net/ipv4: Add support for dumping addresses for a specific device")
Fixes: c33078e3dfb1 ("net/ipv4: Update inet_dump_ifaddr for strict data checking")
Reported-by: Li RongQing <lirongqing@baidu.com>
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/devinet.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 63d5b58fbfdb..9250b309c742 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1761,7 +1761,7 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 	struct net_device *dev;
 	struct in_device *in_dev;
 	struct hlist_head *head;
-	int err;
+	int err = 0;
 
 	s_h = cb->args[0];
 	s_idx = idx = cb->args[1];
@@ -1771,12 +1771,15 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 		err = inet_valid_dump_ifaddr_req(nlh, &fillargs, &tgt_net,
 						 skb->sk, cb);
 		if (err < 0)
-			return err;
+			goto put_tgt_net;
 
+		err = 0;
 		if (fillargs.ifindex) {
 			dev = __dev_get_by_index(tgt_net, fillargs.ifindex);
-			if (!dev)
-				return -ENODEV;
+			if (!dev) {
+				err = -ENODEV;
+				goto put_tgt_net;
+			}
 
 			in_dev = __in_dev_get_rtnl(dev);
 			if (in_dev) {
@@ -1821,7 +1824,7 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 	if (fillargs.netnsid >= 0)
 		put_net(tgt_net);
 
-	return skb->len;
+	return err < 0 ? err : skb->len;
 }
 
 static void rtmsg_ifa(int event, struct in_ifaddr *ifa, struct nlmsghdr *nlh,
-- 
2.11.0

[PATCH net 2/4] net/ipv6: Put target net when address dump fails due to bad attributes

[David Ahern]

From: David Ahern <dsahern@gmail.com>

If tgt_net is set based on IFA_TARGET_NETNSID attribute in the dump
request, make sure all error paths call put_net.

Fixes: 6371a71f3a3b ("net/ipv6: Add support for dumping addresses for a specific device")
Fixes: ed6eff11790a ("net/ipv6: Update inet6_dump_addr for strict data checking")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv6/addrconf.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 45b84dd5c4eb..7eb09c86fa13 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5089,23 +5089,25 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 	struct net_device *dev;
 	struct inet6_dev *idev;
 	struct hlist_head *head;
+	int err = 0;
 
 	s_h = cb->args[0];
 	s_idx = idx = cb->args[1];
 	s_ip_idx = cb->args[2];
 
 	if (cb->strict_check) {
-		int err;
-
 		err = inet6_valid_dump_ifaddr_req(nlh, &fillargs, &tgt_net,
 						  skb->sk, cb);
 		if (err < 0)
-			return err;
+			goto put_tgt_net;
 
+		err = 0;
 		if (fillargs.ifindex) {
 			dev = __dev_get_by_index(tgt_net, fillargs.ifindex);
-			if (!dev)
-				return -ENODEV;
+			if (!dev) {
+				err = -ENODEV;
+				goto put_tgt_net;
+			}
 			idev = __in6_dev_get(dev);
 			if (idev) {
 				err = in6_dump_addrs(idev, skb, cb, s_ip_idx,
@@ -5144,7 +5146,7 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 	if (fillargs.netnsid >= 0)
 		put_net(tgt_net);
 
-	return skb->len;
+	return err < 0 ? err : skb->len;
 }
 
 static int inet6_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
-- 
2.11.0

[PATCH net 3/4] net: Don't return invalid table id error when dumping all families

[David Ahern]

From: David Ahern <dsahern@gmail.com>

When doing a route dump across all address families, do not error out
if the table does not exist. This allows a route dump for AF_UNSPEC
with a table id that may only exist for some of the families.

Do return the table does not exist error if dumping routes for a
specific family and the table does not exist.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/ip_fib.h    | 1 +
 net/ipv4/fib_frontend.c | 4 ++++
 net/ipv4/ipmr.c         | 3 +++
 net/ipv6/ip6_fib.c      | 3 +++
 net/ipv6/ip6mr.c        | 3 +++
 5 files changed, 14 insertions(+)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index e8d9456bf36e..c5969762a8f4 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -226,6 +226,7 @@ struct fib_dump_filter {
 	u32			table_id;
 	/* filter_set is an optimization that an entry is set */
 	bool			filter_set;
+	bool			dump_all_families;
 	unsigned char		protocol;
 	unsigned char		rt_type;
 	unsigned int		flags;
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 5bf653f36911..6df95be96311 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -829,6 +829,7 @@ int ip_valid_fib_dump_req(struct net *net, const struct nlmsghdr *nlh,
 		return -EINVAL;
 	}
 
+	filter->dump_all_families = (rtm->rtm_family == AF_UNSPEC);
 	filter->flags    = rtm->rtm_flags;
 	filter->protocol = rtm->rtm_protocol;
 	filter->rt_type  = rtm->rtm_type;
@@ -899,6 +900,9 @@ static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 	if (filter.table_id) {
 		tb = fib_get_table(net, filter.table_id);
 		if (!tb) {
+			if (filter.dump_all_families)
+				return skb->len;
+
 			NL_SET_ERR_MSG(cb->extack, "ipv4: FIB table does not exist");
 			return -ENOENT;
 		}
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 7a3e2acda94c..a6defbec4f1b 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2542,6 +2542,9 @@ static int ipmr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb)
 
 		mrt = ipmr_get_table(sock_net(skb->sk), filter.table_id);
 		if (!mrt) {
+			if (filter.dump_all_families)
+				return skb->len;
+
 			NL_SET_ERR_MSG(cb->extack, "ipv4: MR table does not exist");
 			return -ENOENT;
 		}
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 2a058b408a6a..1b8bc008b53b 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -620,6 +620,9 @@ static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 	if (arg.filter.table_id) {
 		tb = fib6_get_table(net, arg.filter.table_id);
 		if (!tb) {
+			if (arg.filter.dump_all_families)
+				return skb->len;
+
 			NL_SET_ERR_MSG_MOD(cb->extack, "FIB table does not exist");
 			return -ENOENT;
 		}
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index c3317ffb09eb..e2ea691e42c6 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -2473,6 +2473,9 @@ static int ip6mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb)
 
 		mrt = ip6mr_get_table(sock_net(skb->sk), filter.table_id);
 		if (!mrt) {
+			if (filter.dump_all_families)
+				return skb->len;
+
 			NL_SET_ERR_MSG_MOD(cb->extack, "MR table does not exist");
 			return -ENOENT;
 		}
-- 
2.11.0

[PATCH net 4/4] net: rtnl_dump_all needs to propagate error from dumpit function

[David Ahern]

From: David Ahern <dsahern@gmail.com>

If an address, route or netconf dump request is sent for AF_UNSPEC, then
rtnl_dump_all is used to do the dump across all address families. If one
of the dumpit functions fails (e.g., invalid attributes in the dump
request) then rtnl_dump_all needs to propagate that error so the user
gets an appropriate response instead of just getting no data.

Fixes: effe67926624 ("net: Enable kernel side filtering of route dumps")
Fixes: 5fcd266a9f64 ("net/ipv4: Add support for dumping addresses for a specific device")
Fixes: 6371a71f3a3b ("net/ipv6: Add support for dumping addresses for a specific device")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/rtnetlink.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 0958c7be2c22..f679c7a7d761 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3333,6 +3333,7 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
 	int idx;
 	int s_idx = cb->family;
 	int type = cb->nlh->nlmsg_type - RTM_BASE;
+	int ret = 0;
 
 	if (s_idx == 0)
 		s_idx = 1;
@@ -3365,12 +3366,13 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
 			cb->prev_seq = 0;
 			cb->seq = 0;
 		}
-		if (dumpit(skb, cb))
+		ret = dumpit(skb, cb);
+		if (ret < 0)
 			break;
 	}
 	cb->family = idx;
 
-	return skb->len;
+	return skb->len ? : ret;
 }
 
 struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
-- 
2.11.0

Re: [PATCH net 0/4] net: Fixups for recent dump filtering changes

[David Miller]

From: David Ahern <dsahern@kernel.org>
Date: Wed, 24 Oct 2018 12:58:58 -0700

> Li RongQing noted that tgt_net is leaked in ipv4 due to the recent change
> to handle address dumps for a specific device. The report also applies to
> ipv6 and other error paths. Patches 1 and 2 fix those leaks.
> 
> Patch 3 stops route dumps from erroring out when dumping across address
> families and a table id is given. This is needed in preparation for
> patch 4.
> 
> Patch 4 updates the rtnl_dump_all to handle a failure in one of the dumpit
> functions. At the moment, if an address dump returns an error the dump all
> loop breaks but the error is dropped. The result can be no data is returned
> and no error either leaving the user wondering about the addresses.
> 
> Patches were tested with a modified iproute2 to add invalid data to the
> dump request causing each specific failure path to be hit in addition
> to positive testing that it works as it should when given valid data.

Series applied, thanks David.