From 53be16d053328941f4738d83385a9c19b2c3776a Mon Sep 17 00:00:00 2001 From: Klement Sekera Date: Tue, 15 Dec 2020 21:47:36 +0100 Subject: ip: fix possible missing trace indexes Add safeguards when tracing packets to avoid cases where clear trace was issue while buffers were held in reassembly. Type: fix Change-Id: I1bdd1e629e8bc08ce63913fd3c4b2327e47dec04 Signed-off-by: Klement Sekera --- src/vnet/ip/reass/ip4_full_reass.c | 7 +++ src/vnet/ip/reass/ip4_sv_reass.c | 7 +++ src/vnet/ip/reass/ip6_full_reass.c | 7 +++ src/vnet/ip/reass/ip6_sv_reass.c | 7 +++ test/test_reassembly.py | 101 +++++++++++++++++++++++++++++++++++++ 5 files changed, 129 insertions(+) diff --git a/src/vnet/ip/reass/ip4_full_reass.c b/src/vnet/ip/reass/ip4_full_reass.c index ecd2fb0e5c6..69d418e9d51 100644 --- a/src/vnet/ip/reass/ip4_full_reass.c +++ b/src/vnet/ip/reass/ip4_full_reass.c @@ -342,6 +342,13 @@ ip4_full_reass_add_trace (vlib_main_t * vm, vlib_node_runtime_t * node, { vlib_buffer_t *b = vlib_get_buffer (vm, bi); vnet_buffer_opaque_t *vnb = vnet_buffer (b); + if (pool_is_free_index + (vm->trace_main.trace_buffer_pool, vlib_buffer_get_trace_index (b))) + { + // this buffer's trace is gone + b->flags &= ~VLIB_BUFFER_IS_TRACED; + return; + } bool is_after_handoff = false; if (vlib_buffer_get_trace_thread (b) != vm->thread_index) { diff --git a/src/vnet/ip/reass/ip4_sv_reass.c b/src/vnet/ip/reass/ip4_sv_reass.c index c74f8e5b661..e9582638047 100644 --- a/src/vnet/ip/reass/ip4_sv_reass.c +++ b/src/vnet/ip/reass/ip4_sv_reass.c @@ -235,6 +235,13 @@ ip4_sv_reass_add_trace (vlib_main_t * vm, vlib_node_runtime_t * node, u32 ip_proto, u16 l4_src_port, u16 l4_dst_port) { vlib_buffer_t *b = vlib_get_buffer (vm, bi); + if (pool_is_free_index + (vm->trace_main.trace_buffer_pool, vlib_buffer_get_trace_index (b))) + { + // this buffer's trace is gone + b->flags &= ~VLIB_BUFFER_IS_TRACED; + return; + } ip4_sv_reass_trace_t *t = vlib_add_trace (vm, node, b, sizeof (t[0])); if (reass) { diff --git a/src/vnet/ip/reass/ip6_full_reass.c b/src/vnet/ip/reass/ip6_full_reass.c index 80a31e83215..fe5c6376882 100644 --- a/src/vnet/ip/reass/ip6_full_reass.c +++ b/src/vnet/ip/reass/ip6_full_reass.c @@ -319,6 +319,13 @@ ip6_full_reass_add_trace (vlib_main_t * vm, vlib_node_runtime_t * node, vlib_buffer_t *b = vlib_get_buffer (vm, bi); vnet_buffer_opaque_t *vnb = vnet_buffer (b); bool is_after_handoff = false; + if (pool_is_free_index + (vm->trace_main.trace_buffer_pool, vlib_buffer_get_trace_index (b))) + { + // this buffer's trace is gone + b->flags &= ~VLIB_BUFFER_IS_TRACED; + return; + } if (vlib_buffer_get_trace_thread (b) != vm->thread_index) { is_after_handoff = true; diff --git a/src/vnet/ip/reass/ip6_sv_reass.c b/src/vnet/ip/reass/ip6_sv_reass.c index c5c84e5f93a..8bc1fd85449 100644 --- a/src/vnet/ip/reass/ip6_sv_reass.c +++ b/src/vnet/ip/reass/ip6_sv_reass.c @@ -228,6 +228,13 @@ ip6_sv_reass_add_trace (vlib_main_t * vm, vlib_node_runtime_t * node, u32 ip_proto, u16 l4_src_port, u16 l4_dst_port) { vlib_buffer_t *b = vlib_get_buffer (vm, bi); + if (pool_is_free_index + (vm->trace_main.trace_buffer_pool, vlib_buffer_get_trace_index (b))) + { + // this buffer's trace is gone + b->flags &= ~VLIB_BUFFER_IS_TRACED; + return; + } ip6_sv_reass_trace_t *t = vlib_add_trace (vm, node, b, sizeof (t[0])); if (reass) { diff --git a/test/test_reassembly.py b/test/test_reassembly.py index 797fb527415..a8dc60a2df6 100644 --- a/test/test_reassembly.py +++ b/test/test_reassembly.py @@ -170,6 +170,21 @@ class TestIPv4Reassembly(VppTestCase): self.verify_capture(packets) self.src_if.assert_nothing_captured() + def test_verify_clear_trace_mid_reassembly(self): + """ verify clear trace works mid-reassembly """ + + self.pg_enable_capture() + self.src_if.add_stream(self.fragments_200[0:-1]) + self.pg_start() + + self.logger.debug(self.vapi.cli("show trace")) + self.vapi.cli("clear trace") + + self.src_if.add_stream(self.fragments_200[-1]) + self.pg_start() + packets = self.dst_if.get_capture(len(self.pkt_infos)) + self.verify_capture(packets) + def test_reversed(self): """ reverse order reassembly """ @@ -588,6 +603,42 @@ class TestIPv4SVReassembly(VppTestCase): self.assertEqual(sent[IP].dst, recvd[IP].dst) self.assertEqual(sent[Raw].payload, recvd[Raw].payload) + def test_verify_clear_trace_mid_reassembly(self): + """ verify clear trace works mid-reassembly """ + payload_len = 1000 + payload = "" + counter = 0 + while len(payload) < payload_len: + payload += "%u " % counter + counter += 1 + + p = (Ether(dst=self.src_if.local_mac, src=self.src_if.remote_mac) / + IP(id=1, src=self.src_if.remote_ip4, + dst=self.dst_if.remote_ip4) / + UDP(sport=1234, dport=5678) / + Raw(payload)) + fragments = fragment_rfc791(p, payload_len/4) + + self.pg_enable_capture() + self.src_if.add_stream(fragments[1]) + self.pg_start() + + self.logger.debug(self.vapi.cli("show trace")) + self.vapi.cli("clear trace") + + self.pg_enable_capture() + self.src_if.add_stream(fragments[0]) + self.pg_start() + self.dst_if.get_capture(2) + + self.logger.debug(self.vapi.cli("show trace")) + self.vapi.cli("clear trace") + + self.pg_enable_capture() + self.src_if.add_stream(fragments[2:]) + self.pg_start() + self.dst_if.get_capture(len(fragments[2:])) + def test_timeout(self): """ reassembly timeout """ payload_len = 1000 @@ -1073,6 +1124,21 @@ class TestIPv6Reassembly(VppTestCase): self.src_if.assert_nothing_captured() self.dst_if.assert_nothing_captured() + def test_verify_clear_trace_mid_reassembly(self): + """ verify clear trace works mid-reassembly """ + + self.pg_enable_capture() + self.src_if.add_stream(self.fragments_400[0:-1]) + self.pg_start() + + self.logger.debug(self.vapi.cli("show trace")) + self.vapi.cli("clear trace") + + self.src_if.add_stream(self.fragments_400[-1]) + self.pg_start() + packets = self.dst_if.get_capture(len(self.pkt_infos)) + self.verify_capture(packets) + def test_reversed(self): """ reverse order reassembly """ @@ -1635,6 +1701,41 @@ class TestIPv6SVReassembly(VppTestCase): self.assertEqual(sent[IPv6].dst, recvd[IPv6].dst) self.assertEqual(sent[Raw].payload, recvd[Raw].payload) + def test_verify_clear_trace_mid_reassembly(self): + """ verify clear trace works mid-reassembly """ + payload_len = 1000 + payload = "" + counter = 0 + while len(payload) < payload_len: + payload += "%u " % counter + counter += 1 + + p = (Ether(dst=self.src_if.local_mac, src=self.src_if.remote_mac) / + IPv6(src=self.src_if.remote_ip6, dst=self.dst_if.remote_ip6) / + UDP(sport=1234, dport=5678) / + Raw(payload)) + fragments = fragment_rfc8200(p, 1, payload_len/4) + + self.pg_enable_capture() + self.src_if.add_stream(fragments[1]) + self.pg_start() + + self.logger.debug(self.vapi.cli("show trace")) + self.vapi.cli("clear trace") + + self.pg_enable_capture() + self.src_if.add_stream(fragments[0]) + self.pg_start() + self.dst_if.get_capture(2) + + self.logger.debug(self.vapi.cli("show trace")) + self.vapi.cli("clear trace") + + self.pg_enable_capture() + self.src_if.add_stream(fragments[2:]) + self.pg_start() + self.dst_if.get_capture(len(fragments[2:])) + def test_timeout(self): """ reassembly timeout """ payload_len = 1000 -- cgit 1.2.3-korg