route-table: Fix potential memory leak.

As highlighted by static analysis [0], a potential memory leak
was introduced in the commit referenced in the fixes tag.

The issue was introduced by what I can describe as premature
optimization, and while very unlikely to hit, let's make the code
correct.

The logic for cleanup assumes rdnh will always be added to the
list of nexthops, and then I apparently chose to skip that when
processing the outer message with a RTA_MULTIPATH attribute,
presumably because its nexthop attributes will be added when
processing nested attributes, making the list addition
redundant.

Skipping the list addition was technically safe, because at
this point rdnh would be pointing at primary_next_hop__ on the
stack.

Separate out the nexthop cleanup code in private helper for
internal use, while this is the only action for the public
route_data_destroy() today, it might grow other powers in the
future.

Always add rdnh to list of nexthops and remove it when processing
RTA_MULTIPATH nested attributes.

0: https://mail.openvswitch.org/pipermail/ovs-dev/2025-January/419818.html
Fixes: 91fc511 ("route-table: Support parsing multipath routes.")
Acked-by: Eelco Chaudron <[email protected]>
Signed-off-by: Frode Nordahl <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>

lib/route-table.c

@@ -71,8 +71,8 @@ static void route_map_clear(void);
 static void name_table_init(void);
 static void name_table_change(const struct rtnetlink_change *, void *);
 
-void
-route_data_destroy(struct route_data *rd)
+static void
+route_data_destroy_nexthops__(struct route_data *rd)
 {
     struct route_data_nexthop *rdnh;
 
@@ -83,6 +83,12 @@ route_data_destroy(struct route_data *rd)
     }
 }
 
+void
+route_data_destroy(struct route_data *rd)
+{
+    route_data_destroy_nexthops__(rd);
+}
+
 uint64_t
 route_table_get_change_seq(void)
 {
@@ -275,12 +281,9 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs,
 
         ovs_list_init(&change->rd.nexthops);
         rdnh = rtnh ? xzalloc(sizeof *rdnh) : &change->rd.primary_next_hop__;
+        ovs_list_insert(&change->rd.nexthops, &rdnh->nexthop_node);
 
-        if (!attrs[RTA_MULTIPATH]) {
-            rdnh->family = rtm->rtm_family;
-            ovs_list_insert(&change->rd.nexthops, &rdnh->nexthop_node);
-        }
-
+        rdnh->family = rtm->rtm_family;
         change->relevant = true;
 
         if (rtm->rtm_scope == RT_SCOPE_NOWHERE) {
@@ -408,6 +411,20 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs,
                 goto error_out;
             }
 
+            /* The change->rd->nexthops list is unconditionally populated with
+             * a single rdnh entry as we start parsing above.  Multiple
+             * branches above may access it or jump to error_out, and having it
+             * on the list is the only way to ensure proper cleanup.
+             *
+             * Getting to this point, we know that the above branches has not
+             * provided next hop information, because information about
+             * multiple next hops is encoded in the nested attributes after the
+             * RTA_MULTIPATH attribute.
+             *
+             * Before retrieving those we need to remove the empty rdnh entry
+             * from the list. */
+            route_data_destroy_nexthops__(&change->rd);
+
             NL_NESTED_FOR_EACH (nla, left, attrs[RTA_MULTIPATH]) {
                 struct route_table_msg mp_change;
                 struct rtnexthop *mp_rtnh;