From 3d10206eec0692e79647b6b20de5acf1919710a4 Mon Sep 17 00:00:00 2001 From: Daniel JB Clark Date: Mon, 17 Aug 2026 16:47:55 -0400 Subject: [PATCH] Fixed default route selection to pick the lowest-metric route GetNetworkingInfo() declared `long lowest_metric = 0;` and never assigned it again, so once any candidate had been chosen the guard `JsonPrimitiveGetAsInteger(metric) < lowest_metric` compared each later route's metric against 0 and could never be true for the non-negative metrics the kernel prints. The first active default route in /proc/net/route therefore always won, regardless of metric, ever since the loop was introduced (CFE-1991, first released in 3.9.0). The metric of the selected route is now recorded, so a later route with a strictly lower metric replaces the earlier choice; equal metrics keep the earlier entry, as before. On hosts with several active default routes where a lower-metric route appears after a higher-metric one, this changes the reported sys.inet.default_route and sys.inet.default_gateway to the lower-metric route. The selection loop moved into FindLowestMetricDefaultRoute() so that the new tests/unit/unix_iface_test.c can drive it with constructed route data; its lowest-metric-last case fails against the previous logic. Ticket: CFE-4723 Changelog: Title --- libenv/unix_iface.c | 74 +++++++++++++------- tests/unit/Makefile.am | 6 ++ tests/unit/unix_iface_test.c | 128 +++++++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 23 deletions(-) create mode 100644 tests/unit/unix_iface_test.c diff --git a/libenv/unix_iface.c b/libenv/unix_iface.c index fe7bcee417..a93cd78bdd 100644 --- a/libenv/unix_iface.c +++ b/libenv/unix_iface.c @@ -1392,6 +1392,56 @@ JsonElement* GetProcFileInfo(EvalContext *ctx, const char* filename, const char* /*******************************************************************/ +/** + * @brief Select the active default route with the lowest metric. + * + * NetworkingRoutesPostProcessInfo() marks a route with + * "active_default_gateway": true when its destination is 0.0.0.0 and its + * flags include RTF_UP and RTF_GATEWAY. Among those routes, the one with + * the numerically lowest metric is preferred; equal metrics keep the + * earliest entry. + * + * @param routes JSON array of routes parsed from /proc/net/route + * @return the preferred default route (owned by @p routes), or NULL if + * there is no active default route with a usable metric + */ +static const JsonElement *FindLowestMetricDefaultRoute(const JsonElement *routes) +{ + assert(routes != NULL); + + const JsonElement *default_route = NULL; + long lowest_metric = 0; + + JsonIterator iter = JsonIteratorInit(routes); + const JsonElement *route = NULL; + while ((route = JsonIteratorNextValue(&iter))) + { + JsonElement *active = JsonObjectGet(route, "active_default_gateway"); + if (active != NULL && + JsonGetElementType(active) == JSON_ELEMENT_TYPE_PRIMITIVE && + JsonGetPrimitiveType(active) == JSON_PRIMITIVE_TYPE_BOOL && + JsonPrimitiveGetAsBool(active)) + { + JsonElement *metric = JsonObjectGet(route, "metric"); + if (metric != NULL && + JsonGetElementType(metric) == JSON_ELEMENT_TYPE_PRIMITIVE && + JsonGetPrimitiveType(metric) == JSON_PRIMITIVE_TYPE_INTEGER) + { + long metric_value = JsonPrimitiveGetAsInteger(metric); + if (default_route == NULL || metric_value < lowest_metric) + { + default_route = route; + lowest_metric = metric_value; + } + } + } + } + + return default_route; +} + +/*******************************************************************/ + void GetNetworkingInfo(EvalContext *ctx) { const char *procdir_root = GetRelocatedProcdirRoot(); @@ -1420,29 +1470,7 @@ void GetNetworkingInfo(EvalContext *ctx) { JsonObjectAppendElement(inet, "routes", routes); - JsonIterator iter = JsonIteratorInit(routes); - const JsonElement *default_route = NULL; - long lowest_metric = 0; - const JsonElement *route = NULL; - while ((route = JsonIteratorNextValue(&iter))) - { - JsonElement *active = JsonObjectGet(route, "active_default_gateway"); - if (active != NULL && - JsonGetElementType(active) == JSON_ELEMENT_TYPE_PRIMITIVE && - JsonGetPrimitiveType(active) == JSON_PRIMITIVE_TYPE_BOOL && - JsonPrimitiveGetAsBool(active)) - { - JsonElement *metric = JsonObjectGet(route, "metric"); - if (metric != NULL && - JsonGetElementType(metric) == JSON_ELEMENT_TYPE_PRIMITIVE && - JsonGetPrimitiveType(metric) == JSON_PRIMITIVE_TYPE_INTEGER && - (default_route == NULL || - JsonPrimitiveGetAsInteger(metric) < lowest_metric)) - { - default_route = route; - } - } - } + const JsonElement *default_route = FindLowestMetricDefaultRoute(routes); if (default_route != NULL) { diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index 3864b67876..839078b6ea 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -420,6 +420,12 @@ check_PROGRAMS += nfs_test nfs_test_SOURCES = nfs_test.c nfs_test_LDADD = ../../libpromises/libpromises.la libtest.la +check_PROGRAMS += unix_iface_test +unix_iface_test_SOURCES = unix_iface_test.c +unix_iface_test_LDADD = libtest.la \ + ../../libenv/libenv.la \ + ../../libpromises/libpromises.la + init_script_test_helper_SOURCES = init_script_test_helper.c init_script_test.sh: init_script_test_helper CLEANFILES += init_script_test_helper diff --git a/tests/unit/unix_iface_test.c b/tests/unit/unix_iface_test.c new file mode 100644 index 0000000000..75b96945aa --- /dev/null +++ b/tests/unit/unix_iface_test.c @@ -0,0 +1,128 @@ +#include + +#include + +/* + * FindLowestMetricDefaultRoute() operates on the JSON produced by parsing + * /proc/net/route, after NetworkingRoutesPostProcessInfo() has annotated + * each route with "active_default_gateway" and a numeric "metric". The + * routes here are constructed directly in that shape, so the selection + * logic can be exercised on any platform. + */ +static JsonElement *AppendRoute( + JsonElement *routes, const char *gateway, int metric, bool active) +{ + JsonElement *route = JsonObjectCreate(3); + JsonObjectAppendString(route, "gateway", gateway); + JsonObjectAppendInteger(route, "metric", metric); + JsonObjectAppendBool(route, "active_default_gateway", active); + JsonArrayAppendElement(routes, route); + return route; +} + +static void test_no_routes(void) +{ + JsonElement *routes = JsonArrayCreate(1); + + assert_true(FindLowestMetricDefaultRoute(routes) == NULL); + + JsonDestroy(routes); +} + +static void test_no_active_default_route(void) +{ + JsonElement *routes = JsonArrayCreate(2); + AppendRoute(routes, "192.168.0.1", 0, false); + AppendRoute(routes, "192.168.0.2", 100, false); + + assert_true(FindLowestMetricDefaultRoute(routes) == NULL); + + JsonDestroy(routes); +} + +static void test_single_active_default_route(void) +{ + JsonElement *routes = JsonArrayCreate(2); + AppendRoute(routes, "192.168.0.1", 0, false); + JsonElement *expected = AppendRoute(routes, "192.168.0.2", 1024, true); + + assert_true(FindLowestMetricDefaultRoute(routes) == expected); + + JsonDestroy(routes); +} + +static void test_lowest_metric_first(void) +{ + JsonElement *routes = JsonArrayCreate(2); + JsonElement *expected = AppendRoute(routes, "192.168.0.1", 100, true); + AppendRoute(routes, "192.168.0.2", 600, true); + + assert_true(FindLowestMetricDefaultRoute(routes) == expected); + + JsonDestroy(routes); +} + +static void test_lowest_metric_last(void) +{ + /* The lowest metric must win even when it appears after another + * active default route; this is the case CFE-4723 got wrong. */ + JsonElement *routes = JsonArrayCreate(3); + AppendRoute(routes, "192.168.0.1", 600, true); + AppendRoute(routes, "192.168.0.2", 100, true); + JsonElement *expected = AppendRoute(routes, "192.168.0.3", 50, true); + + const JsonElement *found = FindLowestMetricDefaultRoute(routes); + assert_true(found != NULL); + assert_string_equal(JsonObjectGetAsString(found, "gateway"), "192.168.0.3"); + assert_true(found == expected); + + JsonDestroy(routes); +} + +static void test_equal_metrics_keep_first(void) +{ + JsonElement *routes = JsonArrayCreate(2); + JsonElement *expected = AppendRoute(routes, "192.168.0.1", 100, true); + AppendRoute(routes, "192.168.0.2", 100, true); + + assert_true(FindLowestMetricDefaultRoute(routes) == expected); + + JsonDestroy(routes); +} + +static void test_inactive_and_incomplete_routes_are_skipped(void) +{ + JsonElement *routes = JsonArrayCreate(3); + + /* Lower metric, but not an active default gateway. */ + AppendRoute(routes, "192.168.0.1", 1, false); + + /* Active, but without a usable metric. */ + JsonElement *no_metric = JsonObjectCreate(2); + JsonObjectAppendString(no_metric, "gateway", "192.168.0.2"); + JsonObjectAppendBool(no_metric, "active_default_gateway", true); + JsonArrayAppendElement(routes, no_metric); + + JsonElement *expected = AppendRoute(routes, "192.168.0.3", 600, true); + + assert_true(FindLowestMetricDefaultRoute(routes) == expected); + + JsonDestroy(routes); +} + +int main() +{ + PRINT_TEST_BANNER(); + const UnitTest tests[] = + { + unit_test(test_no_routes), + unit_test(test_no_active_default_route), + unit_test(test_single_active_default_route), + unit_test(test_lowest_metric_first), + unit_test(test_lowest_metric_last), + unit_test(test_equal_metrics_keep_first), + unit_test(test_inactive_and_incomplete_routes_are_skipped), + }; + + return run_tests(tests); +}