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: 91fc51106cfe ("route-table: Support parsing multipath routes.") Signed-off-by: Frode Nordahl <[email protected]> --- lib/route-table.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/route-table.c b/lib/route-table.c index b7964522f..bec628405 100644 --- a/lib/route-table.c +++ b/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,8 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs, goto error_out; } + 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; -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
