[ovs-dev] [patch v1] conntrack: Fix ICMPV4 error data L4 length check.
dlu998 at gmail.com
Tue Aug 27 20:22:17 UTC 2019
On Tue, Aug 27, 2019 at 2:02 AM Vishal Deep Ajmera <
vishal.deep.ajmera at ericsson.com> wrote:
> Hi Darrell,
> Thanks for the patch. When I applied the patch to latest master,
> I see that we take care of length check (< 8) only for ICMPv6 and
> not for ICMPv4.
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
to extract_l4_icmp6() rather than extract_l4_icmp()
I'll make the changes on a clean branch and resend.
JTBC, the 8 byte ICMP error data L4 length restriction is only for V4.
ICMP6 does not have this restriction; see
> We need to do it for ICMPv4 as well.
The intention is only for ICMPv4; see above.
> Also, we are already using 'related' to skip or not to skip length check.
We cannot use 'related' for this as ICMPv6 is different than ICMPv4
> * If 'related' is NULL, it means that we're already parsing a header
> * in an ICMP error. In this case, we skip checksum and length validation.
> However we continue to validate length in extract_l4_tcp (<8 or <20).
> I understand that check for minimum 8 bytes header is needed to make
> sure we can extract tcp port numbers.
No, it is a sanity check like other sanity checks.
The length check varies depending on the context.
> Can we instead try to converge all checks at one place and still take care
> of nested header? In my opinion it will simplify the code.
See the information above
> Warm Regards,
> Vishal Ajmera
More information about the dev