From patchwork Fri Dec 22 20:36:26 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Craig Gallek X-Patchwork-Id: 852561 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3z3Kzk5wN4z9s7g for ; Sat, 23 Dec 2017 07:36:34 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756621AbdLVUg3 (ORCPT ); Fri, 22 Dec 2017 15:36:29 -0500 Received: from mail-qt0-f194.google.com ([209.85.216.194]:46595 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756149AbdLVUg2 (ORCPT ); Fri, 22 Dec 2017 15:36:28 -0500 Received: by mail-qt0-f194.google.com with SMTP id r39so37589877qtr.13 for ; Fri, 22 Dec 2017 12:36:28 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=X6qc8VBkKUwDfTtSan+3uVCtDKhTVGvFplm1rnmHjHo=; b=kvu8IRQxauQwxCyMZXovsYYSWapX5puT3OIsZh+mlJYkXxHIo+A+MDsPoyVM1Sc5rn ariWEsqzbwKAfu7wPalX1h8DHtCHEdEiRJmrc8ArHYU9NH+ddjpXkOTDik+HJdBKskQP +KY/C6yCzNeK4UsalKb4I7iWzpjcJrDmGSw9/T61zOagYRzqKdSH1H4KLd7q7HhUsbWy 9c8L0wcYDp8V/LiQdWVTgQGYzGuzLMaXpJxKiS4+XtPBEQbfNXC4Z3I0WqtJzxVlbT4A 1xHhctkP12zi227y3sTDJwjMqJKEj3UfUh6ysXg8W80T1dA5i0shJuIIJAxHvBrY6k19 XawQ== X-Gm-Message-State: AKGB3mJDVjrvVJIrabcKS96FXh30VCMruhO/wipiKxi3K86uLH5kpqH0 wxAjSqiDkGWM6/FXe9rcuALLZA== X-Google-Smtp-Source: ACJfBou3XsK7o1+zM7uLO+jZyfdySMueoaFCNa+wRrn+7/q9Pyk3TI0nOVhL1sS95U7VxLIUrZvSwQ== X-Received: by 10.200.54.176 with SMTP id a45mr20664871qtc.176.1513974987932; Fri, 22 Dec 2017 12:36:27 -0800 (PST) Received: from monkey.nyc.corp.google.com ([100.101.212.120]) by smtp.gmail.com with ESMTPSA id w61sm14214023qte.55.2017.12.22.12.36.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 22 Dec 2017 12:36:27 -0800 (PST) From: Craig Gallek To: David Miller , Jiri Benc Cc: netdev@vger.kernel.org, Nicolas Dichtel , "Jason A . Donenfeld" Subject: [PATCH net v2] netns, rtnetlink: fix struct net reference leak Date: Fri, 22 Dec 2017 15:36:26 -0500 Message-Id: <20171222203626.113363-1-kraigatgoog@gmail.com> X-Mailer: git-send-email 2.15.1.620.gb9897f4670-goog Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Craig Gallek 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 CC: Nicolas Dichtel CC: Jason A. Donenfeld Signed-off-by: Craig Gallek --- 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);