Netdev List
 help / color / mirror / Atom feed
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
To: Craig Gallek <kraigatgoog@gmail.com>,
	David Miller <davem@davemloft.net>, Jiri Benc <jbenc@redhat.com>
Cc: netdev@vger.kernel.org, "Jason A . Donenfeld" <Jason@zx2c4.com>
Subject: Re: [PATCH net] rtnetlink: fix struct net reference leak
Date: Fri, 22 Dec 2017 09:11:25 +0100	[thread overview]
Message-ID: <60dfb17d-3da8-ea53-cf9f-912dca52889b@6wind.com> (raw)
In-Reply-To: <20171221221832.89616-1-kraigatgoog@gmail.com>

Le 21/12/2017 à 23:18, Craig Gallek a écrit :
> From: Craig Gallek <kraig@google.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.


Regards,
Nicolas

  reply	other threads:[~2017-12-22  8:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-21 22:18 [PATCH net] rtnetlink: fix struct net reference leak Craig Gallek
2017-12-22  8:11 ` Nicolas Dichtel [this message]
2017-12-22 13:59   ` Craig Gallek
2017-12-22 19:14     ` 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=60dfb17d-3da8-ea53-cf9f-912dca52889b@6wind.com \
    --to=nicolas.dichtel@6wind.com \
    --cc=Jason@zx2c4.com \
    --cc=davem@davemloft.net \
    --cc=jbenc@redhat.com \
    --cc=kraigatgoog@gmail.com \
    --cc=netdev@vger.kernel.org \
    /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