g_network_monitor_base_add_network() improperly unrefs GInetAddressMask
@pwithnall, I'm a week late but found a problem with 4a488ced. Look here:
void
g_network_monitor_base_add_network (GNetworkMonitorBase *monitor,
GInetAddressMask *network)
{
if (!g_hash_table_add (monitor->priv->networks, network))
return;
g_object_ref (network); /* for the element now stored in the hash table */
/* ... */
}
If network is already in the hash table, the old value gets destroyed (unreffed) and replaced with the new value (which was not reffed). That means that if we return early here, we've accidentally lost a ref and maybe destroyed the caller's GInetAddressMask. We need to ref it before adding it to the hash table. Proposed fix:
void
g_network_monitor_base_add_network (GNetworkMonitorBase *monitor,
GInetAddressMask *network)
{
if (!g_hash_table_add (monitor->priv->networks, g_object_ref (network)))
return;
/* ... */
}
I noticed this because it introduced a crash when starting evolution
$ jhbuild run valgrind evolution
==86427== Memcheck, a memory error detector
==86427== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==86427== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==86427== Command: evolution
==86427==
--86427-- WARNING: unhandled amd64-linux syscall: 315
--86427-- You may be able to write your own handler.
--86427-- Read the file README_MISSING_SYSCALL_OR_IOCTL.
--86427-- Nevertheless we consider this a bug. Please report
--86427-- it at http://valgrind.org/support/bug_reports.html.
==86427== Invalid read of size 8
==86427== at 0x63AA491: g_type_check_instance_is_fundamentally_a (gtype.c:4026)
==86427== by 0x638FB26: g_object_unref (gobject.c:3396)
==86427== by 0x4E159EB: ptr_array_free (garray.c:1440)
==86427== by 0x4E1597E: g_ptr_array_free (garray.c:1417)
==86427== by 0x62574ED: finish_dump (gnetworkmonitornetlink.c:292)
==86427== by 0x6257909: read_netlink_messages (gnetworkmonitornetlink.c:407)
==86427== by 0x6256F07: g_network_monitor_netlink_initable_init (gnetworkmonitornetlink.c:145)
==86427== by 0x6258561: g_network_monitor_nm_initable_init (gnetworkmonitornm.c:329)
==86427== by 0x61DC584: g_initable_init (ginitable.c:128)
==86427== by 0x61DC7A4: g_initable_new_valist (ginitable.c:248)
==86427== by 0x61DC63A: g_initable_new (ginitable.c:162)
==86427== by 0x61E08AD: try_implementation (giomodule.c:858)
==86427== Address 0x2024b580 is 32 bytes inside a block of size 72 free'd
==86427== at 0x483AA0C: free (vg_replace_malloc.c:540)
==86427== by 0x4E583CD: g_free (gmem.c:195)
==86427== by 0x4E737E2: g_slice_free1 (gslice.c:1135)
==86427== by 0x63A6802: g_type_free_instance (gtype.c:1939)
==86427== by 0x638FE67: g_object_unref (gobject.c:3516)
==86427== by 0x4E3B01A: g_hash_table_insert_node (ghash.c:1343)
==86427== by 0x4E3B555: g_hash_table_insert_internal (ghash.c:1591)
==86427== by 0x4E3B5E2: g_hash_table_add (ghash.c:1676)
==86427== by 0x61F3843: g_network_monitor_base_add_network (gnetworkmonitorbase.c:499)
==86427== by 0x61F39C7: g_network_monitor_base_set_networks (gnetworkmonitorbase.c:584)
==86427== by 0x62574D4: finish_dump (gnetworkmonitornetlink.c:289)
==86427== by 0x6257909: read_netlink_messages (gnetworkmonitornetlink.c:407)
==86427== Block was alloc'd at
==86427== at 0x483980B: malloc (vg_replace_malloc.c:309)
==86427== by 0x4E5828C: g_malloc (gmem.c:102)
==86427== by 0x4E735A7: g_slice_alloc (gslice.c:1024)
==86427== by 0x4E735E6: g_slice_alloc0 (gslice.c:1050)
==86427== by 0x63A63AD: g_type_create_instance (gtype.c:1839)
==86427== by 0x638C152: g_object_new_internal (gobject.c:1937)
==86427== by 0x638CFEF: g_object_new_valist (gobject.c:2262)
==86427== by 0x61DC773: g_initable_new_valist (ginitable.c:244)
==86427== by 0x61DC63A: g_initable_new (ginitable.c:162)
==86427== by 0x61DA9E6: g_inet_address_mask_new (ginetaddressmask.c:260)
==86427== by 0x6257281: create_inet_address_mask (gnetworkmonitornetlink.c:235)
==86427== by 0x62572C3: add_network (gnetworkmonitornetlink.c:247)
==86427==
==86427== Invalid read of size 8
==86427== at 0x63AA4A4: g_type_check_instance_is_fundamentally_a (gtype.c:4028)
==86427== by 0x638FB26: g_object_unref (gobject.c:3396)
==86427== by 0x4E159EB: ptr_array_free (garray.c:1440)
==86427== by 0x4E1597E: g_ptr_array_free (garray.c:1417)
==86427== by 0x62574ED: finish_dump (gnetworkmonitornetlink.c:292)
==86427== by 0x6257909: read_netlink_messages (gnetworkmonitornetlink.c:407)
==86427== by 0x6256F07: g_network_monitor_netlink_initable_init (gnetworkmonitornetlink.c:145)
==86427== by 0x6258561: g_network_monitor_nm_initable_init (gnetworkmonitornm.c:329)
==86427== by 0x61DC584: g_initable_init (ginitable.c:128)
==86427== by 0x61DC7A4: g_initable_new_valist (ginitable.c:248)
==86427== by 0x61DC63A: g_initable_new (ginitable.c:162)
==86427== by 0x61E08AD: try_implementation (giomodule.c:858)
==86427== Address 0x2024b580 is 32 bytes inside a block of size 72 free'd
==86427== at 0x483AA0C: free (vg_replace_malloc.c:540)
==86427== by 0x4E583CD: g_free (gmem.c:195)
==86427== by 0x4E737E2: g_slice_free1 (gslice.c:1135)
==86427== by 0x63A6802: g_type_free_instance (gtype.c:1939)
==86427== by 0x638FE67: g_object_unref (gobject.c:3516)
==86427== by 0x4E3B01A: g_hash_table_insert_node (ghash.c:1343)
==86427== by 0x4E3B555: g_hash_table_insert_internal (ghash.c:1591)
==86427== by 0x4E3B5E2: g_hash_table_add (ghash.c:1676)
==86427== by 0x61F3843: g_network_monitor_base_add_network (gnetworkmonitorbase.c:499)
==86427== by 0x61F39C7: g_network_monitor_base_set_networks (gnetworkmonitorbase.c:584)
==86427== by 0x62574D4: finish_dump (gnetworkmonitornetlink.c:289)
==86427== by 0x6257909: read_netlink_messages (gnetworkmonitornetlink.c:407)
==86427== Block was alloc'd at
==86427== at 0x483980B: malloc (vg_replace_malloc.c:309)
==86427== by 0x4E5828C: g_malloc (gmem.c:102)
==86427== by 0x4E735A7: g_slice_alloc (gslice.c:1024)
==86427== by 0x4E735E6: g_slice_alloc0 (gslice.c:1050)
==86427== by 0x63A63AD: g_type_create_instance (gtype.c:1839)
==86427== by 0x638C152: g_object_new_internal (gobject.c:1937)
==86427== by 0x638CFEF: g_object_new_valist (gobject.c:2262)
==86427== by 0x61DC773: g_initable_new_valist (ginitable.c:244)
==86427== by 0x61DC63A: g_initable_new (ginitable.c:162)
==86427== by 0x61DA9E6: g_inet_address_mask_new (ginetaddressmask.c:260)
==86427== by 0x6257281: create_inet_address_mask (gnetworkmonitornetlink.c:235)
==86427== by 0x62572C3: add_network (gnetworkmonitornetlink.c:247)
==86427==
==86427== Invalid read of size 8
==86427== at 0x63AA4A7: g_type_check_instance_is_fundamentally_a (gtype.c:4028)
==86427== by 0x638FB26: g_object_unref (gobject.c:3396)
==86427== by 0x4E159EB: ptr_array_free (garray.c:1440)
==86427== by 0x4E1597E: g_ptr_array_free (garray.c:1417)
==86427== by 0x62574ED: finish_dump (gnetworkmonitornetlink.c:292)
==86427== by 0x6257909: read_netlink_messages (gnetworkmonitornetlink.c:407)
==86427== by 0x6256F07: g_network_monitor_netlink_initable_init (gnetworkmonitornetlink.c:145)
==86427== by 0x6258561: g_network_monitor_nm_initable_init (gnetworkmonitornm.c:329)
==86427== by 0x61DC584: g_initable_init (ginitable.c:128)
==86427== by 0x61DC7A4: g_initable_new_valist (ginitable.c:248)
==86427== by 0x61DC63A: g_initable_new (ginitable.c:162)
==86427== by 0x61E08AD: try_implementation (giomodule.c:858)
==86427== Address 0xaaaaaaaaaaaaaaaa is not stack'd, malloc'd or (recently) free'd
==86427==
==86427==
==86427== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==86427== General Protection Fault
==86427== at 0x63AA4A7: g_type_check_instance_is_fundamentally_a (gtype.c:4028)
==86427== by 0x638FB26: g_object_unref (gobject.c:3396)
==86427== by 0x4E159EB: ptr_array_free (garray.c:1440)
==86427== by 0x4E1597E: g_ptr_array_free (garray.c:1417)
==86427== by 0x62574ED: finish_dump (gnetworkmonitornetlink.c:292)
==86427== by 0x6257909: read_netlink_messages (gnetworkmonitornetlink.c:407)
==86427== by 0x6256F07: g_network_monitor_netlink_initable_init (gnetworkmonitornetlink.c:145)
==86427== by 0x6258561: g_network_monitor_nm_initable_init (gnetworkmonitornm.c:329)
==86427== by 0x61DC584: g_initable_init (ginitable.c:128)
==86427== by 0x61DC7A4: g_initable_new_valist (ginitable.c:248)
==86427== by 0x61DC63A: g_initable_new (ginitable.c:162)
==86427== by 0x61E08AD: try_implementation (giomodule.c:858)
==86427==
==86427== HEAP SUMMARY:
==86427== in use at exit: 2,020,795 bytes in 23,214 blocks
==86427== total heap usage: 200,210 allocs, 176,996 frees, 13,528,361 bytes allocated
==86427==
==86427== LEAK SUMMARY:
==86427== definitely lost: 3,528 bytes in 37 blocks
==86427== indirectly lost: 121 bytes in 2 blocks
==86427== possibly lost: 7,192 bytes in 92 blocks
==86427== still reachable: 1,843,514 bytes in 21,772 blocks
==86427== of which reachable via heuristic:
==86427== length64 : 10,632 bytes in 162 blocks
==86427== newarray : 2,240 bytes in 60 blocks
==86427== suppressed: 0 bytes in 0 blocks
==86427== Rerun with --leak-check=full to see details of leaked memory
==86427==
==86427== For lists of detected and suppressed errors, rerun with: -s
==86427== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)
This bug has only existed in git master for two weeks, so security impact should be minimal.
Edited by Michael Catanzaro