colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bond-slb: fix memory leak

Browse Source 
If sendto() fails, the function returns and the remaining entries are
not deallocated. Use nm_auto_freev instead to free the array and the
pointer it contains.

Add a test to check that nm_auto_freev does the right thing on the
value returned by nm_linux_platform_get_bridge_fdb().

Fixes: 3f2f922dd9 ('bonding: send ARP announcement on bonding-slb link/carrier down')
This commit is contained in:
Beniamino Galvani
2025-07-07 09:43:12 +02:00
parent 7d23ed9f73
commit 16ef33d380
2 changed files with 66 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
src/core/nm-bond-manager.c
View File

@@ -916,9 +916,9 @@ nm_bond_manager_send_arp(int bond_ifindex,
if (announce_fdb) {
/* if we are announcing the FDB we do a RARP, we don't set the
* source/dest IPv4 address */
int ifindexes[] = {bridge_ifindex, bond_ifindex};
int i;
gs_free NMEtherAddr **fdb_addrs = NULL;
int ifindexes[] = {bridge_ifindex, bond_ifindex};
int i;
nm_auto_freev NMEtherAddr **fdb_addrs = NULL;
fdb_addrs = nm_linux_platform_get_bridge_fdb(platform, ifindexes, 2);
/* we want to send a Reverse ARP (RARP) packet */
@@ -927,10 +927,10 @@ nm_bond_manager_send_arp(int bond_ifindex,
i = 0;
while (fdb_addrs[i] != NULL) {
NMEtherAddr *tmp_hwaddr = fdb_addrs[i];
memcpy(data.s_hw_addr, tmp_hwaddr, ETH_ALEN);
memcpy(data.d_hw_addr, tmp_hwaddr, ETH_ALEN);
memcpy(data.s_addr, tmp_hwaddr, ETH_ALEN);
g_free(tmp_hwaddr);
if (sendto(sockfd, &data, sizeof(data), 0, (struct sockaddr *) &addr, sizeof(addr)) < 0)
return FALSE;
i++;

62
src/core/platform/tests/test-link.c
View File

@@ -2725,6 +2725,66 @@ test_link_set_properties(void)
/*****************************************************************************/
static void
test_link_get_bridge_fdb(void)
{
const NMPlatformLink *link;
nm_auto_freev NMEtherAddr **addrs;
int ifindex[2];
guint8 expected[][6] = {
{0x00, 0x99, 0x00, 0x00, 0x00, 0x01},
{0x00, 0x99, 0x00, 0x00, 0x00, 0x02},
{0x00, 0x99, 0x00, 0x00, 0x00, 0x03},
{0x00, 0x99, 0x00, 0x00, 0x00, 0x05},
};
guint i;
guint j;
ifindex[0] =
nmtstp_link_bridge_add(NULL, -1, "br-test-1", &nm_platform_lnk_bridge_default)->ifindex;
ifindex[1] =
nmtstp_link_bridge_add(NULL, -1, "br-test-2", &nm_platform_lnk_bridge_default)->ifindex;
link = nmtstp_link_get(NULL, ifindex[0], "br-test-1");
g_assert(link);
link = nmtstp_link_get(NULL, ifindex[1], "br-test-2");
g_assert(link);
nmtstp_run_command_check("bridge fdb add dev br-test-1 00:99:00:00:00:01");
nmtstp_run_command_check("bridge fdb add dev br-test-1 00:99:00:00:00:02");
nmtstp_run_command_check("bridge fdb add dev br-test-1 00:99:00:00:00:03");
nmtstp_run_command_check("bridge fdb add dev br-test-2 00:99:00:00:00:01");
nmtstp_run_command_check("bridge fdb add dev br-test-2 00:99:00:00:00:05");
addrs = nm_linux_platform_get_bridge_fdb(NM_PLATFORM_GET, ifindex, 2);
g_assert(addrs);
/* Check for expected entries */
for (i = 0; i < G_N_ELEMENTS(expected); i++) {
gboolean found = FALSE;
for (j = 0; addrs[j]; j++) {
if (memcmp(addrs[j], expected[i], ETH_ALEN) == 0) {
found = TRUE;
break;
}
}
g_assert(found);
}
/* No dupes */
for (i = 0; addrs[i]; i++) {
for (j = i + 1; addrs[j]; j++) {
g_assert_cmpint(memcmp(addrs[i], addrs[j], ETH_ALEN), !=, 0);
}
}
nmtstp_link_delete(NULL, -1, ifindex[0], "br-test-1", TRUE);
nmtstp_link_delete(NULL, -1, ifindex[1], "br-test-2", TRUE);
}
/*****************************************************************************/
static void
test_create_many_links_do(guint n_devices)
{
@@ -4106,6 +4166,8 @@ _nmtstp_setup_tests(void)
test_software_detect_add("/link/software/detect/wireguard/1", NM_LINK_TYPE_WIREGUARD, 1);
test_software_detect_add("/link/software/detect/wireguard/2", NM_LINK_TYPE_WIREGUARD, 2);
g_test_add_func("/link/get-bridge-fdb", test_link_get_bridge_fdb);
g_test_add_func("/link/software/vlan/set-xgress", test_vlan_set_xgress);
g_test_add_func("/link/set-properties", test_link_set_properties);