On Wed, Aug 28, 2019 at 1:43 AM Vishal Deep Ajmera < vishal.deep.ajm...@ericsson.com> wrote:
> That is interesting > > i just tried applying on top of tree and I see that the git applies some > changes (2 lines) > > in extract_l4_icmp6() rather the intended extract_l4_icmp() as in the > patch I sent out. > > My guess is that the surrounding lines are identical in the 2 functions > and I had other > > patches in the same branch shifting the patch downward, hence git applied > the changes > > to extract_l4_icmp6() rather than extract_l4_icmp() > > > > I'll make the changes on a clean branch and resend. > > > > Thanks. I applied this patch and looks ok to me. > Thanks for confirming > > > JTBC, the 8 byte ICMP error data L4 length restriction is only for V4. > > ICMP6 does not have this restriction; see > https://tools.ietf.org/html/rfc4443 > > > > In my opinion, we should limit the check to < 8 bytes even in case of > ICMPv6 as that is all > > is required from the TCP header to extract port numbers and aligns it with > ICMPv4. > > Specially because RFC is not mandating minimum size for L4 header in case > of ICMPv6. > For V6, the ICMP error L4 length can even be zero, legitimately, with a large extension header presence, theoretically; practically, these are almost certainly crafted packets. The existing check for ICMP6 is mostly permissive but reasonable. The ICMP6 sanity check can be enhanced. For the most part, it can be made more strict while handling the above corner case perfectly. I don't think we should mess with enhancing ICMP6 as part of this bug fix for ICMPv4. Thats why this patch leaves ICMP6 unmodified. > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
