From patchwork Thu Dec 21 22:18:32 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Craig Gallek X-Patchwork-Id: 852150 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 3z2mHy3wCPz9rvt for ; Fri, 22 Dec 2017 09:18:38 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753961AbdLUWSf (ORCPT ); Thu, 21 Dec 2017 17:18:35 -0500 Received: from mail-yb0-f196.google.com ([209.85.213.196]:44178 "EHLO mail-yb0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752066AbdLUWSe (ORCPT ); Thu, 21 Dec 2017 17:18:34 -0500 Received: by mail-yb0-f196.google.com with SMTP id z62so17316351yba.11 for ; Thu, 21 Dec 2017 14:18:34 -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=JnM03lvdM8FiFAsm4iU2hle9W42rvzvi/6QSRnBWT0k=; b=OtAydwARzWJzLJH/DTadIsAn8ARiksk8Y7cIPDtZ5vEvluCuNOvZMjWlvjOP4LuF+F meaw6tHhncverOUWFADAzLBfIePM9AaczDP1ysbvxZV5s0mGJ/xRMBI7keK8J87rHePm LtyCyyA2XD0+TYjyGIf+2eYReVtNmxeJxJBxzuUxoIpQKgTKlzl2cxRiMSgU9jxq0Pqg JvrvgZBauvyyQYVenpEtDK50BZXLVzaT/CQl1xNx+y2sn9WevtU/QvFjw0a1BP02G46t sJsxeIeBFGR16bibj8rIV5A9fhUqakyjNvtHRKzTK76Zf8ZeuBIfa9PJpdhtLxrqdx7n I8CQ== X-Gm-Message-State: AKGB3mIzr+3gdsrVMeXORtZAfNw7DclthyML9QrifAy33L3NxiohnDxR tUG+f4FfTpA2kzABLYm4bdsQpg== X-Google-Smtp-Source: ACJfBotF2sMMTjSZteyYwvASyTdcDTxArBr1lbqX24zwZDPIZb7NPczrnYIcA1nkk3Iupvp6B4lV0w== X-Received: by 10.37.217.146 with SMTP id q140mr642297ybg.125.1513894713817; Thu, 21 Dec 2017 14:18:33 -0800 (PST) Received: from monkey.nyc.corp.google.com ([100.101.212.120]) by smtp.gmail.com with ESMTPSA id l33sm9396394ywh.6.2017.12.21.14.18.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 21 Dec 2017 14:18:33 -0800 (PST) From: Craig Gallek To: David Miller , Jiri Benc Cc: netdev@vger.kernel.org, Nicolas Dichtel , "Jason A . Donenfeld" Subject: [PATCH net] rtnetlink: fix struct net reference leak Date: Thu, 21 Dec 2017 17:18:32 -0500 Message-Id: <20171221221832.89616-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 The below referenced commit extended the RTM_GETLINK interface to allow querying by netns id. The netnsid property was previously defined as a signed integer, but this patch assumes that the user always passes a positive integer. syzkaller discovered this problem by setting a negative netnsid and then calling the get-link path in a tight loop. This surprisingly quickly overflows the reference count on the associated struct net, potentially destroying it. When the default namespace is used, the machine crashes in strange and interesting ways. Unfortunately, this is not easy to reproduce with just the ip tool as it enforces unsigned integer parsing despite the interface interpeting the NETNSID attribute as signed. I'm not sure why this attribute is signed in the first place, but the first commit that introduced it (6621dd29eb9b) is in v4.15-rc4, so I assume it's too late to change. This patch removes the positive netns id assumption, but adds another assumption that the netns id 0 is always the 'self' identifying id (for which an additional struct net reference is not necessary). 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/rtnetlink.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index dabba2a91fc8..3de033b7e4b9 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -1451,7 +1451,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, ifm->ifi_flags = dev_get_flags(dev); ifm->ifi_change = change; - if (tgt_netnsid >= 0 && nla_put_s32(skb, IFLA_IF_NETNSID, tgt_netnsid)) + if (tgt_netnsid && nla_put_s32(skb, IFLA_IF_NETNSID, tgt_netnsid)) goto nla_put_failure; if (nla_put_string(skb, IFLA_IFNAME, dev->name) || @@ -1712,7 +1712,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb) const struct rtnl_link_ops *kind_ops = NULL; unsigned int flags = NLM_F_MULTI; int master_idx = 0; - int netnsid = -1; + int netnsid = 0; int err; int hdrlen; @@ -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) { + tgt_net = get_target_net(skb, netnsid); + if (IS_ERR(tgt_net)) { + tgt_net = net; + netnsid = 0; + } } } @@ -1786,7 +1788,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb) cb->args[0] = h; cb->seq = net->dev_base_seq; nl_dump_check_consistent(cb, nlmsg_hdr(skb)); - if (netnsid >= 0) + if (netnsid) put_net(tgt_net); return err; @@ -2873,7 +2875,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh, struct nlattr *tb[IFLA_MAX+1]; struct net_device *dev = NULL; struct sk_buff *nskb; - int netnsid = -1; + int netnsid = 0; int err; u32 ext_filter_mask = 0; @@ -2883,9 +2885,11 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh, 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)) - return PTR_ERR(tgt_net); + if (netnsid) { + tgt_net = get_target_net(skb, netnsid); + if (IS_ERR(tgt_net)) + return PTR_ERR(tgt_net); + } } if (tb[IFLA_IFNAME]) @@ -2923,7 +2927,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh, } else err = rtnl_unicast(nskb, net, NETLINK_CB(skb).portid); out: - if (netnsid >= 0) + if (netnsid) put_net(tgt_net); return err;