udiskslinuxmanager:use dbus interface after free

[wxphaha]

In handle_get_block_devices, call get_block_objects to obtain iface_block_device of all current UDisksLinuxBlockObject, and then obtain the corresponding UDisksLinuxBlockObject's object_path through iface_block_device.iface_block_device is a GDBusInterfaceSkeleton, which saves the object through g_dbus_interface_skeleton_set_object. g_object_add_weak_pointer is used here. This function is not thread-safe.At this time, if other threads are releasing the object, the program will crash.
This scene can be reproduced by quickly plugging and unplugging the USB disk. The core is as follows (the redundant stack is omitted): When accessing object in thread 1, the object is released by thread 2 info threads
Id Target Id Frame

• 1 Thread 0x7f80979e70 (LWP 24559) 0x0000007f8a48dda0 in g_dbus_object_get_object_path (object=0x0) at ../../../gio/gdbusobject.c:109
  2 Thread 0x7f88a43010 (LWP 1159) 0x0000007f8a0a6ae8 in __GI___libc_free
  (mem=0x556a919c80) at malloc.c:3093

thread 1
(gdb) bt
0 0x0000007f8a48dda0 in g_dbus_object_get_object_path (object=0x0) at ../../../gio/gdbusobject.c:109
1 0x000000556a56911c in handle_get_block_devices (object=0x7f7c007ed0, invocation= 0x7f74016f20 [GDBusMethodInvocation], arg_options=)
at udiskslinuxmanager.c:1063

(gdb) p ((GObject*)(blocks_p->data))->ref_count
$3 = 1
(gdb) p ((GDBusInterfaceSkeleton)(blocks_p->data)) $6 = {parent_instance = {g_type_instance = {g_class = 0x556a64e740 [g_type: UDisksLinuxBlock/UDisksBlockSkeleton/GDBusInterfaceSkeleton]}, ref_count = 1, qdata = 0x0}, priv = 0x7f7c004ac0}
(gdb) p ((GDBusInterfaceSkeleton)(blocks_p->data))->priv $7 = {lock = {p = 0x0, i = {0, 0}}, object = 0x0,
flags = G_DBUS_INTERFACE_SKELETON_FLAGS_HANDLE_METHOD_INVOCATIONS_IN_THREAD, connections = 0x0, object_path = 0x0, hooked_vtable = 0x556a62b9f0}

thread 2
(gdb) bt
0 0x0000007f8a0a6ae8 in __GI___libc_free (mem=0x556a919c80) at malloc.c:3093 1 0x0000007f89ff1224 in () at /lib/aarch64-linux-gnu/libudev.so.1 2 0x0000007f89ff1348 in () at /lib/aarch64-linux-gnu/libudev.so.1 3 0x0000007f89ff5520 in () at /lib/aarch64-linux-gnu/libudev.so.1 4 0x0000007f89fff878 in udev_device_unref () at /lib/aarch64-linux-gnu/libudev.so.1 5 0x0000007f8a7aeb74 in () at /lib/aarch64-linux-gnu/libgudev-1.0.so.0 6 0x0000007f8a3193f8 in g_object_unref (_object=) at ../../../gobject/gobject.c:3346
7 0x0000007f8a3193f8 in g_object_unref (_object=0x7f680038a0) at ../../../gobject/gobject.c:3238
8 0x000000556a57700c in udisks_linux_device_finalize (object=0x7f5c005730 [UDisksLinuxDevice]) at udiskslinuxdevice.c:75
9 0x0000007f8a3193f8 in g_object_unref (_object=) at ../../../gobject/gobject.c:3346
10 0x0000007f8a3193f8 in g_object_unref (_object=0x7f5c005730) at ../../../gobject/gobject.c:3238
11 0x000000556a55d0fc in udisks_linux_drive_object_uevent
(object=object@entry=0x556a5df370 [UDisksLinuxDriveObject],
action=action@entry=0x556a87b120
"remove",device=device@entry=0x7f74007610 [UDisksLinuxDevice])
at udiskslinuxdriveobject.c:715
12 0x000000556a54840c in handle_block_uevent_for_drive
(provider=provider@entry=0x556a5c8200 [UDisksLinuxProvider],
action=action@entry=0x556a87b120 "remove",device=device@entry=0x7f74007610 [UDisksLinuxDevice]) at udiskslinuxprovider.c:1035 13 0x000000556a548ab8 in handle_block_uevent (device=0x7f74007610 [UDisksLinuxDevice], action=0x556a87b120 "remove", provider=0x556a5c8200 [UDisksLinuxProvider]) at udiskslinuxprovider.c:1349
14 0x000000556a548ab8 in udisks_linux_provider_handle_uevent
(provider=0x556a5c8200 [UDisksLinuxProvider], action=0x556a87b120 "remove",
device=0x7f74007610 [UDisksLinuxDevice]) at udiskslinuxprovider.c:1399 15 0x000000556a548cac in on_idle_with_probed_uevent (user_data=0x556a7e65a0) at udiskslinuxprovider.c:230

[StorageGhoul]

Can one of the admins verify this patch?

[tbzatek]

Good catch, thanks for reporting this. This is a race condition since all method invocations are handled in threads.

However the root of the problem here are references to UDisksObject not being held - only their block interfaces do carry a reference. So in the meantime as the object gets finalized in some other thread, it's not possible to get an object path anymore.

I would normally suggest to keep the object references but for the sole purpose of handle_get_block_devices() and handle_resolve_device() it makes no sense for the user.

Let's go with your fix, I would also ask you to fix another occurrence down in the handle_resolve_device() function.

[tbzatek]

Jenkins, ok to test.

[wxphaha]

Good catch, thanks for reporting this. This is a race condition since all method invocations are handled in threads.

However the root of the problem here are references to UDisksObject not being held - only their block interfaces do carry a reference. So in the meantime as the object gets finalized in some other thread, it's not possible to get an object path anymore.

I would normally suggest to keep the object references but for the sole purpose of handle_get_block_devices() and handle_resolve_device() it makes no sense for the user.

Let's go with your fix, I would also ask you to fix another occurrence down in the handle_resolve_device() function.

Thank you for your reply. I have also made the same modification in handle_resolve_device. The udisks_linux_block_matches_id called by this function will also get the object, but it uses the dup_object interface. This interface is thread_safe, so I think it only needs to be judged when calculating ret_paths. Please review.

[tbzatek]

Perfect, thank you!