From ca39902cee649b55fa0b7e01d2f85477d7d77448 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Wed, 2 Apr 2025 10:04:36 +0200 Subject: [PATCH 1/3] bond-slb: initialize dest hw address in GARP packets Detected by Coverity: 1. NetworkManager-1.53.1/src/core/nm-bond-manager.c:885:5: var_decl: Declaring variable "data" without initializer. 7. NetworkManager-1.53.1/src/core/nm-bond-manager.c:948:13: uninit_use_in_call: Using uninitialized value "data". Field "data.d_hw_addr" is uninitialized when calling "sendto". 946| unaligned_write_ne32(data.s_ip_addr, tmp_addr); 947| unaligned_write_ne32(data.d_ip_addr, tmp_addr); 948|-> if (sendto(sockfd, &data, sizeof(data), 0, (struct sockaddr *) &addr, sizeof(addr)) < 0) 949| return FALSE; 950| } Fixes: 3f2f922dd943 ('bonding: send ARP announcement on bonding-slb link/carrier down') (cherry picked from commit 42edb37499fdf848ec751b059a361fd37eab69d5) --- src/core/nm-bond-manager.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/nm-bond-manager.c b/src/core/nm-bond-manager.c index 2f7fe36c16..116cb73744 100644 --- a/src/core/nm-bond-manager.c +++ b/src/core/nm-bond-manager.c @@ -882,7 +882,7 @@ nm_bond_manager_send_arp(int bond_ifindex, .sll_protocol = htons(ETH_P_ARP), .sll_ifindex = bond_ifindex, }; - ARPPacket data; + ARPPacket data = {0}; const guint8 *hwaddr; gsize hwaddrlen = 0; nm_auto_close int sockfd = -1; @@ -940,6 +940,7 @@ nm_bond_manager_send_arp(int bond_ifindex, data.op = htons(ARP_OP_GARP); memcpy(data.s_addr, hwaddr, hwaddrlen); memcpy(data.s_hw_addr, hwaddr, hwaddrlen); + memset(data.d_hw_addr, 0xff, ETH_ALEN); for (int i = 0; i < addrs_len; i++) { const in_addr_t tmp_addr = addrs_array[i]; From d19068c9e349f8a219fad712c8af38bfda670f3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Wed, 2 Apr 2025 10:16:15 +0200 Subject: [PATCH 2/3] core: fix use after free in ping operations Detected by coverity, the ping_op pointers are used after being freed in cleanup_ping_operations. Although calling to g_list_remove is probably safe because it only needs the value of the pointer, not to dereference it, better to follow best practices. One of the use after free was actually an error because we dereference ping_op->log_domain. Fixes: 658aef0fa185 ('connection: Support connection.ip-ping-addresses') (cherry picked from commit ae7de5b353b8bdbfefd6a67c8fe53678cf78a60a) --- src/core/devices/nm-device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 5aabc4cb90..4c1cc4160b 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -15088,8 +15088,8 @@ respawn_ping_cb(gpointer user_data) nm_clear_g_source_inst(&ping_op->watch); if (!spawn_ping_for_operation(self, ping_op)) { - cleanup_ping_operation(ping_op); priv->ping_operations = g_list_remove(priv->ping_operations, ping_op); + cleanup_ping_operation(ping_op); if (g_list_length(priv->ping_operations) == 0) { ip_check_pre_up(self); @@ -15132,7 +15132,6 @@ ip_check_ping_watch_cb(GPid pid, int status, gpointer user_data) if (success) { if (ping_op->ping_addresses_require_all) { - cleanup_ping_operation(ping_op); priv->ping_operations = g_list_remove(priv->ping_operations, ping_op); if (g_list_length(priv->ping_operations) == 0) { _LOGD(ping_op->log_domain, @@ -15142,6 +15141,7 @@ ip_check_ping_watch_cb(GPid pid, int status, gpointer user_data) nm_clear_g_source_inst(&priv->ping_timeout); ip_check_pre_up(self); } + cleanup_ping_operation(ping_op); } else { nm_assert(priv->ping_operations); From a1e1dd29787d75c3603190687dbb71a90a0eda4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Wed, 2 Apr 2025 10:31:30 +0200 Subject: [PATCH 3/3] n-dhcp4: fix resource leaks The function n_dhcp4_c_connection_send_request does not release or take ownership of its request argument. Because of that, setting it to NULL in the caller prevents the auto-cleanup of the variable to be executed, causing a resource leak. Fix it. Fixes: e23b3c9c3ac8 ('Squashed 'shared/n-dhcp4/' content from commit fb1d43449') Fixes: 243cc433fb77 ('n-dhcp4: add new client probe function to send RELEASE message') (cherry picked from commit 9edfc0438cb6391b01999022567e6ed5aa6bb8db) --- src/n-dhcp4/src/n-dhcp4-c-probe.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/n-dhcp4/src/n-dhcp4-c-probe.c b/src/n-dhcp4/src/n-dhcp4-c-probe.c index 7a6def340c..58d61e72ba 100644 --- a/src/n-dhcp4/src/n-dhcp4-c-probe.c +++ b/src/n-dhcp4/src/n-dhcp4-c-probe.c @@ -1140,8 +1140,6 @@ int n_dhcp4_client_probe_transition_decline(NDhcp4ClientProbe *probe, NDhcp4Inco r = n_dhcp4_c_connection_send_request(&probe->connection, request, ns_now); if (r) return r; - else - request = NULL; /* consumed */ n_dhcp4_client_lease_unlink(probe->current_lease); probe->current_lease = n_dhcp4_client_lease_unref(probe->current_lease); @@ -1346,7 +1344,6 @@ int n_dhcp4_client_probe_release(NDhcp4ClientProbe *probe) { probe->state = N_DHCP4_CLIENT_PROBE_STATE_INIT; n_dhcp4_client_lease_unlink(probe->current_lease); - request_out = NULL; return 0; }