On Fri, Aug 23, 2019 at 9:09 AM Darrell Ball <[email protected]> wrote:
> Thanks for the patch > > Goes back to release 2.6/day one :-). > > I'll provide more feedback after today. > I sent an alternative patch here https://mail.openvswitch.org/pipermail/ovs-dev/2019-August/362013.html pls take a look. > > On Fri, Aug 23, 2019 at 6:20 AM Vishal Deep Ajmera < > [email protected]> wrote: > >> An ICMP packet with type destination or host not reachable also carries >> 28 bytes of ICMP data field. This data field contains IP header and TCP >> header (partial first 8 bytes) of the original packet for which ICMP >> is being generated. >> >> Conntrack module when processing these ICMP packets checks for TCP header >> length (20 bytes). Since TCP header is partial the length check fails and >> packet is erroneously dropped. >> >> This patch fixes length check for TCP header when processing ICMP data >> fields. >> >> Signed-off-by: Vishal Deep Ajmera <[email protected]> >> --- >> lib/conntrack.c | 14 +++++++++++--- >> lib/packets.h | 1 + >> 2 files changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/lib/conntrack.c b/lib/conntrack.c >> index 5f60fea..0618fdd 100644 >> --- a/lib/conntrack.c >> +++ b/lib/conntrack.c >> @@ -1513,10 +1513,18 @@ check_l4_icmp6(const struct conn_key *key, const >> void *data, size_t size, >> return validate_checksum ? checksum_valid(key, data, size, l3) : >> true; >> } >> >> +/* If related is NULL, we are parsing nested TCP header inside ICMP >> packet. >> + * Only 8 bytes of TCP header is required by RFC to be present in such >> case. >> + */ >> static inline bool >> -extract_l4_tcp(struct conn_key *key, const void *data, size_t size) >> +extract_l4_tcp(struct conn_key *key, const void *data, size_t size, >> + bool *related) >> { >> - if (OVS_UNLIKELY(size < TCP_HEADER_LEN)) { >> + if (!related) { >> + if (size < ICMP_L4_DATA_LEN) { >> + return false; >> + } >> + } else if (size < TCP_HEADER_LEN) { >> return false; >> } >> >> @@ -1750,7 +1758,7 @@ extract_l4(struct conn_key *key, const void *data, >> size_t size, bool *related, >> { >> if (key->nw_proto == IPPROTO_TCP) { >> return (!related || check_l4_tcp(key, data, size, l3, >> - validate_checksum)) && extract_l4_tcp(key, data, size); >> + validate_checksum)) && extract_l4_tcp(key, data, size, >> related); >> } else if (key->nw_proto == IPPROTO_UDP) { >> return (!related || check_l4_udp(key, data, size, l3, >> validate_checksum)) && extract_l4_udp(key, data, size); >> diff --git a/lib/packets.h b/lib/packets.h >> index a4bee38..2bc65c9 100644 >> --- a/lib/packets.h >> +++ b/lib/packets.h >> @@ -886,6 +886,7 @@ struct tcp_header { >> ovs_be16 tcp_urg; >> }; >> BUILD_ASSERT_DECL(TCP_HEADER_LEN == sizeof(struct tcp_header)); >> +#define ICMP_L4_DATA_LEN 8 >> >> /* Connection states. >> * >> -- >> 1.9.1 >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
