netdev - Re: [Bugme-new] [Bug 11469] New: TUN with 1024 neighbours: ip6_dst_lookup

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]

Message-Id: <20080908.131547.26037628.davem@davemloft.net>
Date:	Mon, 08 Sep 2008 13:15:47 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	rdenis@...phalempin.com
Cc:	johnpol@....mipt.ru, akpm@...ux-foundation.org,
	netdev@...r.kernel.org, bugme-daemon@...zilla.kernel.org
Subject: Re: [Bugme-new] [Bug 11469] New: TUN with 1024 neighbours:
 ip6_dst_lookup_tail NULL crash

From: Rémi Denis-Courmont <rdenis@...phalempin.com>
Date: Sun, 7 Sep 2008 21:19:49 +0300

> Le dimanche 7 septembre 2008 21:11:09 Evgeniy Polyakov, vous avez écrit :
> > Since dst entry is allowed not to have neighbour entry, flush it just
> > like with incomplete one. This drops performance of your application
> > with more than 1024 neighbours to 1024 messages, to fix it you should
> > tune ipv6 routing parameters (gc intervals, gc threshold, maximum number
> > of entries and so on). There may be another problem with perfomance
> > though, at least I was able to bump it 10 times with different settings,
> > but still two times smaller than with 4k neighbours.
> 
> That looks like a trivial local DoS against the IPv6 stack though?
> 
> Especially in the case that the interface has IFF_NOARP, that seems like a 
> weird limitation. Oh well...

It's less of a DoS than the NULL pointer deref we get now, isn't it? :-)

But I don't like this patch for several reasons:

1) Slapping on a NULL check in response to a OOPS at that exact
   location is usually a very big red flag, and deserves high scrutiny
   instead of blind acceptance.

2) Looking at the indentation of this DAD code block (it's all one tab
   too much) it's obviously a very shitty cut and paste job.  If the
   coding style was too difficult to get right, what does this say
   about that change that brought the code here, semantically? :-/

   This means we should figure out how this code got to this place,
   and what kind of invariants existed at the old location that might
   make this dst->neighbour dereference valid, and what implications
   there are for the fact that it can now be NULL.

Really, we really need to understand much more deeply this situation.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html