Fix: network instrumentation handling of corrupted TCP headers

A malformed packet may contain a valid IPv4/IPv6 header, but an
inconsistent TCP header. As a result, the trace contains a fully
formed IPv4/IPv6 header, including the "protocol" or "nexthdr"
fields indicating TCP, but no following TCP header.

This scenario leads to an unreadable CTF trace, because the
trace viewer expects a TCP header, but instead gets the next
event.

Therefore, using the IP header fields as selector for the
transport layer variant is not the right approach: introduce
our own selector field, which allows to properly deal with this
corner-case.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/instrumentation/events/lttng-module/net.h b/instrumentation/events/lttng-module/net.h
index 19a22a844e9d046e318d03b6eb95326d55bdc105..e48c4db1ea3557d975f38e31f1ca0bd513b6122d 100644 (file)
--- a/instrumentation/events/lttng-module/net.h
+++ b/instrumentation/events/lttng-module/net.h
@@ -104,11 +104,11 @@ static struct lttng_event_field transport_fields[] = {
 };
 
 enum transport_header_types {
-       TH_NONE,
-       TH_TCP,
+       TH_NONE = 0,
+       TH_TCP = 1,
 };
 
-static inline unsigned char __get_transport_header_type(struct sk_buff *skb)
+static inline enum transport_header_types __get_transport_header_type(struct sk_buff *skb)
 {
        if (__has_network_hdr(skb)) {
                /*
@@ -135,20 +135,15 @@ static inline unsigned char __get_transport_header_type(struct sk_buff *skb)
 
 static struct lttng_enum_entry transport_enum_entries[] = {
        [0] = {
-               .start = { .value = 0, .signedness = 0, },
-               .end = { .value = IPPROTO_TCP - 1, .signedness = 0, },
+               .start = { .value = TH_NONE, .signedness = 0, },
+               .end = { .value = TH_NONE, .signedness = 0, },
                .string = "_unknown",
        },
        [1] = {
-               .start = { .value = IPPROTO_TCP, .signedness = 0, },
-               .end = { .value = IPPROTO_TCP, .signedness = 0, },
+               .start = { .value = TH_TCP, .signedness = 0, },
+               .end = { .value = TH_TCP, .signedness = 0, },
                .string = "_tcp",
        },
-       [2] = {
-               .start = { .value = IPPROTO_TCP + 1, .signedness = 0, },
-               .end = { .value = 255, .signedness = 0, },
-               .string = "_unknown",
-       },
 };
 
 static const struct lttng_enum_desc transport_header_type = {
@@ -239,10 +234,25 @@ static struct lttng_event_field ipv4fields[] = {
                },
        },
        [11] = {
+               .name = "transport_header_type",
+               .type = {
+                       .atype = atype_enum,
+                       .u.basic.enumeration.desc = &transport_header_type,
+                       .u.basic.enumeration.container_type = {
+                               .size = 8,
+                               .alignment = 8,
+                               .signedness = 0,
+                               .reverse_byte_order = 0,
+                               .base = 10,
+                               .encoding = lttng_encode_none,
+                       },
+               },
+       },
+       [12] = {
                .name = "transport_header",
                .type = {
                        .atype = atype_variant,
-                       .u.variant.tag_name = "protocol",
+                       .u.variant.tag_name = "transport_header_type",
                        .u.variant.choices = transport_fields,
                        .u.variant.nr_choices = ARRAY_SIZE(transport_fields),
                },
@@ -320,10 +330,25 @@ static struct lttng_event_field ipv6fields[] = {
                },
        },
        [8] = {
+               .name = "transport_header_type",
+               .type = {
+                       .atype = atype_enum,
+                       .u.basic.enumeration.desc = &transport_header_type,
+                       .u.basic.enumeration.container_type = {
+                               .size = 8,
+                               .alignment = 8,
+                               .signedness = 0,
+                               .reverse_byte_order = 0,
+                               .base = 10,
+                               .encoding = lttng_encode_none,
+                       },
+               },
+       },
+       [9] = {
                .name = "transport_header",
                .type = {
                        .atype = atype_variant,
-                       .u.variant.tag_name = "nexthdr",
+                       .u.variant.tag_name = "transport_header_type",
                        .u.variant.choices = transport_fields,
                        .u.variant.nr_choices = ARRAY_SIZE(transport_fields),
                },
@@ -436,18 +461,22 @@ LTTNG_TRACEPOINT_EVENT_CLASS(net_dev_template,
                        ),
                        network_header,
                        ctf_custom_code(
+                               bool has_network_header = false;
+
                                /* Copy the network header. */
                                switch (__get_network_header_type(skb)) {
                                case NH_IPV4: {
                                        ctf_align(uint16_t)
                                        ctf_array_type(uint8_t, ip_hdr(skb),
                                                        sizeof(struct iphdr))
+                                       has_network_header = true;
                                        break;
                                }
                                case NH_IPV6: {
                                        ctf_align(uint16_t)
                                        ctf_array_type(uint8_t, ipv6_hdr(skb),
                                                        sizeof(struct ipv6hdr))
+                                       has_network_header = true;
                                        break;
                                }
                                default:
@@ -458,17 +487,24 @@ LTTNG_TRACEPOINT_EVENT_CLASS(net_dev_template,
                                        break;
                                }
 
-                               /* Copy the transport header. */
-                               if (__get_transport_header_type(skb)
-                                               == TH_TCP) {
-                                       ctf_align(uint32_t)
-                                       ctf_array_type(uint8_t, tcp_hdr(skb),
-                                                       sizeof(struct tcphdr))
+                               if (has_network_header) {
+                                       enum transport_header_types th_type =
+                                               __get_transport_header_type(skb);
+
+                                       /* Transport header type field. */
+                                       ctf_integer_type(unsigned char, th_type)
+
+                                       /* Copy the transport header. */
+                                       if (th_type == TH_TCP) {
+                                               ctf_align(uint32_t)
+                                               ctf_array_type(uint8_t, tcp_hdr(skb),
+                                                               sizeof(struct tcphdr))
+                                       }
+                                       /*
+                                        * For any other transport header type,
+                                        * there is nothing to do.
+                                        */
                                }
-                               /*
-                                * For any other transport header type,
-                                * there is nothing to do.
-                                */
                        )
                )
        )