Skip to content

darwin: keep the parent device link correct across re-enumeration - #1917

Open
sonatique wants to merge 2 commits into
libusb:masterfrom
sonatique:darwin-parent-device-reenumeration
Open

darwin: keep the parent device link correct across re-enumeration#1917
sonatique wants to merge 2 commits into
libusb:masterfrom
sonatique:darwin-parent-device-reenumeration

Conversation

@sonatique

@sonatique sonatique commented Aug 3, 2026

Copy link
Copy Markdown
Member

Two fixes in the darwin re-enumeration path, found while investigating the
libusb_get_port_numbers / process_new_device report on #1798. They are different
kinds of fix in the same code, so they are separate commits. A companion fix bounding
the reader's fill loop is in PR#1915.

darwin: refresh the cached parent session on re-enumeration (functional, no
concurrency involved)

darwin_get_cached_device assigns parent_session on every path, not only when it
allocates a cached device, so a device returning from a re-enumeration carries the
session id of the parent it currently sits behind. When a parent hub is re-enumerated
and receives a new session id, its children resolve that new id the next time they are
re-enumerated, and libusb_get_parent and libusb_get_port_numbers keep reporting the
real topology.

darwin: publish the new parent device before releasing the old one (addendum item
12 of #1798)

process_new_device resolves the new parent and stores it before releasing the previous
one, so dev->parent_dev always holds either NULL or a device this device owns a
reference to. A concurrent libusb_get_port_numbers walk sees one parent or the other,
with no NULL gap while the lookup runs. Reference counting stays balanced:
usbi_get_device_by_session_id returns a referenced device, so this is the same +1/-1
pair in the opposite order.

It narrows the reader race rather than closing it. A walk that already loaded the old
pointer can still dereference it after the release, and closing that needs the reader to
hold a reference, which is a core-level change.

Reference: #1798 (addendum item 12)

Regarding testing:
The first commit has a visible symptom and is the one worth exercising: with a device
behind a hub, replug or re-enumerate the hub, then reset the child, and check that
libusb_get_parent and the port path still resolve. hotplugtest plus testlibusb
covers it.

The second has no targeted reproducer, so the usual suite for no-regression
is enough.

If somebody re-runs the item-9/10 tester (its scan thread calls libusb_get_port_numbers),
the TSan report on parent_dev still appears. It flags the unlocked read of the field,
which neither commit changes; the recent clean runs used tests that never walk the
parent chain, so they did not exercise it. Silencing it needs the field made atomic, or
both the reader and the backend writer holding a common lock, with ctx->usb_devs_lock
the natural candidate since usbi_hotplug_exit already walks parent chains under it.
Both are core changes, and separate from the use-after-free question above.

darwin_get_cached_device assigns parent_session on every path, not
only when it allocates a cached device, so a device returning from a
re-enumeration carries the session id of the parent it currently sits
behind.

A parent hub that is itself re-enumerated receives a new session id.
Its children now resolve that new id the next time they are
re-enumerated, so libusb_get_parent keeps returning the parent and
libusb_get_port_numbers keeps reporting the full path.

Reference: libusb#1798
process_new_device resolves the new parent and stores it into
dev->parent_dev before releasing the previous one, so the field always
holds either NULL or a device this device owns a reference to.

The device being updated is already visible to the application, and
libusb_get_port_numbers walks the parent chain without a lock. A
concurrent walk therefore sees one parent or the other for the whole
update, with no NULL gap while the lookup runs and no pointer to a
device that has been freed. That matters because the reader measures
the chain depth and fills the caller's array in two separate passes,
and a chain that changes length between them yields a result that does
not match what was measured. Where the parent is unchanged the stored
value is identical, so no transition is observable at all.

Reference counting stays balanced: usbi_get_device_by_session_id
returns a referenced device, so this is the same +1/-1 pair in the
opposite order.

The window is narrowed rather than closed. A walk that loaded the old
pointer before the store can still dereference it after the release;
closing that requires the reader to hold a reference, which is a
core-level change.

Reference: libusb#1798 (addendum item 12)
@Youw

Youw commented Aug 4, 2026

Copy link
Copy Markdown
Member

@sonatique I assume this was (at least partially) generated by AI? or was this all you?

@sonatique

Copy link
Copy Markdown
Member Author

@sonatique I assume this was (at least partially) generated by AI? or was this all you?

@Youw: the code (2 commits) is mine, but indeed the PR description is from Claude,

I was working concurrently on this PR and the more complicated #1919. I had these commits done on one branch and started the other one on top, but I finally decided to rely on Claude for #1919 because I was getting lost.

When done, I re-separated the commits to make 2 PRs and asked Claude to make the PR description for that one too, out of laziness.

Should I add a line saying the description is AI generated?

@Youw

Youw commented Aug 4, 2026

Copy link
Copy Markdown
Member

Should I add a line saying the description is AI generated?

That is still an open question.

For the most part people mostly care about the code being "not poluted by AI bugs".

When I see this much of the text in PR description, I automatically assume human is too lazy to write it, so it must be an AI. But then, if AI has written the description, maybe the code too. But it is not the case this time...

Maybe we should extend our AGENTS.md to have Assisted-by: attribution if the code was AI generated, and have the Description assisted-by: in case if AI generated the description/title, but the code was written by human.

@Youw

Youw commented Aug 4, 2026

Copy link
Copy Markdown
Member

Codex reviewed both commits at 67dc4536e2736bdfa1df093080b89039a3caff53 against their base and current master (including the merged #1915). No actionable defects were found in this diff.

  • parent_session is now refreshed on the location/in_reenumerate reuse path; fresh-device behavior is unchanged.
  • The parent swap keeps reference accounting balanced when the old and new parents are identical, different, or the new lookup returns NULL: the lookup's new reference is acquired before the old reference is released.
  • This remains a partial mitigation of Darwin backend: concurrency audit (8 issues, partial PR coverage, proposed roadmap) #1798 item 12, not a complete concurrency fix. parent_dev is still read and written without common synchronization, and a walker that loaded the old pointer can still dereference it after the writer releases it. The PR description already states this accurately, so the existing TSan report is expected to remain.
  • git diff --check is clean, and the macOS workflow for this head completed successfully.

Generated by Codex (GPT-5.6 Sol).

@seanm

seanm commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

...and asked Claude to make the PR description for that one too, out of laziness.

The problem is: these AIs are always so verbose, reading everything they output takes so much time. "If you're too lazy to write it, then I'm too lazy to read it" is a sentiment I'm seeing more and more, and I'm starting to appreciate it. It's easy to generate tonnes and tonnes of AI text, but hard and slow to read it all. (I'm not trying to pick on you here, this is happening everywhere.)

@Youw

Youw commented Aug 5, 2026

Copy link
Copy Markdown
Member

these AIs are always so verbose

There is two sides to the coin to this.

I agree by default it generates tons of code and reading it by a human usually exhausting. This actually can easily be adjusted by giving a corresponding AGENTS.md directive.

On the other hand, I usually use other agents to read/analyze/fix and give me a short summary, and in such cases - the more context provided for input - the higher chance to get an accurate secondary analysys.

@mcuee

mcuee commented Aug 7, 2026

Copy link
Copy Markdown
Member

Let me test this PR over the weekend.

@mcuee
mcuee self-requested a review August 7, 2026 02:19
@mcuee

mcuee commented Aug 7, 2026

Copy link
Copy Markdown
Member

First test -- make check passes the normal build, TSAN debug build and ASAN debug build.

@mcuee

mcuee commented Aug 10, 2026

Copy link
Copy Markdown
Member

Test code created by Claude Sonnet 5 Medium (free web chatbot).

mcuee in ~/build/libusb/libusb_test/issue1798_point12_pr1917 % cat issue1798_point12_reenum_parent_pr1917.c 
/*
 * issue1798_point12_reenum_parent_pr1917.c
 *
 * Same reproducer as issue1798_point12_reenum_parent.c, retargeted to build
 * against libusb PR #1917 ("darwin: publish the new parent device before
 * releasing the old one", commit 67dc4536e2736bdfa1df093080b89039a3caff53)
 * instead of system libusb, so a clean run is evidence the fix holds.
 *
 * Build libusb itself first, pinned to the exact PR commit:
 *
 *   git clone https://github.com/libusb/libusb.git
 *   cd libusb
 *   git fetch origin pull/1917/head:pr1917
 *   git checkout 67dc4536e2736bdfa1df093080b89039a3caff53
 *   ./bootstrap.sh
 *   CC=clang CFLAGS="-g -O1 -fsanitize=thread" \
 *   LDFLAGS="-fsanitize=thread" \
 *       ./configure --disable-shared
 *   make -j"$(sysctl -n hw.ncpu)"
 *
 * Then build this test against that tree's headers/static lib directly
 * (NOT pkg-config, which would pick up your installed libusb instead):
 *
 *   clang -Wall -Wextra -g -O1 -fsanitize=thread \
 *       -I/path/to/libusb/libusb \
 *       -o issue1798_point12_reenum_parent_pr1917 issue1798_point12_reenum_parent_pr1917.c \
 *       /path/to/libusb/libusb/.libs/libusb-1.0.a \
 *       -framework CoreFoundation -framework IOKit -lpthread
 *
 * Recommended: also build and run against plain master (checkout master
 * instead of the PR commit) as a baseline, since PR #1917's own commit
 * message is explicit that this NARROWS the race rather than closing it --
 * a walk that already loaded the old parent_dev pointer before the store
 * can still dereference it after the unref, and that residual window needs
 * a core-level fix (the reader holding a reference) that is out of scope
 * for this PR. So the expected, honest outcome here is:
 *   - master:  frequent TSan reports AND occasional crashes (the old code
 *              has a real NULL-gap window plus a use-after-free window)
 *   - PR 1917: the NULL-gap class of failure should be gone; TSan may
 *              still flag parent_dev as a data race (it is, by design,
 *              still an unsynchronized pointer write vs. lock-free reads),
 *              and a crash is still theoretically possible if a walker
 *              loads the pointer right before the release -- just far
 *              rarer, since the window is now only the unref call itself
 *              rather than the whole gap between clearing and re-resolving.
 * Don't read "PR still shows a TSan report" alone as a regression; read
 * "PR still crashes at a similar RATE to master" as the real red flag.
 *
 * Reproducer for libusb/libusb#1798, addendum item 12:
 *   "re-enumeration reuse path mutates published libusb_device fields
 *    racing the lock-free public getters"
 *
 * When a device re-enumerates, the hotplug thread's process_new_device()
 * finds the existing, already-published libusb_device (matched by its old
 * session id) and updates it in place, e.g.:
 *
 *     libusb_unref_device(dev->parent_dev);
 *     dev->parent_dev = NULL;
 *     ...
 *     dev->parent_dev = usbi_get_device_by_session_id(...);
 *
 * plus session_data and the device_descriptor memcpy. None of this is
 * protected against concurrent readers, because the public getters are
 * lock-free by design:
 *
 *     libusb_get_port_numbers()     -- walks dev->parent_dev chain
 *     libusb_get_device_descriptor() -- copies dev->device_descriptor
 *
 * The severe case is parent_dev: the unref in process_new_device can drop
 * the parent's last reference and free it while another thread is still
 * mid-walk inside libusb_get_port_numbers() dereferencing that same
 * parent_dev pointer -- a use-after-free, not just a torn read.
 *
 * This test needs:
 *   - the target device plugged in BEHIND AN EXTERNAL HUB, so parent_dev
 *     is a real, distinct libusb_device (root-port devices may have a
 *     trivial/absent parent and won't exercise the free path as clearly).
 *   - a target device that actually re-enumerates on libusb_reset_device().
 *     Not all devices do; if yours doesn't, unplug/replug by hand (or
 *     script it with an external port-power tool, e.g. uhubctl) instead
 *     of the reset thread below, using --no-reset.
 *
 * Build (TSan strongly recommended, ideally against a TSan-built libusb,
 * since the race is inside libusb, not this test):
 *
 * (See the build instructions above this comment for the PR-pinned build --
 * do not use pkg-config here, it will silently link your installed libusb.)
 *
 * Usage:
 *   ./issue1798_point12_reenum_parent_pr1917 <vid_hex> <pid_hex> [walkers=8] \
 *       [duration_s=20] [--no-reset]
 *
 * With --no-reset, the resetter thread is skipped; drive re-enumeration
 * externally (manual unplug/replug, or a hub port-power script) while the
 * walkers run.
 */

#include <stdio.h>
#include <stdlib.h>
#include <stdatomic.h>
#include <string.h>
#include <unistd.h>
#include <pthread.h>
#include <time.h>
#include <libusb-1.0/libusb.h>

static libusb_context *g_ctx   = NULL;
static unsigned         g_vid, g_pid;
static atomic_int        g_stop = 0;
static atomic_long       g_walks       = 0;
static atomic_long       g_walk_fail   = 0;
static atomic_long       g_desc_reads  = 0;
static atomic_long       g_resets      = 0;
static atomic_long       g_reset_fail  = 0;

/* Find our target device in a freshly-obtained device list. Returns a
 * ref'd libusb_device (caller must libusb_unref_device it), or NULL. */
static libusb_device *find_target(libusb_device **list, ssize_t n)
{
    for (ssize_t i = 0; i < n; i++) {
        struct libusb_device_descriptor desc;
        if (libusb_get_device_descriptor(list[i], &desc) != LIBUSB_SUCCESS)
            continue;
        if (desc.idVendor == g_vid && desc.idProduct == g_pid)
            return libusb_ref_device(list[i]);
    }
    return NULL;
}

/* Walker threads: repeatedly re-list devices, then hammer the lock-free
 * public getters on the target device -- libusb_get_port_numbers() walks
 * dev->parent_dev, libusb_get_device_descriptor() copies the descriptor.
 * Both fields are what process_new_device() mutates in place on
 * re-enumeration. */
static void *walker_main(void *arg)
{
    (void)arg;
    while (!atomic_load_explicit(&g_stop, memory_order_relaxed)) {
        libusb_device **list = NULL;
        ssize_t n = libusb_get_device_list(g_ctx, &list);
        if (n < 0) {
            usleep(1000);
            continue;
        }

        libusb_device *dev = find_target(list, n);
        libusb_free_device_list(list, 1);

        if (dev == NULL) {
            usleep(1000);
            continue;
        }

        /* Hammer the walk many times per list acquisition so we spend as
         * much time as possible inside the parent_dev chain-walk while
         * the resetter thread's reenumeration is in flight. */
        for (int i = 0; i < 200 && !atomic_load_explicit(&g_stop, memory_order_relaxed); i++) {
            uint8_t path[8];
            int rc = libusb_get_port_numbers(dev, path, sizeof(path));
            if (rc < 0)
                atomic_fetch_add(&g_walk_fail, 1);
            else
                atomic_fetch_add(&g_walks, 1);

            struct libusb_device_descriptor desc;
            if (libusb_get_device_descriptor(dev, &desc) == LIBUSB_SUCCESS)
                atomic_fetch_add(&g_desc_reads, 1);
        }

        libusb_unref_device(dev);
    }
    return NULL;
}

/* Resetter thread: repeatedly opens the target and calls
 * libusb_reset_device(), which on Darwin can drive darwin_reenumerate_device
 * -> process_new_device() on the hotplug thread when the device actually
 * re-enumerates. If your device doesn't re-enumerate on reset, run with
 * --no-reset and trigger re-enumeration externally instead. */
static void *resetter_main(void *arg)
{
    (void)arg;
    while (!atomic_load_explicit(&g_stop, memory_order_relaxed)) {
        libusb_device **list = NULL;
        ssize_t n = libusb_get_device_list(g_ctx, &list);
        if (n < 0) {
            usleep(5000);
            continue;
        }
        libusb_device *dev = find_target(list, n);
        libusb_free_device_list(list, 1);
        if (dev == NULL) {
            usleep(5000);
            continue;
        }

        libusb_device_handle *handle = NULL;
        if (libusb_open(dev, &handle) == LIBUSB_SUCCESS) {
            int rc = libusb_reset_device(handle);
            if (rc == LIBUSB_SUCCESS)
                atomic_fetch_add(&g_resets, 1);
            else
                atomic_fetch_add(&g_reset_fail, 1);
            libusb_close(handle);
        }
        libusb_unref_device(dev);

        /* Give re-enumeration (device disappearing/reappearing on the
         * hotplug thread) time to actually land before we reset again. */
        usleep(150 * 1000);
    }
    return NULL;
}

int main(int argc, char **argv)
{
    if (argc < 3) {
        fprintf(stderr,
            "usage: %s <vid_hex> <pid_hex> [walkers=8] [duration_s=20] [--no-reset]\n",
            argv[0]);
        return 2;
    }

    g_vid = (unsigned)strtoul(argv[1], NULL, 16);
    g_pid = (unsigned)strtoul(argv[2], NULL, 16);
    int walkers    = (argc > 3) ? atoi(argv[3]) : 8;
    int duration_s = (argc > 4) ? atoi(argv[4]) : 20;
    int do_reset   = 1;
    for (int i = 5; i < argc; i++)
        if (strcmp(argv[i], "--no-reset") == 0)
            do_reset = 0;

    int rc = libusb_init(&g_ctx);
    if (rc != LIBUSB_SUCCESS) {
        fprintf(stderr, "libusb_init failed: %s\n", libusb_error_name(rc));
        return 1;
    }

    fprintf(stderr,
        "point12: %d walkers hammering get_port_numbers/get_device_descriptor "
        "on %04x:%04x for %ds, resetter %s\n",
        walkers, g_vid, g_pid, duration_s, do_reset ? "ON" : "OFF (drive re-enum externally)");

    pthread_t *wtids = calloc((size_t)walkers, sizeof(pthread_t));
    for (int i = 0; i < walkers; i++)
        pthread_create(&wtids[i], NULL, walker_main, NULL);

    pthread_t rtid;
    if (do_reset)
        pthread_create(&rtid, NULL, resetter_main, NULL);

    sleep((unsigned)duration_s);

    atomic_store(&g_stop, 1);

    for (int i = 0; i < walkers; i++)
        pthread_join(wtids[i], NULL);
    if (do_reset)
        pthread_join(rtid, NULL);

    fprintf(stderr,
        "point12: done. walks ok=%ld fail=%ld, desc_reads=%ld, resets ok=%ld fail=%ld\n"
        "point12: a crash/UAF or TSan report during the run is the finding; "
        "clean completion does not rule out the race (needs an actual "
        "re-enumeration to fire mid-walk).\n",
        (long)g_walks, (long)g_walk_fail, (long)g_desc_reads,
        (long)g_resets, (long)g_reset_fail);

    free(wtids);
    libusb_exit(g_ctx);
    return 0;
}

libusb git TSan build will crash.

mcuee in ~/build/libusb/libusb_test/issue1798_point12_pr1917 % clang -Wall -Wextra -g -fsanitize=thread -I /Users/mcuee/mybin/bin_git_tsan_debug/include issue1798_point12_reenum_parent.c -o issue1798_point12_reenum_parent_git_tsan -L /Users/mcuee/mybin/bin_git_tsan_debug/lib -lusb-1.0 -pthread

mcuee in ~/build/libusb/libusb_test/issue1798_point12_pr1917 % ./issue1798_point12_reenum_parent_git_tsan 04b4 8613
point12: 8 walkers hammering get_port_numbers/get_device_descriptor on 04b4:8613 for 20s, resetter ON
==================
WARNING: ThreadSanitizer: data race (pid=14733)
  Write of size 8 at 0x00010be02590 by thread T1 (mutexes: write M0):
    #0 process_new_device darwin_usb.c:1787 (libusb-1.0.0.dylib:arm64+0x1c698)
    #1 darwin_devices_attached darwin_usb.c:706 (libusb-1.0.0.dylib:arm64+0x1b29c)
    #2 IODispatchCalloutFromCFMessage <null> (IOKit:arm64e+0x5244)

  Previous read of size 8 at 0x00010be02590 by thread T7:
    #0 libusb_get_port_numbers core.c:1008 (libusb-1.0.0.dylib:arm64+0x2234)
    #1 walker_main issue1798_point12_reenum_parent.c:116 (issue1798_point12_reenum_parent_git_tsan:arm64+0x100000dac)

  Location is heap block of size 120 at 0x00010be02580 allocated by main thread:
    #0 calloc <null> (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x67c40)
    #1 usbi_alloc_device core.c:730 (libusb-1.0.0.dylib:arm64+0xe38)
    #2 process_new_device darwin_usb.c:1757 (libusb-1.0.0.dylib:arm64+0x1c460)
    #3 darwin_scan_devices darwin_usb.c:1852 (libusb-1.0.0.dylib:arm64+0x19b0c)
    #4 darwin_init_context darwin_usb.c:1004 (libusb-1.0.0.dylib:arm64+0x19634)
    #5 darwin_init darwin_usb.c:1008 (libusb-1.0.0.dylib:arm64+0x17130)
    #6 libusb_init_context core.c:2704 (libusb-1.0.0.dylib:arm64+0x60dc)
    #7 libusb_init core.c:2584 (libusb-1.0.0.dylib:arm64+0x5a14)
    #8 main issue1798_point12_reenum_parent.c:190 (issue1798_point12_reenum_parent_git_tsan:arm64+0x100000904)

  Mutex M0 (0x000102624000) created at:
    #0 pthread_mutex_lock <null> (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x356bc)
    #1 usbi_mutex_static_lock threads_posix.h:35 (libusb-1.0.0.dylib:arm64+0x5924)
    #2 libusb_init_context core.c:2621 (libusb-1.0.0.dylib:arm64+0x5b54)
    #3 libusb_init core.c:2584 (libusb-1.0.0.dylib:arm64+0x5a14)
    #4 main issue1798_point12_reenum_parent.c:190 (issue1798_point12_reenum_parent_git_tsan:arm64+0x100000904)

  Thread T1 (tid=472837, running) created by main thread at:
    #0 pthread_create <null> (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x33834)
    #1 darwin_first_time_init darwin_usb.c:969 (libusb-1.0.0.dylib:arm64+0x19808)
    #2 darwin_init_context darwin_usb.c:996 (libusb-1.0.0.dylib:arm64+0x195f4)
    #3 darwin_init darwin_usb.c:1008 (libusb-1.0.0.dylib:arm64+0x17130)
    #4 libusb_init_context core.c:2704 (libusb-1.0.0.dylib:arm64+0x60dc)
    #5 libusb_init core.c:2584 (libusb-1.0.0.dylib:arm64+0x5a14)
    #6 main issue1798_point12_reenum_parent.c:190 (issue1798_point12_reenum_parent_git_tsan:arm64+0x100000904)

  Thread T7 (tid=472843, running) created by main thread at:
    #0 pthread_create <null> (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x33834)
    #1 main issue1798_point12_reenum_parent.c:203 (issue1798_point12_reenum_parent_git_tsan:arm64+0x100000a64)

SUMMARY: ThreadSanitizer: data race darwin_usb.c:1787 in process_new_device
==================
point12: done. walks ok=30441103 fail=0, desc_reads=30441103, resets ok=86 fail=0
point12: a crash/UAF or TSan report during the run is the finding; clean completion does not rule out the race (needs an actual re-enumeration to fire mid-walk).
ThreadSanitizer: reported 1 warnings
zsh: abort      ./issue1798_point12_reenum_parent_git_tsan 04b4 8613

This PR also crashes.

mcuee in ~/build/libusb/libusb_test/issue1798_point12_pr1917 % ./issue1798_point12_reenum_parent_pr1917_tsan 04b4 8613
point12: 8 walkers hammering get_port_numbers/get_device_descriptor on 04b4:8613 for 20s, resetter ON
==================
WARNING: ThreadSanitizer: data race (pid=14743)
  Write of size 8 at 0x00010e102590 by thread T1 (mutexes: write M0):
    #0 process_new_device darwin_usb.c:1799 (libusb-1.0.0.dylib:arm64+0x1c6b4)
    #1 darwin_devices_attached darwin_usb.c:706 (libusb-1.0.0.dylib:arm64+0x1b288)
    #2 IODispatchCalloutFromCFMessage <null> (IOKit:arm64e+0x5244)

  Previous read of size 8 at 0x00010e102590 by thread T9:
    #0 libusb_get_port_numbers core.c:1008 (libusb-1.0.0.dylib:arm64+0x2234)
    #1 walker_main issue1798_point12_reenum_parent.c:116 (issue1798_point12_reenum_parent_pr1917_tsan:arm64+0x100000dac)

  Location is heap block of size 120 at 0x00010e102580 allocated by main thread:
    #0 calloc <null> (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x67c40)
    #1 usbi_alloc_device core.c:730 (libusb-1.0.0.dylib:arm64+0xe38)
    #2 process_new_device darwin_usb.c:1761 (libusb-1.0.0.dylib:arm64+0x1c44c)
    #3 darwin_scan_devices darwin_usb.c:1867 (libusb-1.0.0.dylib:arm64+0x19af8)
    #4 darwin_init_context darwin_usb.c:1004 (libusb-1.0.0.dylib:arm64+0x19620)
    #5 darwin_init darwin_usb.c:1008 (libusb-1.0.0.dylib:arm64+0x1711c)
    #6 libusb_init_context core.c:2700 (libusb-1.0.0.dylib:arm64+0x60c8)
    #7 libusb_init core.c:2580 (libusb-1.0.0.dylib:arm64+0x5a00)
    #8 main issue1798_point12_reenum_parent.c:190 (issue1798_point12_reenum_parent_pr1917_tsan:arm64+0x100000904)

  Mutex M0 (0x00010497c000) created at:
    #0 pthread_mutex_lock <null> (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x356bc)
    #1 usbi_mutex_static_lock threads_posix.h:35 (libusb-1.0.0.dylib:arm64+0x5910)
    #2 libusb_init_context core.c:2617 (libusb-1.0.0.dylib:arm64+0x5b40)
    #3 libusb_init core.c:2580 (libusb-1.0.0.dylib:arm64+0x5a00)
    #4 main issue1798_point12_reenum_parent.c:190 (issue1798_point12_reenum_parent_pr1917_tsan:arm64+0x100000904)

  Thread T1 (tid=474440, running) created by main thread at:
    #0 pthread_create <null> (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x33834)
    #1 darwin_first_time_init darwin_usb.c:969 (libusb-1.0.0.dylib:arm64+0x197f4)
    #2 darwin_init_context darwin_usb.c:996 (libusb-1.0.0.dylib:arm64+0x195e0)
    #3 darwin_init darwin_usb.c:1008 (libusb-1.0.0.dylib:arm64+0x1711c)
    #4 libusb_init_context core.c:2700 (libusb-1.0.0.dylib:arm64+0x60c8)
    #5 libusb_init core.c:2580 (libusb-1.0.0.dylib:arm64+0x5a00)
    #6 main issue1798_point12_reenum_parent.c:190 (issue1798_point12_reenum_parent_pr1917_tsan:arm64+0x100000904)

  Thread T9 (tid=474448, running) created by main thread at:
    #0 pthread_create <null> (libclang_rt.tsan_osx_dynamic.dylib:arm64e+0x33834)
    #1 main issue1798_point12_reenum_parent.c:203 (issue1798_point12_reenum_parent_pr1917_tsan:arm64+0x100000a64)

SUMMARY: ThreadSanitizer: data race darwin_usb.c:1799 in process_new_device
==================
point12: done. walks ok=32484210 fail=0, desc_reads=32484210, resets ok=85 fail=0
point12: a crash/UAF or TSan report during the run is the finding; clean completion does not rule out the race (needs an actual re-enumeration to fire mid-walk).
ThreadSanitizer: reported 1 warnings
zsh: abort      ./issue1798_point12_reenum_parent_pr1917_tsan 04b4 8613

@mcuee

mcuee commented Aug 10, 2026

Copy link
Copy Markdown
Member

@sonatique

Test setup -- two USB devices (04b4:8613 and 16c0:05dc) behind the external USB 3.2 Type C multifunction hub (with USB Ethernet adapter and USB Storage device built-in)

mcuee in ~/build/libusb/libusb_test/issue1798_point12_pr1917 % lsusb_macos 
Bus 000 Device 001: ID 05ac:8015 Apple USB2.0 Hub  Serial: Not Provided
Bus 000 Device 003: ID 05e3:0610 GenesysLogic USB2.1 Hub  Serial: Not Provided
Bus 000 Device 006: ID 16c0:05dc www.fischl.de USBasp  Serial: 0001
Bus 000 Device 008: ID 04b4:8613  Unnamed Device  Serial: Not Provided
Bus 000 Device 002: ID 05e3:0625 GenesysLogic USB3.2 Hub  Serial: Not Provided
Bus 000 Device 004: ID 0bda:8156 Realtek USB 10/100/1G/2.5G LAN  Serial: 4013000001
Bus 000 Device 005: ID 05e3:0747 Generic USB Storage  Serial: 000000000819
Bus 000 Device 001: ID 05ac:8007 Apple Inc. XHCI Root Hub USB 2.0 Simulation 

mcuee in ~/build/libusb/libusb_test/issue1798_point12_pr1917 % lsusb_macos -t
/:  Bus 000.Dev 001: XHCI Root Hub USB 2.0 Simulation, 5000M
    |__ Bus 000.Dev 001: USB2.0 Hub, 480Mb/s
        |__ Bus 000.Dev 003: USB2.1 Hub, 480Mb/s
            |__ Bus 000.Dev 006: USBasp, 1.5Mb/s
            |__ Bus 000.Dev 008: Unnamed Device, 480Mb/s
    |__ Bus 000.Dev 002: USB3.2 Hub, 10Gb/s
        |__ Bus 000.Dev 004: USB 10/100/1G/2.5G LAN, 5Gb/s
        |__ Bus 000.Dev 005: USB Storage, 5Gb/s

@mcuee

mcuee commented Aug 10, 2026

Copy link
Copy Markdown
Member

@sonatique

Using hotplugtest I cannot see the difference.

mcuee in ~/build/libusb/libusb_pr1917/build_tsan on darwin-parent-device-reenumeration % ./examples/hotplugtest 
Use LIBUSB_HOTPLUG_ENUMERATE for already-connected devices? [Y/n]: n
Monitoring hotplug events. Press q or Q to quit, or Ctrl-C.

Device detached: 04b4:8613 (bus 0, device 8) path: 1.2.2
    speed = 480M
[arrived=0 departed=1 delta=-1]

Device attached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=1 departed=1 delta=0]

Device detached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=1 departed=2 delta=-1]

Device attached: 04b4:8613 (bus 0, device 8) path: 1.2.2
    speed = 480M
[arrived=2 departed=2 delta=0]

Device detached: 04b4:8613 (bus 0, device 8) path: 1.2.2
    speed = 480M
[arrived=2 departed=3 delta=-1]

Device attached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=3 departed=3 delta=0]

Device detached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=3 departed=4 delta=-1]

Device attached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=4 departed=4 delta=0]

Device detached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=4 departed=5 delta=-1]

Device attached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=5 departed=5 delta=0]

Device detached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=5 departed=6 delta=-1]

Device attached: 04b4:8613 (bus 0, device 8) path: 1.2.2
    speed = 480M
[arrived=6 departed=6 delta=0]

mcuee in ~/build/libusb/libusb/build_git_tsan on master % ./examples/hotplugtest 
Use LIBUSB_HOTPLUG_ENUMERATE for already-connected devices? [Y/n]: n
Monitoring hotplug events. Press q or Q to quit, or Ctrl-C.

Device detached: 04b4:8613 (bus 0, device 8) path: 1.2.2
    speed = 480M
[arrived=0 departed=1 delta=-1]

Device attached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=1 departed=1 delta=0]

Device detached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=1 departed=2 delta=-1]

Device attached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=2 departed=2 delta=0]

Device detached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=2 departed=3 delta=-1]

Device attached: 04b4:8613 (bus 0, device 8) path: 1.2.2
    speed = 480M
[arrived=3 departed=3 delta=0]

Device detached: 04b4:8613 (bus 0, device 8) path: 1.2.2
    speed = 480M
[arrived=3 departed=4 delta=-1]

Device attached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=4 departed=4 delta=0]

Device detached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=4 departed=5 delta=-1]

Device attached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=5 departed=5 delta=0]

Device detached: 04b4:8613 (bus 0, device 7) path: 1.2.2
    speed = 480M
[arrived=5 departed=6 delta=-1]

Device attached: 04b4:8613 (bus 0, device 8) path: 1.2.2
    speed = 480M
[arrived=6 departed=6 delta=0]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants