[ upstream commit 74530b1880ec080959eb95aa71d6ce1d7101570b ]

Commit c96b9d81a8a6 ("ipcache: Remove superfluous if condition")
introduced a double-free for cases where the ipcache allocates a new
(reference to a) security identity and there is already an identity
corresponding to those labels which another subsystem injected directly
into the ipcache via Upsert() and friends. This commit fixes the issue.

IPCache has two sets of APIs:
- Older APIs like Upsert() / UpsertGeneratedIdentities which require the
  caller to retain reference counts against each Identity and balance
  allocate/release calls.
- Newer APIs like UpsertMetadata() / UpsertLabels() / UpsertPrefixes()
  which delegate the reference counting responsibility to the ipcache
  itself, which will hold one reference to the identity for all callers
  of the APIs.

InjectLabels() is responsible for reference counting the identities
associated with ipcache entries that it manages. In order to ensure
balanced allocate and release, this function allocates and retains one
reference to the identity for itself internally within this function.
Under normal operation it will typically release this reference again
once it has performed the ipcache update operations. It may be triggered
multiple times for the same prefix, in which case it should only ever
acquire one reference to the identity, otherwise the references could
leak and the identity would not be freed.

There is one exception to the "always allocate" + "always" release
behaviour in InjectLabels(): Given that the ipcache needs to hold
exactly one reference to the identity over time to ensure it remains
live while in use by the ipcache, the very first time that
InjectLabels() allocates the identity reference, it must hold onto the
reference and not immediately release it. In particular, if another
subsystem injected an ipcache entry via older APIs, then the first time
that InjectLabels() runs, it also needs to acquire and hold a reference
to the identity, even though the underlying ipcache already contains an
CIDR -> Identity mapping that was injected via the older Upsert() API.

In commit c96b9d81a8a6 ("ipcache: Remove superfluous if condition"), the
check which was ensuring that ipcache entries that transition from being
managed by older APIs towards newer APIs would not trigger allocation of
a new identity reference to be held by the ipcache itself. The result
would be that the resting number of references to the identity would be
one less than what is necessary to keep the identity alive. If the other
subsystem ever released its reference to the identity, then that would
be the last reference and the identity would be freed, despite still
being in use.

This scenario 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. Given this
mix of API usage and the bug introduced in the identity reference
counting internally in ipcache, this triggers the bug.

This commit fixes it by checking whether InjectLabels() already assumed
ownership over the underlying ipcache entry, which implies that the
ipcache already holds a reference to the identity on behalf of all newer
API users. During subsequent calls to InjectLabels(), the logic checks
that the ipcache entry was installed by this function and accounts for
the fact that a prior iteration had allocated an identity reference. It
can then safely release references that were acquired during the current
execution of the function.

Fixes: c96b9d81a8a6 ("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>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>