From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Dichtel Subject: Re: [PATCH net] rtnetlink: fix struct net reference leak Date: Fri, 22 Dec 2017 09:11:25 +0100 Message-ID: <60dfb17d-3da8-ea53-cf9f-912dca52889b@6wind.com> References: <20171221221832.89616-1-kraigatgoog@gmail.com> Reply-To: nicolas.dichtel@6wind.com Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: netdev@vger.kernel.org, "Jason A . Donenfeld" To: Craig Gallek , David Miller , Jiri Benc Return-path: Received: from mail-wm0-f54.google.com ([74.125.82.54]:43538 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750961AbdLVIL3 (ORCPT ); Fri, 22 Dec 2017 03:11:29 -0500 Received: by mail-wm0-f54.google.com with SMTP id n138so20170806wmg.2 for ; Fri, 22 Dec 2017 00:11:28 -0800 (PST) In-Reply-To: <20171221221832.89616-1-kraigatgoog@gmail.com> Content-Language: fr Sender: netdev-owner@vger.kernel.org List-ID: Le 21/12/2017 à 23:18, Craig Gallek a écrit : > 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. A valid (assigned) nsid is always >= 0. > > 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). We cannot make this assumption, this is wrong. nsids may be automatically allocated by the kernel, and it starts by 0. The current netns can be identify by NETNSA_NSID_NOT_ASSIGNED, ie -1. Regards, Nicolas