diff mbox series

[v3] rtnetlink: fix data overflow in rtnl_calcit()

Message ID 20201021020053.1401-1-zhudi21@huawei.com
State Accepted
Delegated to: David Miller
Headers show
Series [v3] rtnetlink: fix data overflow in rtnl_calcit() | expand

Checks

Context Check Description
jkicinski/cover_letter success Link
jkicinski/fixes_present success Link
jkicinski/patch_count success Link
jkicinski/tree_selection success Guessed tree name to be net-next
jkicinski/subject_prefix warning Target tree name not specified in the subject
jkicinski/source_inline success Was 0 now: 0
jkicinski/verify_signedoff success Link
jkicinski/module_param success Was 0 now: 0
jkicinski/build_32bit success Errors and warnings before: 9362 this patch: 9362
jkicinski/kdoc success Errors and warnings before: 0 this patch: 0
jkicinski/verify_fixes success Link
jkicinski/checkpatch success total: 0 errors, 0 warnings, 0 checks, 43 lines checked
jkicinski/build_allmodconfig_warn success Errors and warnings before: 9860 this patch: 9860
jkicinski/header_inline success Link
jkicinski/stable success Stable not CCed

Commit Message

zhudi Oct. 21, 2020, 2 a.m. UTC 
From: Di Zhu <zhudi21@huawei.com>

"ip addr show" command execute error when we have a physical
network card with a large number of VFs

The return value of if_nlmsg_size() in rtnl_calcit() will exceed
range of u16 data type when any network cards has a larger number of
VFs. rtnl_vfinfo_size() will significant increase needed dump size when
the value of num_vfs is larger.

Eventually we get a wrong value of min_ifinfo_dump_size because of overflow
which decides the memory size needed by netlink dump and netlink_dump()
will return -EMSGSIZE because of not enough memory was allocated.

So fix it by promoting  min_dump_alloc data type to u32 to
avoid whole netlink message size overflow and it's also align
with the data type of struct netlink_callback{}.min_dump_alloc
which is assigned by return value of rtnl_calcit()

Signed-off-by: Di Zhu <zhudi21@huawei.com>
---
/* v1 */
Link: https://lore.kernel.org/netdev/20201019101555.64b9f723/

/* v2 */
- Jakub Kicinski <kuba@kernel.org>
  - sorted the variable declarations from longest to shortest.
  - use size_t for the local variable in rtnl_calcit().
- update commit message

/* v3 */
- fix a warning reported by checkpatch.pl

---
 include/linux/netlink.h |  2 +-
 net/core/rtnetlink.c    | 13 ++++++-------
 2 files changed, 7 insertions(+), 8 deletions(-)

Comments

Jakub Kicinski Oct. 22, 2020, 1:31 a.m. UTC | #1 
On Wed, 21 Oct 2020 10:00:53 +0800 zhudi wrote:
> From: Di Zhu <zhudi21@huawei.com>
> 
> "ip addr show" command execute error when we have a physical
> network card with a large number of VFs
> 
> The return value of if_nlmsg_size() in rtnl_calcit() will exceed
> range of u16 data type when any network cards has a larger number of
> VFs. rtnl_vfinfo_size() will significant increase needed dump size when
> the value of num_vfs is larger.
> 
> Eventually we get a wrong value of min_ifinfo_dump_size because of overflow
> which decides the memory size needed by netlink dump and netlink_dump()
> will return -EMSGSIZE because of not enough memory was allocated.
> 
> So fix it by promoting  min_dump_alloc data type to u32 to
> avoid whole netlink message size overflow and it's also align
> with the data type of struct netlink_callback{}.min_dump_alloc
> which is assigned by return value of rtnl_calcit()
> 
> Signed-off-by: Di Zhu <zhudi21@huawei.com>

Applied, thanks!
diff mbox series

Patch

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 666cd0390699..9f118771e248 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -240,7 +240,7 @@  struct netlink_dump_control {
 	int (*done)(struct netlink_callback *);
 	void *data;
 	struct module *module;
-	u16 min_dump_alloc;
+	u32 min_dump_alloc;
 };
 
 int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 68e0682450c6..7d7223691783 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3709,13 +3709,13 @@  static int rtnl_dellinkprop(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return rtnl_linkprop(RTM_DELLINKPROP, skb, nlh, extack);
 }
 
-static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
+static u32 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
 	struct net *net = sock_net(skb->sk);
-	struct net_device *dev;
+	size_t min_ifinfo_dump_size = 0;
 	struct nlattr *tb[IFLA_MAX+1];
 	u32 ext_filter_mask = 0;
-	u16 min_ifinfo_dump_size = 0;
+	struct net_device *dev;
 	int hdrlen;
 
 	/* Same kernel<->userspace interface hack as in rtnl_dump_ifinfo. */
@@ -3735,9 +3735,8 @@  static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
 	 */
 	rcu_read_lock();
 	for_each_netdev_rcu(net, dev) {
-		min_ifinfo_dump_size = max_t(u16, min_ifinfo_dump_size,
-					     if_nlmsg_size(dev,
-						           ext_filter_mask));
+		min_ifinfo_dump_size = max(min_ifinfo_dump_size,
+					   if_nlmsg_size(dev, ext_filter_mask));
 	}
 	rcu_read_unlock();
 
@@ -5494,7 +5493,7 @@  static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (kind == 2 && nlh->nlmsg_flags&NLM_F_DUMP) {
 		struct sock *rtnl;
 		rtnl_dumpit_func dumpit;
-		u16 min_dump_alloc = 0;
+		u32 min_dump_alloc = 0;
 
 		link = rtnl_get_link(family, type);
 		if (!link || !link->dumpit) {