netdev - Re: [PATCH net] rtnetlink: fix struct net reference leak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 22 Dec 2017 08:59:42 -0500
From:   Craig Gallek <kraigatgoog@...il.com>
To:     Nicolas Dichtel <nicolas.dichtel@...nd.com>
Cc:     David Miller <davem@...emloft.net>, Jiri Benc <jbenc@...hat.com>,
        netdev <netdev@...r.kernel.org>,
        "Jason A . Donenfeld" <Jason@...c4.com>
Subject: Re: [PATCH net] rtnetlink: fix struct net reference leak

On Fri, Dec 22, 2017 at 3:11 AM, Nicolas Dichtel
<nicolas.dichtel@...nd.com> wrote:
> Le 21/12/2017 à 23:18, Craig Gallek a écrit :
>> From: Craig Gallek <kraig@...gle.com>
>>
>> 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.
Thank you, I'll respin this with NETNSA_NSID_NOT_ASSIGNED as the sentinel value.

Craig