ipcache: Fix refcounting with mix of APIs

Commit c96b9d8 ("ipcache: Remove superfluous if condition")
triggers a double-free for cases where there is a mix of users for
older and newer internal ipcache APIs. In this scenario, the older
ipcache APIs are used to inject entries into the ipcache, then
InjectLabels() attempts to allocate a new security identity reference
for the same CIDR and assumes that it already holds a reference to the
corresponding identity. It then releases the reference it just acquired.
If the other module ever releases its reference, then that results in
freeing of the identity regardless of its continued expected usage by
users of the newer ipcache APIs. This leads to policy recalculation that
removes any datapath allow rules for the corresponding CIDRs, ultimately
resulting in packet loss for the impacted CIDRs.

One such example involves CIDR identity restore startup logic in the
daemon. That path allocates identities then injects them into the
ipcache using older APIs. If any such CIDRs are used by network
policies, then the network policies subsystem will insert the CIDR into
the ipcache using newer ipcache APIs, which will then trigger this
double-free.

Fixes: c96b9d8 ("ipcache: Remove superfluous if condition")
Reported-by: Boris Petrovic <carnerito.b@gmail.com>
Reported-by: Kim-Eirik Karlsen <kim.eirik@gmail.com>
Reported-by: Jason Witkowski <jason@witkow.ski>
Signed-off-by: Joe Stringer <joe@cilium.io>

pkg/ipcache/metadata.go

@@ -22,6 +22,7 @@ import (
 	cidrlabels "github.com/cilium/cilium/pkg/labels/cidr"
 	"github.com/cilium/cilium/pkg/lock"
 	"github.com/cilium/cilium/pkg/logging/logfields"
+	"github.com/cilium/cilium/pkg/option"
 	"github.com/cilium/cilium/pkg/source"
 )
 
@@ -272,8 +273,28 @@ func (ipc *IPCache) InjectLabels(ctx context.Context, modifiedPrefixes []netip.P
 			// iteration of the loop, then we must balance the
 			// allocation from the prior InjectLabels() call by
 			// releasing the previous reference.
-			previouslyAllocatedIdentities[prefix] = oldID
-
+			entry, entryToBeReplaced := entriesToReplace[prefix]
+			if !oldID.createdFromMetadata && entryToBeReplaced {
+				// If the previous ipcache entry for the prefix
+				// was not managed by this function, then the
+				// previous ipcache user to inject the IPCache
+				// entry retains its own reference to the
+				// Security Identity. Given that this function
+				// is going to assume responsibility for the
+				// IPCache entry now, this path must retain its
+				// own reference to the Security Identity to
+				// ensure that if the other owner ever releases
+				// their reference, this reference stays live.
+				if option.Config.Debug {
+					log.WithFields(logrus.Fields{
+						logfields.Prefix:      prefix,
+						logfields.OldIdentity: oldID.ID,
+						logfields.Identity:    entry.identity.ID,
+					}).Debug("Acquiring Identity reference")
+				}
+			} else {
+				previouslyAllocatedIdentities[prefix] = oldID
+			}
 			// If all associated metadata for this prefix has been removed,
 			// and the existing IPCache entry was never touched by any other
 			// subsystem using the old Upsert API, then we can safely remove