From: Florian Westphal <fw@xxxxxxxxx>
Date: Thu, 26 Jan 2017 17:24:33 +0100
> Eric Dumazet <eric.dumazet@xxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
