mctpd: add_peer: Update userdata pointer for active event sources

Avoids a heap UAF caught by ASAN:

    ==179005==ERROR: AddressSanitizer: heap-use-after-free on address 0x61f000000c38 at pc 0x55dfaa7fa308 bp 0x7ffe10264420 sp 0x7ffe10264418
    READ of size 8 at 0x61f000000c38 thread T0
        #0 0x55dfaa7fa307 in peer_endpoint_recover ../src/mctpd.c:2570
        CodeConstruct#1 0x7f9a43dadae3  (/lib/x86_64-linux-gnu/libsystemd.so.0+0x78ae3)
        CodeConstruct#2 0x7f9a43dade04 in sd_event_dispatch (/lib/x86_64-linux-gnu/libsystemd.so.0+0x78e04)
        CodeConstruct#3 0x7f9a43daf2e7 in sd_event_run (/lib/x86_64-linux-gnu/libsystemd.so.0+0x7a2e7)
        CodeConstruct#4 0x7f9a43daf506 in sd_event_loop (/lib/x86_64-linux-gnu/libsystemd.so.0+0x7a506)
        CodeConstruct#5 0x55dfaa80a609 in main ../src/mctpd.c:4547
        CodeConstruct#6 0x7f9a42c46249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
        CodeConstruct#7 0x7f9a42c46304 in __libc_start_main_impl ../csu/libc-start.c:360
        CodeConstruct#8 0x55dfaa7e38d0 in _start (mctp/build/test-mctpd+0x688d0)

    0x61f000000c38 is located 3000 bytes inside of 3040-byte region [0x61f000000080,0x61f000000c60)
    freed by thread T0 here:
        #0 0x7f9a436b78d5 in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:85
        CodeConstruct#1 0x55dfaa7ef028 in add_peer ../src/mctpd.c:1419
        CodeConstruct#2 0x55dfaa7f1587 in endpoint_assign_eid ../src/mctpd.c:1601
        CodeConstruct#3 0x55dfaa7f55a0 in method_setup_endpoint ../src/mctpd.c:2038
        CodeConstruct#4 0x7f9a43d650ad  (/lib/x86_64-linux-gnu/libsystemd.so.0+0x300ad)

    previously allocated by thread T0 here:
        #0 0x7f9a436b78d5 in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:85
        CodeConstruct#1 0x55dfaa7ef028 in add_peer ../src/mctpd.c:1419
        CodeConstruct#2 0x55dfaa805741 in add_local_eid ../src/mctpd.c:4052
        CodeConstruct#3 0x55dfaa80627f in add_interface_local ../src/mctpd.c:4114
        CodeConstruct#4 0x55dfaa806ffa in setup_nets ../src/mctpd.c:4200
        CodeConstruct#5 0x55dfaa80a380 in main ../src/mctpd.c:4525
        CodeConstruct#6 0x7f9a42c46249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

    SUMMARY: AddressSanitizer: heap-use-after-free ../src/mctpd.c:2570 in peer_endpoint_recover
    Shadow bytes around the buggy address:
      0x0c3e7fff8130: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      0x0c3e7fff8140: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      0x0c3e7fff8150: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      0x0c3e7fff8160: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      0x0c3e7fff8170: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
    =>0x0c3e7fff8180: fd fd fd fd fd fd fd[fd]fd fd fd fd fa fa fa fa
      0x0c3e7fff8190: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c3e7fff81a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c3e7fff81b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c3e7fff81c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c3e7fff81d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    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
    ==179005==ABORTING

Signed-off-by: Andrew Jeffery <[email protected]>

Author: amboar

src/mctpd.c

@@ -1423,6 +1423,12 @@ static int add_peer(ctx *ctx, const dest_phys *dest, mctp_eid_t eid,
 		memset(&ctx->peers[ctx->size_peers], 0x0,
 			sizeof(*ctx->peers) * (new_size - ctx->size_peers));
 		ctx->size_peers = new_size;
+		// Update user data for recovery tasks
+		for (int ridx = 0; ridx < (ssize_t)ctx->size_peers; ridx++) {
+			peer = &ctx->peers[ridx];
+			if (sd_event_source_get_enabled(peer->recovery.source, NULL) > 0)
+				sd_event_source_set_userdata(peer->recovery.source, peer);
+		}
 	}
 
 	// Populate it

tests/test_mctpd.py

@@ -483,3 +483,68 @@ async def test_network_local_eids_none(dbus, mctpd):
     eids = list(await net.get_local_eids())
 
     assert eids == []
+
+async def test_concurrent_recovery_setup(dbus, mctpd):
+    iface = mctpd.system.interfaces[0]
+    mctp_i = await mctpd_mctp_iface_obj(dbus, iface)
+
+    # mctpd context tracks 20 peer objects by default, add and set up 19 so we
+    # reach the allocation boundary.
+    split = 19
+    for i in range(split):
+        pep = Endpoint(iface, bytes([0x1e + i]))
+        mctpd.network.add_endpoint(pep)
+        (_, _, path, _) = await mctp_i.call_setup_endpoint(pep.lladdr)
+
+    # Grab the DBus path for an endpoint that we will cause to be removed from
+    # the network through the recovery path. Arbitrarily use the most recent
+    # one added
+    ep = await dbus.get_proxy_object(MCTPD_C, path)
+    ep_props = await ep.get_interface(DBUS_PROPERTIES_I)
+
+    # Set up a match for Connectivity transitioning to Degraded on the endpoint
+    # for which we request recovery
+    degraded = trio.Semaphore(initial_value = 0)
+    def ep_connectivity_changed(iface, changed, invalidated):
+        if iface == MCTPD_ENDPOINT_I and 'Connectivity' in changed:
+            if 'Degraded' == changed['Connectivity'].value:
+                degraded.release()
+    await ep_props.on_properties_changed(ep_connectivity_changed)
+
+    # Set up a match for the recovery endpoint object being removed from DBus
+    mctp_p = await dbus.get_proxy_object(MCTPD_C, MCTPD_MCTP_P)
+    mctp_objmgr = await mctp_p.get_interface(DBUS_OBJECT_MANAGER_I)
+    removed = trio.Semaphore(initial_value = 0)
+    def ep_removed(ep_path, interfaces):
+        if ep_path == path and MCTPD_ENDPOINT_I in interfaces:
+            removed.release()
+
+    await mctp_objmgr.on_interfaces_removed(ep_removed)
+
+    # Delete the endpoint from the network so its recovery will fail after
+    # timeout. Note we delete at the split index as the network was already
+    # populated with the default endpoint
+    del mctpd.network.endpoints[split]
+
+    # Begin recovery for the endpoint ...
+    ep_ep = await ep.get_interface(MCTPD_ENDPOINT_I)
+    await ep_ep.call_recover()
+
+    # ... and wait until we're notified the recovery process has begun
+    with trio.move_on_after(1) as expected:
+        await degraded.acquire()
+    assert not expected.cancelled_caught
+
+    # Now that we're asynchronously waiting for the endpoint recovery process
+    # to complete, force a realloc() of the peer object array by adding a new
+    # peer, which will invalidate the recovering peer's pointer
+    pep = Endpoint(iface, bytes([0x1e + split]))
+    mctpd.network.add_endpoint(pep)
+    (_, _, _, new) = await mctp_i.call_setup_endpoint(pep.lladdr)
+    assert new
+
+    # Verify the recovery process completed gracefully with removal of the
+    # endpoint's DBus object
+    with trio.move_on_after(2 * MCTPD_TRECLAIM) as expected:
+        await removed.acquire()
+    assert not expected.cancelled_caught