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: <20080908203418.GA16637@2ka.mipt.ru>
Date:	Tue, 9 Sep 2008 00:34:18 +0400
From:	Evgeniy Polyakov <johnpol@....mipt.ru>
To:	David Miller <davem@...emloft.net>
Cc:	rdenis@...phalempin.com, akpm@...ux-foundation.org,
	netdev@...r.kernel.org, bugme-daemon@...zilla.kernel.org,
	Neil Horman <nhorman@...driver.com>
Subject: Re: [Bugme-new] [Bug 11469] New: TUN with 1024 neighbours: ip6_dst_lookup_tail NULL crash

Hi David.

On Mon, Sep 08, 2008 at 01:15:47PM -0700, David Miller (davem@...emloft.net) wrote:
> 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.

Well, yes. The whole 'optimistic' dad looks a bit suspicious. I think
failed dst entry without neighbour is a result of the 'static' dst entry
returned by the above route lookup and previously neighbour was not
used at all. This patch fixes the opps, but may be just hiding a
problem, but reading how this optimistic duplicate address detection
works, I see no strict requirements that returned route entry has to
have neighbour, so this check actually can be a right fix.

I've added Neil Horman, who created the patch 1.5 years ago, to the
copy list.

-- 
	Evgeniy Polyakov
--
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