From 6493124403bfaccd0e7f6771f9bfe354a801e060 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 3 Mar 2026 10:29:31 +0100 Subject: [PATCH] core: fix again l3cd comparison When only one of the two default routes is set, a NULL pointer is dereferenced: Thread 1 "NetworkManager" received signal SIGSEGV, Segmentation fault. nm_l3_config_data_cmp_full (a=a@entry=0x4e2bde0, b=0x4e87330, flags=flags@entry=40) at ../src/core/nm-l3-config-data.c:2622 2622 NMPlatformIP4Route ra = def_route_a->ip4_route; (gdb) bt 0 nm_l3_config_data_cmp_full (a=a@entry=0x4e2bde0, b=0x4e87330, flags=flags@entry=40) at ../src/core/nm-l3-config-data.c:2622 1 in nm_dns_manager_set_ip_config (self=0x4db7520, addr_family=2, source_tag=0x4eb80a0, l3cd=0x4e2bde0, ip_config_type=NM_DNS_IP_CONFIG_TYPE_DEFAULT, replace_all=1) at ../src/core/dns/nm-dns-manager.c:2109 2 in nm_dns_manager_set_ip_config (self=0x4db7520, addr_family=addr_family@entry=0, source_tag=source_tag@entry=0x4eb80a0, l3cd=l3cd@entry=0x4e2bde0, ip_config_type=NM_DNS_IP_CONFIG_TYPE_DEFAULT, replace_all=replace_all@entry=1) at ../src/core/dns/nm-dns-manager.c:2059 3 in device_l3cd_changed (device=0x4eb80a0, l3cd_old=0x4e87330, l3cd_new=0x4e2bde0, user_data=0x4da4548) at ../src/core/nm-policy.c:2518 4 in ffi_call_unix64 () from target:/lib64/libffi.so.8 5 in ffi_call_int.lto_priv () from target:/lib64/libffi.so.8 6 in ffi_call () from target:/lib64/libffi.so.8 7 in g_cclosure_marshal_generic_va () from target:/lib64/libgobject-2.0.so.0 8 in signal_emit_valist_unlocked () from target:/lib64/libgobject-2.0.so.0 9 in g_signal_emit_valist () from target:/lib64/libgobject-2.0.so.0 10 in g_signal_emit () from target:/lib64/libgobject-2.0.so.0 11 in _dev_l3_cfg_notify_cb (l3cfg=0x4e2af20, notify_data=0x7ffd2176dfb0, self=0x4eb80a0) at ../src/core/devices/nm-device.c:4997 If flag NM_L3_CONFIG_CMP_FLAGS_ROUTES is set, let nmp_object_cmp_full() compare the two routes and deal with NULL values. If flag NM_L3_CONFIG_CMP_FLAGS_ROUTES_ID is set, check that both objects are not NULL before accessing them. The crash is only seen when the routes are equal (according to _dedup_multi_index_cmp(), which compares the network, metric and plen), and only one l3cd has a default route. Only routes in the main table qualify as default routes. Fixes: 0a02995175e0 ('core: fix l3cd comparison') https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1902 --- src/core/nm-l3-config-data.c | 18 ++++++------- src/core/tests/test-core.c | 49 ++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/core/nm-l3-config-data.c b/src/core/nm-l3-config-data.c index 0b2804fa72..13a84d51f0 100644 --- a/src/core/nm-l3-config-data.c +++ b/src/core/nm-l3-config-data.c @@ -2604,15 +2604,15 @@ nm_l3_config_data_cmp_full(const NML3ConfigData *a, const NMPObject *def_route_a = a->best_default_route_x[IS_IPv4]; const NMPObject *def_route_b = b->best_default_route_x[IS_IPv4]; - if (def_route_a != def_route_b) { - if (NM_FLAGS_HAS(flags, NM_L3_CONFIG_CMP_FLAGS_ROUTES)) { - NM_CMP_RETURN( - nmp_object_cmp_full(def_route_a, - def_route_b, - NM_FLAGS_HAS(flags, NM_L3_CONFIG_CMP_FLAGS_IFINDEX) - ? NMP_OBJECT_CMP_FLAGS_NONE - : NMP_OBJECT_CMP_FLAGS_IGNORE_IFINDEX)); - } else if (NM_FLAGS_HAS(flags, NM_L3_CONFIG_CMP_FLAGS_ROUTES_ID)) { + if (NM_FLAGS_HAS(flags, NM_L3_CONFIG_CMP_FLAGS_ROUTES)) { + NM_CMP_RETURN(nmp_object_cmp_full(def_route_a, + def_route_b, + NM_FLAGS_HAS(flags, NM_L3_CONFIG_CMP_FLAGS_IFINDEX) + ? NMP_OBJECT_CMP_FLAGS_NONE + : NMP_OBJECT_CMP_FLAGS_IGNORE_IFINDEX)); + } else if (NM_FLAGS_HAS(flags, NM_L3_CONFIG_CMP_FLAGS_ROUTES_ID)) { + NM_CMP_DIRECT(!!def_route_a, !!def_route_b); + if (def_route_a && def_route_b) { if (NM_FLAGS_HAS(flags, NM_L3_CONFIG_CMP_FLAGS_IFINDEX)) { NM_CMP_DIRECT(def_route_a->obj_with_ifindex.ifindex, def_route_b->obj_with_ifindex.ifindex); diff --git a/src/core/tests/test-core.c b/src/core/tests/test-core.c index cb09ac3862..51f4d4ab1c 100644 --- a/src/core/tests/test-core.c +++ b/src/core/tests/test-core.c @@ -20,6 +20,7 @@ #include "dns/nm-dns-manager.h" #include "nm-connectivity.h" #include "nm-firewall-utils.h" +#include "nm-l3-config-data.h" #include "nm-test-utils-core.h" @@ -2823,6 +2824,51 @@ test_icmp6_checksum(void) /*****************************************************************************/ +static void +test_l3_config_data_cmp_default_routes(void) +{ + nm_auto_unref_dedup_multi_index NMDedupMultiIndex *multi_idx = nm_dedup_multi_index_new(); + nm_auto_unref_l3cd_init NML3ConfigData *a = NULL; + nm_auto_unref_l3cd_init NML3ConfigData *b = NULL; + const int IFINDEX = 1; + + a = nm_l3_config_data_new(multi_idx, IFINDEX, NM_IP_CONFIG_SOURCE_USER); + nm_l3_config_data_add_route_4( + a, + NM_PLATFORM_IP4_ROUTE_INIT(.ifindex = IFINDEX, + .network = 0, + .plen = 0, + .gateway = nmtst_inet4_from_string("192.168.1.1"), + .metric = 100)); + + b = nm_l3_config_data_new(multi_idx, IFINDEX, NM_IP_CONFIG_SOURCE_USER); + nm_l3_config_data_add_route_4( + b, + NM_PLATFORM_IP4_ROUTE_INIT(.ifindex = IFINDEX, + .network = 0, + .plen = 0, + .gateway = nmtst_inet4_from_string("192.168.1.1"), + .metric = 100, + .table_coerced = nm_platform_route_table_coerce(100))); + + nm_l3_config_data_seal(a); + nm_l3_config_data_seal(b); + + g_assert(nm_l3_config_data_get_best_default_route(a, AF_INET)); + g_assert(!nm_l3_config_data_get_best_default_route(b, AF_INET)); + + g_assert_cmpint(nm_l3_config_data_cmp_full(a, b, NM_L3_CONFIG_CMP_FLAGS_ROUTES_ID), !=, 0); + g_assert_cmpint(nm_l3_config_data_cmp_full(b, a, NM_L3_CONFIG_CMP_FLAGS_ROUTES_ID), !=, 0); + + g_assert_cmpint(nm_l3_config_data_cmp_full(b, a, NM_L3_CONFIG_CMP_FLAGS_ADDRESSES), ==, 0); + g_assert_cmpint(nm_l3_config_data_cmp_full(b, a, NM_L3_CONFIG_CMP_FLAGS_ADDRESSES), ==, 0); + + g_assert_cmpint(nm_l3_config_data_cmp_full(a, a, NM_L3_CONFIG_CMP_FLAGS_ALL), ==, 0); + g_assert_cmpint(nm_l3_config_data_cmp_full(b, b, NM_L3_CONFIG_CMP_FLAGS_ALL), ==, 0); +} + +/*****************************************************************************/ + NMTST_DEFINE(); int @@ -2901,5 +2947,8 @@ main(int argc, char **argv) g_test_add_func("/core/general/test_icmp6_checksum", test_icmp6_checksum); + g_test_add_func("/core/general/test_l3_config_data_cmp_default_routes", + test_l3_config_data_cmp_default_routes); + return g_test_run(); }