Re: net: GPF in eth_header

Eric Dumazet [off-list ref] wrote:

On Mon, 2016-11-28 at 22:34 +0100, Florian Westphal wrote:

quoted

Eric Dumazet [off-list ref] wrote:

quoted

quoted

Might be a bug added in commit daaa7d647f81f3
("netfilter: ipv6: avoid nf_iterate recursion")

Florian, what do you think of dropping a packet that presumably was
mangled badly by nf_ct_frag6_queue() ?

ipv4 definitely frees malformed packets.
In general, I think netfilter should avoid 'silent' drops if possible
and let skb continue, but of course such skbs should not be made worse
as what we ate to begin with...

quoted

quoted

(Like about 48 byte pulled :(, and/or skb->csum changed )

I think this warrants a review of ipv6 reassembly too, bug reported here
is because ipv6 nf defrag is also done on output.

ip6_frag_queue() definitely frees bad/mangled skbs()

Yes, sorry.  nf_ct_frag6_queue is mostly derived from ip6_frag_queue
so any bugs in one might also exist in other.
Thats all I wanted to say here.  I'll check this tomorrow.

quoted

Looks good, we'll need to change some of the errno return codes in
nf_ct_frag6_gather to 0 though for this to work, which should not be too
hard ;)

If the goal is to let buggy packets pass, then we might need to undo
changes in nf_ct_frag6_queue()

It currently returns -EINVAL in cases where skb wasn't changed/altered
(e.g. because it doesn't have a fragment header), so we should ACCEPT in
that case.

As for 'buggy' packet, I think its ok to mimic ip6_frag_queue, i.e.
if it tosses returning NF_DROP under same circumstance seems ok.

(Passing however will -- on ingress side -- cause snmp stat increments
 in ipv6 reassembly, this still might be desireable).

I'll check where undo might be possible/not too hard.

Thanks Eric for debugging this!