From 2ef973c28c005a40970e23c1b1a998d4c5a9d1b6 Mon Sep 17 00:00:00 2001 From: Guy Arbitman Date: Sat, 16 Nov 2024 21:33:52 +0200 Subject: [PATCH] usm, npm, classification: Fix a leak in connection_protocol If USM was enabled, but classification feature of NPM was disabled, we had a leak in connection_protocol map, as both USM and NPM would have mark the connection as visible from their probes (NPM would have done that from 'update_conn_stats'), but the probe to clean the entry ('kretprobe__tcp_close_clean_protocols') wasn't enabled. To clean an entry, we need all modules to mark it as ready for deletion, but only USM would have done that. The fix it to prevent NPM from processing the protocol if classification is disabled. Hence, NPM won't mark the conneciton as accessible from it, and we will only need USM to delete it --- .../c/protocols/classification/protocol-classification.h | 6 ------ .../ebpf/c/protocols/classification/shared-tracer-maps.h | 6 ++++++ pkg/network/ebpf/c/tracer/stats.h | 4 +++- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/network/ebpf/c/protocols/classification/protocol-classification.h b/pkg/network/ebpf/c/protocols/classification/protocol-classification.h index a235138a5c718..67169936daf69 100644 --- a/pkg/network/ebpf/c/protocols/classification/protocol-classification.h +++ b/pkg/network/ebpf/c/protocols/classification/protocol-classification.h @@ -60,12 +60,6 @@ // file. If, for example, application-layer is known, calling this helper multiple // times will result in traversing only the api and encryption-layer programs -static __always_inline bool is_protocol_classification_supported() { - __u64 val = 0; - LOAD_CONSTANT("protocol_classification_enabled", val); - return val > 0; -} - // updates the the protocol stack and adds the current layer to the routing skip list static __always_inline void update_protocol_information(usm_context_t *usm_ctx, protocol_stack_t *stack, protocol_t proto) { set_protocol(stack, proto); diff --git a/pkg/network/ebpf/c/protocols/classification/shared-tracer-maps.h b/pkg/network/ebpf/c/protocols/classification/shared-tracer-maps.h index ebc1881c1f3c2..1300f2a5f999d 100644 --- a/pkg/network/ebpf/c/protocols/classification/shared-tracer-maps.h +++ b/pkg/network/ebpf/c/protocols/classification/shared-tracer-maps.h @@ -9,6 +9,12 @@ // classification procedures on the same connection BPF_HASH_MAP(connection_protocol, conn_tuple_t, protocol_stack_wrapper_t, 0) +static __always_inline bool is_protocol_classification_supported() { + __u64 val = 0; + LOAD_CONSTANT("protocol_classification_enabled", val); + return val > 0; +} + static __always_inline protocol_stack_t* __get_protocol_stack(conn_tuple_t* tuple) { protocol_stack_wrapper_t *wrapper = bpf_map_lookup_elem(&connection_protocol, tuple); if (!wrapper) { diff --git a/pkg/network/ebpf/c/tracer/stats.h b/pkg/network/ebpf/c/tracer/stats.h index cb768fa71e3e4..6a20ae65f041b 100644 --- a/pkg/network/ebpf/c/tracer/stats.h +++ b/pkg/network/ebpf/c/tracer/stats.h @@ -148,7 +148,9 @@ static __always_inline void update_conn_stats(conn_tuple_t *t, size_t sent_bytes return; } - update_protocol_classification_information(t, val); + if (is_protocol_classification_supported()) { + update_protocol_classification_information(t, val); + } // If already in our map, increment size in-place update_conn_state(t, val, sent_bytes, recv_bytes);