net: ipv6: ioam6: fix double reallocation

If the dst_entry is the same post transformation (which is a valid use
case for IOAM), we don't add it to the cache to avoid a reference loop.
Instead, we use a "fake" dst_entry and add it to the cache as a signal.
When we read the cache, we compare it with our "fake" dst_entry and
therefore detect if we're in the special case.

Signed-off-by: Justin Iurman <justin.iurman@uliege.be>
Link: https://patch.msgid.link/20250415112554.23823-3-justin.iurman@uliege.be
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
index 57200b9991a1dca637b6c6dde0600f6bfe397436..40df8bdfaacdde4b61db4f9f8fad52ac342918d5 100644 (file)
--- a/net/ipv6/ioam6_iptunnel.c
+++ b/net/ipv6/ioam6_iptunnel.c
@@ -38,6 +38,7 @@ struct ioam6_lwt_freq {
 };
 
 struct ioam6_lwt {
+       struct dst_entry null_dst;
        struct dst_cache cache;
        struct ioam6_lwt_freq freq;
        atomic_t pkt_cnt;
@@ -177,6 +178,14 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla,
        if (err)
                goto free_lwt;
 
+       /* This "fake" dst_entry will be stored in a dst_cache, which will call
+        * dst_hold() and dst_release() on it. We must ensure that dst_destroy()
+        * will never be called. For that, its initial refcount is 1 and +1 when
+        * it is stored in the cache. Then, +1/-1 each time we read the cache
+        * and release it. Long story short, we're fine.
+        */
+       dst_init(&ilwt->null_dst, NULL, NULL, DST_OBSOLETE_NONE, DST_NOCOUNT);
+
        atomic_set(&ilwt->pkt_cnt, 0);
        ilwt->freq.k = freq_k;
        ilwt->freq.n = freq_n;
@@ -356,6 +365,17 @@ static int ioam6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
        dst = dst_cache_get(&ilwt->cache);
        local_bh_enable();
 
+       /* This is how we notify that the destination does not change after
+        * transformation and that we need to use orig_dst instead of the cache
+        */
+       if (dst == &ilwt->null_dst) {
+               dst_release(dst);
+
+               dst = orig_dst;
+               /* keep refcount balance: dst_release() is called at the end */
+               dst_hold(dst);
+       }
+
        switch (ilwt->mode) {
        case IOAM6_IPTUNNEL_MODE_INLINE:
 do_inline:
@@ -408,12 +428,19 @@ do_encap:
                        goto drop;
                }
 
-               /* cache only if we don't create a dst reference loop */
-               if (orig_dst->lwtstate != dst->lwtstate) {
-                       local_bh_disable();
+               /* If the destination is the same after transformation (which is
+                * a valid use case for IOAM), then we don't want to add it to
+                * the cache in order to avoid a reference loop. Instead, we add
+                * our fake dst_entry to the cache as a way to detect this case.
+                * Otherwise, we add the resolved destination to the cache.
+                */
+               local_bh_disable();
+               if (orig_dst->lwtstate == dst->lwtstate)
+                       dst_cache_set_ip6(&ilwt->cache,
+                                         &ilwt->null_dst, &fl6.saddr);
+               else
                        dst_cache_set_ip6(&ilwt->cache, dst, &fl6.saddr);
-                       local_bh_enable();
-               }
+               local_bh_enable();
 
                err = skb_cow_head(skb, LL_RESERVED_SPACE(dst->dev));
                if (unlikely(err))
@@ -439,6 +466,10 @@ drop:
 
 static void ioam6_destroy_state(struct lwtunnel_state *lwt)
 {
+       /* Since the refcount of per-cpu dst_entry caches will never be 0 (see
+        * why above) when our "fake" dst_entry is used, it is not necessary to
+        * remove them before calling dst_cache_destroy()
+        */
        dst_cache_destroy(&ioam6_lwt_state(lwt)->cache);
 }