netdev - Re: ip_rcv_finish() NULL pointer kernel panic

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:   Mon, 13 Mar 2017 13:30:02 -0400
From:   Dan Streetman <dan.streetman@...onical.com>
To:     David Miller <davem@...emloft.net>
Cc:     fw@...len.de, eric.dumazet@...il.com, lkml@...ene.org,
        netdev@...r.kernel.org, netfilter-devel@...r.kernel.org
Subject: Re: ip_rcv_finish() NULL pointer kernel panic

On Thu, Jan 26, 2017 at 1:04 PM, David Miller <davem@...emloft.net> wrote:
> From: Florian Westphal <fw@...len.de>
> Date: Thu, 26 Jan 2017 17:24:33 +0100
>
>> Eric Dumazet <eric.dumazet@...il.com> wrote:
>>> > Though possibly with different things not setting the "input" function
>>> > pointer in the "struct dst_entry".
>>> >
>>> > include/net/dst.h:
>>> >   496 static inline int dst_input(struct sk_buff *skb) {
>>> >   498         return skb_dst(skb)->input(skb);
>>> >   499 }
>>> >
>>> > Is there any reason not to check to see if this pointer is NULL before
>>> > blindly calling it ?
>>>
>>> Sure. It can not be NULL at this point.
>>>
>>> Just look at the code in ip_rcv_finish() :
>>>
>>> It first make sure to get a valid dst.
>>>
>>> Something is broken, probably in bridge netfilter code.
>>>
>>> I suspect the dst here points to &br->fake_rtable, and this is not
>>> expected.
>>>
>>> br_drop_fake_rtable() should have been called somewhere ...
>>
>> I think it makes sense to set dst->incoming
>> to a stub in br_netfilter_rtable_init() to just kfree_skb()+
>> WARN_ON_ONCE(), no need to add code to ip stack or crash kernel
>> due to brnf bug.
>
> That would certainly make recovery from such bugs must better.
>
> But I have to say that this netfilter bridging fake dst has caused
> several dozen bugs over the years, it is fundamentally a serious
> problem in and of itself.  It provides DST facilities by hand, in a
> static object, without using any of the usual methods for creating and
> facilitating dst objects.
>
> Therefore every time someone makes an adjustment to common dst code,
> this turd (and yes, it _is_ a turd) breaks.  Every single time.
>
> So in the long term, instead of polishing this turd, let's get rid of
> it.

I'm getting reports of this bug as well; is anyone working on removing
the bridge fake dst?