From: Craig Gallek <kraigatgoog@gmail.com>
To: David Miller <davem@davemloft.net>, Jiri Benc <jbenc@redhat.com>
Cc: netdev@vger.kernel.org,
Nicolas Dichtel <nicolas.dichtel@6wind.com>,
"Jason A . Donenfeld" <Jason@zx2c4.com>
Subject: [PATCH net v2] netns, rtnetlink: fix struct net reference leak
Date: Fri, 22 Dec 2017 15:36:26 -0500 [thread overview]
Message-ID: <20171222203626.113363-1-kraigatgoog@gmail.com> (raw)
From: Craig Gallek <kraig@google.com>
netns ids were added in commit 0c7aecd4bde4 and defined as signed
integers in both the kernel datastructures and the netlink interface.
However, the semantics of the implementation assume that the ids
are always greater than or equal to zero, except for an internal
sentinal value NETNSA_NSID_NOT_ASSIGNED.
Several subsequent patches carried this pattern forward. This patch
updates all of the netlink input paths of this value to enforce the
'greater than or equal to zero' constraint.
This issue was discovered by syskaller. It would set a negative
value for a netns id and then repeatedly call the RTM_GETLINK.
The put_net call in that path would not trigger for negative netns ids,
caused a reference count leak, and eventually overflowed. There are
probably additional error paths that do not handle this situation
correctly, but this was the only one I was able to trigger a real
issue through.
Fixes: 0c7aecd4bde4 ("netns: add rtnl cmd to add and get peer netns ids")
Fixes: 317f4810e45e ("rtnl: allow to create device with IFLA_LINK_NETNSID set")
Fixes: 79e1ad148c84 ("rtnetlink: use netnsid to query interface")
CC: Jiri Benc <jbenc@redhat.com>
CC: Nicolas Dichtel <nicolas.dichtel@6wind.com>
CC: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Craig Gallek <kraig@google.com>
---
net/core/net_namespace.c | 2 ++
net/core/rtnetlink.c | 17 +++++++++++++----
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 60a71be75aea..4b7ea33f5705 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -627,6 +627,8 @@ static int rtnl_net_newid(struct sk_buff *skb, struct nlmsghdr *nlh,
return -EINVAL;
}
nsid = nla_get_s32(tb[NETNSA_NSID]);
+ if (nsid < 0)
+ return -EINVAL;
if (tb[NETNSA_PID]) {
peer = get_net_ns_by_pid(nla_get_u32(tb[NETNSA_PID]));
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index dabba2a91fc8..a928b8f081b8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1733,10 +1733,12 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
ifla_policy, NULL) >= 0) {
if (tb[IFLA_IF_NETNSID]) {
netnsid = nla_get_s32(tb[IFLA_IF_NETNSID]);
- tgt_net = get_target_net(skb, netnsid);
- if (IS_ERR(tgt_net)) {
- tgt_net = net;
- netnsid = -1;
+ if (netnsid >= 0) {
+ tgt_net = get_target_net(skb, netnsid);
+ if (IS_ERR(tgt_net)) {
+ tgt_net = net;
+ netnsid = -1;
+ }
}
}
@@ -2792,6 +2794,11 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (tb[IFLA_LINK_NETNSID]) {
int id = nla_get_s32(tb[IFLA_LINK_NETNSID]);
+ if (id < 0) {
+ err = -EINVAL;
+ goto out;
+ }
+
link_net = get_net_ns_by_id(dest_net, id);
if (!link_net) {
err = -EINVAL;
@@ -2883,6 +2890,8 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (tb[IFLA_IF_NETNSID]) {
netnsid = nla_get_s32(tb[IFLA_IF_NETNSID]);
+ if (netnsid < 0)
+ return -EINVAL;
tgt_net = get_target_net(skb, netnsid);
if (IS_ERR(tgt_net))
return PTR_ERR(tgt_net);
--
2.15.1.620.gb9897f4670-goog
next reply other threads:[~2017-12-22 20:36 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-22 20:36 Craig Gallek [this message]
2017-12-23 22:12 ` [PATCH net v2] netns, rtnetlink: fix struct net reference leak Nicolas Dichtel
2017-12-29 15:56 ` Craig Gallek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171222203626.113363-1-kraigatgoog@gmail.com \
--to=kraigatgoog@gmail.com \
--cc=Jason@zx2c4.com \
--cc=davem@davemloft.net \
--cc=jbenc@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=nicolas.dichtel@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox