free(): invalid pointer

[wuguanghao3]

A "free(): invalid pointer" error occurred during testing scenarios involving the creation of hundreds of paths and the deletion of broken links.
We captured the following call stack using ASan.

=================================================================
==2460381==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c00052c240 at pc 0x7fcb35049cd6 bp 0x7ffca42d89a0 sp 0x7ffca42d8148
WRITE of size 24 at 0x60c00052c240 thread T0
#0 0x7fcb35049cd5 in __interceptor_memset (/usr/lib64/libasan.so.8+0x49cd5)
#1 0x7fcb34f6b7a9 in memset /usr/include/bits/string_fortified.h:59
#2 0x7fcb34f6b7a9 in bucket_mark_free ../src/basic/hashmap.c:467
#3 0x7fcb34f6b7a9 in base_remove_entry ../src/basic/hashmap.c:579
#4 0x7fcb34f6ba85 in _hashmap_first_key_and_value ../src/basic/hashmap.c:1582
#5 0x7fcb34f783bf in _hashmap_clear.constprop.0 ../src/basic/hashmap.c:922
#6 0x7fcb34f7847b in _hashmap_free.part.0.constprop.0 ../src/basic/hashmap.c:897
#7 0x7fcb34f79151 in _hashmap_free ../src/basic/hashmap.c:896
#8 0x7fcb34f79151 in ordered_hashmap_free ../src/basic/hashmap.h:99
#9 0x7fcb34f79151 in device_free.isra.0 ../src/libsystemd/sd-device/sd-device.c:73
#10 0x7fcb34f5e407 in sd_device_unref ../src/libsystemd/sd-device/sd-device.c:85
#11 0x7fcb34f5e407 in udev_device_free ../src/libudev/libudev-device.c:478
#12 0x7fcb34f5e407 in udev_device_unref ../src/libudev/libudev-device.c:508
#13 0x7fcb34f5e410 in udev_device_free ../src/libudev/libudev-device.c:479
#14 0x7fcb34f5e410 in udev_device_unref ../src/libudev/libudev-device.c:508
#15 0x7fcb34f919c1 in mt_udev_device_unref /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmpathutil/mt-libudev.c:189
#16 0x7fcb357000ab in free_path /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/structs.c:172
#17 0x7fcb357000ab in free_path /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/structs.c:164
#18 0x7fcb357001c4 in free_pathvec /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/structs.c:194
#19 0x55e5755955e6 in cleanup_paths /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:3301
#20 0x55e5755955e6 in cleanup_vecs /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:3316
#21 0x55e5755955e6 in cleanup_child /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:3418
#22 0x7fcb34cce134 (/usr/lib64/libc.so.6+0x3a134)
#23 0x7fcb34cce22f in exit (/usr/lib64/libc.so.6+0x3a22f)
#24 0x7fcb34cb7616 (/usr/lib64/libc.so.6+0x23616)
#25 0x7fcb34cb76c8 in __libc_start_main (/usr/lib64/libc.so.6+0x236c8)
#26 0x55e5755935d4 (/usr/sbin/multipathd+0x145d4)

0x60c00052c240 is located 0 bytes inside of 128-byte region [0x60c00052c240,0x60c00052c2c0)

freed by thread T7 here:
#0 0x7fcb350bf470 (/usr/lib64/libasan.so.8+0xbf470)
#1 0x7fcb34f77c4c in resize_buckets ../src/basic/hashmap.c:1118
#2 0x7fcb34f77c4c in hashmap_base_put_boldly.constprop.0 ../src/basic/hashmap.c:1027
#3 0x7fcb34f6c349 in hashmap_replace ../src/basic/hashmap.c:1331
#4 0x7fcb34f62a7e in ordered_hashmap_replace ../src/basic/hashmap.h:165
#5 0x7fcb34f62a7e in device_add_property_aux ../src/libsystemd/sd-device/sd-device.c:118
#6 0x7fcb34f65a2f in device_add_property_internal ../src/libsystemd/sd-device/device-internal.h:104
#7 0x7fcb34f65a2f in handle_uevent_line ../src/libsystemd/sd-device/sd-device.c:730
#8 0x7fcb34f65a2f in device_read_uevent_file ../src/libsystemd/sd-device/sd-device.c:803
#9 0x7fcb34f67583 in sd_device_get_devtype ../src/libsystemd/sd-device/sd-device.c:1212
#10 0x7fcb34f67583 in sd_device_get_parent_with_subsystem_devtype ../src/libsystemd/sd-device/sd-device.c:1243
#11 0x7fcb34f67583 in udev_device_get_parent_with_subsystem_devtype ../src/libudev/libudev-device.c:448
#12 0x7fcb34f91c50 in mt_udev_device_get_parent_with_subsystem_devtype /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmpathutil/mt-libudev.c:229
#13 0x7fcb3570b5ea in sysfs_get_tgt_nodename /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/discovery.c:455
#14 0x7fcb3570b5ea in scsi_sysfs_pathinfo /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/discovery.c:1669
#15 0x7fcb3570b5ea in sysfs_pathinfo /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/discovery.c:1979
#16 0x7fcb3570b5ea in pathinfo /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/discovery.c:2474
#17 0x7fcb3570b5ea in pathinfo /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/discovery.c:2415
#18 0x55e57559ebd2 in uev_update_path /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:1608
#19 0x55e57559fcfb in uev_trigger /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:1781
#20 0x7fcb3572b712 in service_uevq /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/uevent.c:489
#21 0x7fcb3572d109 in uevent_dispatch /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/uevent.c:581
#22 0x55e575594b9b in uevqloop /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:1810
#23 0x7fcb34d15fdd (/usr/lib64/libc.so.6+0x81fdd)
#24 0x7fcb34d9525b (/usr/lib64/libc.so.6+0x10125b)

previously allocated by thread T7 here:
#0 0x7fcb350bf470 (/usr/lib64/libasan.so.8+0xbf470)
#1 0x7fcb34f77c4c in resize_buckets ../src/basic/hashmap.c:1118
#2 0x7fcb34f77c4c in hashmap_base_put_boldly.constprop.0 ../src/basic/hashmap.c:1027
#3 0x7fcb34f6c349 in hashmap_replace ../src/basic/hashmap.c:1331
#4 0x7fcb34f62a7e in ordered_hashmap_replace ../src/basic/hashmap.h:165
#5 0x7fcb34f62a7e in device_add_property_aux ../src/libsystemd/sd-device/sd-device.c:118
#6 0x7fcb34f69987 in device_add_property_internal ../src/libsystemd/sd-device/device-internal.h:104
#7 0x7fcb34f69987 in device_set_subsystem ../src/libsystemd/sd-device/sd-device.c:1115
#8 0x7fcb34f72573 in sd_device_get_subsystem ../src/libsystemd/sd-device/sd-device.c:1181
#9 0x7fcb34f5debd in udev_device_get_subsystem ../src/libudev/libudev-device.c:173
#10 0x7fcb34f91df1 in mt_udev_device_get_subsystem /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmpathutil/mt-libudev.c:256
#11 0x7fcb3570b38b in scsi_sysfs_pathinfo /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/discovery.c:1616
#12 0x7fcb3570b38b in sysfs_pathinfo /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/discovery.c:1979
#13 0x7fcb3570b38b in pathinfo /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/discovery.c:2474
#14 0x7fcb3570b38b in pathinfo /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/discovery.c:2415
#15 0x55e57559ebd2 in uev_update_path /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:1608
#16 0x55e57559fcfb in uev_trigger /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:1781
#17 0x7fcb3572b712 in service_uevq /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/uevent.c:489
#18 0x7fcb3572d109 in uevent_dispatch /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/libmultipath/uevent.c:581
#19 0x55e575594b9b in uevqloop /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:1810
#20 0x7fcb34d15fdd (/usr/lib64/libc.so.6+0x81fdd)
#21 0x7fcb34d9525b (/usr/lib64/libc.so.6+0x10125b)

Thread T7 created by T0 here:
#0 0x7fcb3504bec5 in __interceptor_pthread_create (/usr/lib64/libasan.so.8+0x4bec5)
#1 0x55e57559babb in child /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:3595
#2 0x55e575592ef9 in main /usr/src/debug/multipath-tools-0.9.5-7.h21510.9y.x86_64/multipathd/main.c:3878
#3 0x7fcb34cb760f (/usr/lib64/libc.so.6+0x2360f)

SUMMARY: AddressSanitizer: heap-use-after-free (/usr/lib64/libasan.so.8+0x49cd5) in __interceptor_memset
Shadow bytes around the buggy address:
0x0c188009d7f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c188009d800: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c188009d810: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c188009d820: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x0c188009d830: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c188009d840: fa fa fa fa fa fa fa fa[fd]fd fd fd fd fd fd fd
0x0c188009d850: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x0c188009d860: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c188009d870: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c188009d880: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x0c188009d890: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==2460381==ABORTING

I saw this scenario discussed previously, but no solution was provided. systemd/systemd#18376
Applying a lock to udev calls does not fully resolve the UAF issue that arises from the interaction between pthread_cancel() and the resource cleanup during process termination.
Are there any other solutions for this scenario?

[wuguanghao3]

Since exclusive locks are already in place to protect udev calls, is it possible to wait for the udev lock before executing pthread_cancel()?

[mwilck]

Which version of multipath-tools are you testing?

[mwilck]

I'm asking because we introduced libudev wrappers in 0.14.0 that should mitigate this issue (2dafe7f ("multipath-tools: use the libudev wrapper functions"). 1ef540a ("multipath-tools: compile with -fno-strict-aliasing") (also in 0.14.0) also helped fixing issues of this kind for me.

[wuguanghao3]

I'm asking because we introduced libudev wrappers in 0.14.0 that should mitigate this issue (2dafe7f ("multipath-tools: use the libudev wrapper functions"). 1ef540a ("multipath-tools: compile with -fno-strict-aliasing") (also in 0.14.0) also helped fixing issues of this kind for me.

I am using version 0.9.5, which includes the commit 2dafe7f. I'll re-test this commit 1ef540a ("multipath-tools: compile with -fno-strict-aliasing").

Thanks for the reply.

[mwilck]

I am using version 0.9.5, which includes the commit 2dafe7f

No, it does not. 0.9.5 is more than 600 commits behind 2dafe7f. It's in 0.14.0 and newer only.

[mwilck]

In general, when you report issues here, I have to ask you to test with the master branch or later.

[wuguanghao3]

In general, when you report issues here, I have to ask you to test with the master branch or later.

I am currently using version 0.9.5 with the two commits mentioned above applied, but I am still encountering the UAF issue. Are there any other commits that might be related to fixing this problem? Unfortunately, I cannot upgrade directly to the latest version.

I am still testing the latest version, and so far, no issues have come up.

[mwilck]

I am currently using version 0.9.5 with the two commits mentioned above applied, but I am still encountering the UAF issue. Are there any other commits that might be related to fixing this problem?

Not off the top of my head. We've done lots of cleanups since then. Can you share the test program(s) you are using?

It's possible that you'd need to modify 2dafe7f. You must make sure that every call to libudev is protected by the lock. If you just apply this patch to 0.9.5, there might be libudev calls that didn't exist in the pre-0.14.0 code any more.

I am still testing the latest version, and so far, no issues have come up.

That sounds promising! Fingers crossed that it will stay this way.
If this finding is confirmed, you may even try a bisection technique to find the fix.

Looking at your stack traces, the problem is so deeply inside libsystemd that it might actually be a systemd issue. In particular the fact that the memory that causes the error had been freed in a call stack involving device_add_property_internal(). What version of systemd do you use?

[wuguanghao3]

Unfortunately, the same issue persists in the latest version. Adding a mutex lock to the libudev interface resolves the runtime
multi-threading concurrency issue. However, it fails to fix the UAF issue caused by pthread_cancel() terminating the thread before the udev interface completes its execution.

As long as we ensure that threads are not cancelled during libudev API calls, the issue no longer reproduces.