From c205cb9d91e18b2d4b38a284dfbcc19e751d02ee Mon Sep 17 00:00:00 2001
From: Jarno Rajahalme <jarno@isovalent.com>
Date: Fri, 29 Nov 2024 18:17:14 +0100
Subject: [PATCH] socket_option: Change policy_resolver_ to a weak pointer

A shared_ptr policy_resolver_ holds on to the passed BpfMetadata::Config,
which owns the policy map reference. This could keep the the policy map
alive when the listeners are destructed, leading to the destruction of
the policy map from a worker thread. Change policy_resolver_ to a weak
pointer to resolve this.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
---
 cilium/BUILD           |  2 ++
 cilium/bpf_metadata.cc |  2 +-
 cilium/host_map.h      |  9 +--------
 cilium/socket_option.h | 15 +++++++++++----
 4 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/cilium/BUILD b/cilium/BUILD
index 66ee680a0..04346397b 100644
--- a/cilium/BUILD
+++ b/cilium/BUILD
@@ -66,6 +66,7 @@ envoy_cc_library(
     name = "socket_option_lib",
     hdrs = [
         "socket_option.h",
+        "policy_id.h",
     ],
     repository = "@envoy",
     deps = [
@@ -300,6 +301,7 @@ envoy_cc_library(
     hdrs = [
         "bpf_metadata.h",
         "host_map.h",
+        "policy_id.h",
     ],
     repository = "@envoy",
     deps = [
diff --git a/cilium/bpf_metadata.cc b/cilium/bpf_metadata.cc
index 630ba8681..7659e9dab 100644
--- a/cilium/bpf_metadata.cc
+++ b/cilium/bpf_metadata.cc
@@ -445,7 +445,7 @@ Cilium::SocketOptionSharedPtr Config::getMetadata(Network::ConnectionSocket& soc
   return std::make_shared<Cilium::SocketOption>(
       mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_, dip->port(),
       std::move(pod_ip), std::move(src_address), std::move(source_addresses.ipv4_),
-      std::move(source_addresses.ipv6_), shared_from_this(), proxy_id_, sni);
+      std::move(source_addresses.ipv6_), weak_from_this(), proxy_id_, sni);
 }
 
 Network::FilterStatus Instance::onAccept(Network::ListenerFilterCallbacks& cb) {
diff --git a/cilium/host_map.h b/cilium/host_map.h
index 32bf0d910..32a7f7b08 100644
--- a/cilium/host_map.h
+++ b/cilium/host_map.h
@@ -17,6 +17,7 @@
 #include "absl/numeric/int128.h"
 #include "cilium/api/nphds.pb.h"
 #include "cilium/api/nphds.pb.validate.h"
+#include "cilium/policy_id.h"
 
 // std::hash specialization for Abseil uint128, needed for unordered_map key.
 namespace std {
@@ -47,14 +48,6 @@ template <typename I> I masked(I addr, unsigned int plen) {
   return plen == 0 ? I(0) : addr & ~hton((I(1) << (PLEN_MAX - plen)) - 1);
 };
 
-enum ID : uint64_t {
-  UNKNOWN = 0,
-  WORLD = 2,
-  // LocalIdentityFlag is the bit in the numeric identity that identifies
-  // a numeric identity to have local scope
-  LocalIdentityFlag = 1 << 24,
-};
-
 class PolicyHostDecoder : public Envoy::Config::OpaqueResourceDecoder {
 public:
   PolicyHostDecoder() : validation_visitor_(ProtobufMessage::getNullValidationVisitor()) {}
diff --git a/cilium/socket_option.h b/cilium/socket_option.h
index e2c6ab788..020d5c31f 100644
--- a/cilium/socket_option.h
+++ b/cilium/socket_option.h
@@ -8,6 +8,7 @@
 #include "source/common/common/utility.h"
 
 #include "cilium/conntrack.h"
+#include "cilium/policy_id.h"
 #include "cilium/privileged_service_client.h"
 
 namespace Envoy {
@@ -227,7 +228,7 @@ class SocketOption : public SocketMarkOption {
                Network::Address::InstanceConstSharedPtr original_source_address,
                Network::Address::InstanceConstSharedPtr ipv4_source_address,
                Network::Address::InstanceConstSharedPtr ipv6_source_address,
-               const std::shared_ptr<PolicyResolver>& policy_resolver, uint32_t proxy_id,
+               const std::weak_ptr<PolicyResolver>& policy_resolver, uint32_t proxy_id,
                absl::string_view sni)
       : SocketMarkOption(mark, source_identity, original_source_address, ipv4_source_address,
                          ipv6_source_address),
@@ -246,11 +247,17 @@ class SocketOption : public SocketMarkOption {
   }
 
   uint32_t resolvePolicyId(const Network::Address::Ip* ip) const {
-    return policy_resolver_->resolvePolicyId(ip);
+    const auto resolver = policy_resolver_.lock();
+    if (resolver)
+      return resolver->resolvePolicyId(ip);
+    return Cilium::ID::WORLD; // default to WORLD policy ID if resolver is no longer available
   }
 
   const PolicyInstanceConstSharedPtr getPolicy() const {
-    return policy_resolver_->getPolicy(pod_ip_);
+    const auto resolver = policy_resolver_.lock();
+    if (resolver)
+      return resolver->getPolicy(pod_ip_);
+    return nullptr;
   }
 
   // policyUseUpstreamDestinationAddress returns 'true' if policy enforcement should be done on the
@@ -267,7 +274,7 @@ class SocketOption : public SocketMarkOption {
   std::string sni_;
 
 private:
-  const std::shared_ptr<PolicyResolver> policy_resolver_;
+  const std::weak_ptr<PolicyResolver> policy_resolver_;
 };
 
 using SocketOptionSharedPtr = std::shared_ptr<SocketOption>;