From 378cca96bd54eb3e369d50470b641b6b9c7419c4 Mon Sep 17 00:00:00 2001 From: Yaakov Selkowitz Date: Tue, 5 Aug 2025 10:18:38 -0400 Subject: [PATCH 1/3] Always regenerate autotools Since we're modifying a Makefile.am, the corresponding Makefile.in will have to be regenerated, and we should never rely on any release having matching autotools versions. --- libusb1.spec | 2 -- 1 file changed, 2 deletions(-) diff --git a/libusb1.spec b/libusb1.spec index 6c2f458..256a12d 100644 --- a/libusb1.spec +++ b/libusb1.spec @@ -94,12 +94,10 @@ MinGW Windows %{name} library. chmod -x examples/*.c mkdir -p m4 sed -i '/AM_LDFLAGS = -static/d' tests/Makefile.am -%if (0%{?fedora} && 0%{?fedora} < 44) || (0%{?rhel} && 0%{?rhel} < 11) autoscan aclocal autoconf automake --add-missing -%endif %build From 3c2e5bfcc55851d96c3beae4a1c3c0d01ea02a86 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Mon, 11 Aug 2025 18:21:04 +0200 Subject: [PATCH 2/3] Add patches to skip uninitialized devices Otherwise there is a race condition if libusb enumerates the devices while udev is processing the event. Resolves: #2346771 --- ...v-remove-UMOCKDEV_HOTPLUG-workaround.patch | 77 +++++++++++ ...return-initialized-devices-during-en.patch | 120 ++++++++++++++++++ libusb1.spec | 4 + 3 files changed, 201 insertions(+) create mode 100644 0001-tests-umockdev-remove-UMOCKDEV_HOTPLUG-workaround.patch create mode 100644 0002-linux_udev-only-return-initialized-devices-during-en.patch diff --git a/0001-tests-umockdev-remove-UMOCKDEV_HOTPLUG-workaround.patch b/0001-tests-umockdev-remove-UMOCKDEV_HOTPLUG-workaround.patch new file mode 100644 index 0000000..2bbe1bd --- /dev/null +++ b/0001-tests-umockdev-remove-UMOCKDEV_HOTPLUG-workaround.patch @@ -0,0 +1,77 @@ +From 76bbaf23ac3f1c91c93e82a6c0947c66fb246edc Mon Sep 17 00:00:00 2001 +From: Benjamin Berg +Date: Thu, 20 Mar 2025 22:38:39 +0100 +Subject: [PATCH 1/2] tests/umockdev: remove UMOCKDEV_HOTPLUG workaround + +umockdev 0.17.7 is old enough to rely on it for testing. Remove the +workaround as it might not be compatible with the follow up changes. + +Signed-off-by: Benjamin Berg +--- + configure.ac | 8 ++------ + tests/umockdev.c | 7 ------- + 2 files changed, 2 insertions(+), 13 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 9b519234..abdbf0de 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -209,14 +209,10 @@ linux) + AC_CHECK_HEADER([libudev.h], [], [AC_MSG_ERROR([udev support requested but libudev header not installed])]) + AC_CHECK_LIB([udev], [udev_new], [], [AC_MSG_ERROR([udev support requested but libudev not installed])]) + +- # We can build umockdev tests (if available) ++ # We can build umockdev tests (if available); need 0.17.7 because hotplug was racy before + m4_ifdef([PKG_PROG_PKG_CONFIG],[ + PKG_PROG_PKG_CONFIG +- PKG_CHECK_MODULES([UMOCKDEV], [umockdev-1.0 >= 0.16.0], [ac_have_umockdev=yes], [ac_have_umockdev=no]) +- PKG_CHECK_MODULES([UMOCKDEV_HOTPLUG], [umockdev-1.0 >= 0.17.7], [ac_umockdev_hotplug=yes], [ac_umockdev_hotplug=no]) +- if test $ac_umockdev_hotplug = yes; then +- AC_DEFINE([UMOCKDEV_HOTPLUG], [1], [UMockdev hotplug code is not racy]) +- fi ++ PKG_CHECK_MODULES([UMOCKDEV], [umockdev-1.0 >= 0.17.7], [ac_have_umockdev=yes], [ac_have_umockdev=no]) + ], []) + else + AC_CHECK_HEADERS([asm/types.h]) +diff --git a/tests/umockdev.c b/tests/umockdev.c +index a2d457ee..f979ed8f 100644 +--- a/tests/umockdev.c ++++ b/tests/umockdev.c +@@ -971,7 +971,6 @@ hotplug_count_arrival_cb(libusb_context *ctx, + return 0; + } + +-#ifdef UMOCKDEV_HOTPLUG + static int LIBUSB_CALL + hotplug_count_removal_cb(libusb_context *ctx, + libusb_device *device, +@@ -987,7 +986,6 @@ hotplug_count_removal_cb(libusb_context *ctx, + + return 0; + } +-#endif + + static void + test_hotplug_enumerate(UMockdevTestbedFixture * fixture, UNUSED_DATA) +@@ -1041,7 +1039,6 @@ test_hotplug_enumerate(UMockdevTestbedFixture * fixture, UNUSED_DATA) + static void + test_hotplug_add_remove(UMockdevTestbedFixture * fixture, UNUSED_DATA) + { +-#ifdef UMOCKDEV_HOTPLUG + libusb_device **devs = NULL; + libusb_hotplug_callback_handle handle_add; + libusb_hotplug_callback_handle handle_remove; +@@ -1111,10 +1108,6 @@ test_hotplug_add_remove(UMockdevTestbedFixture * fixture, UNUSED_DATA) + + libusb_hotplug_deregister_callback(fixture->ctx, handle_add); + libusb_hotplug_deregister_callback(fixture->ctx, handle_remove); +-#else +- (void) fixture; +- g_test_skip("UMockdev is too old to test hotplug"); +-#endif + } + + int +-- +2.50.1 + diff --git a/0002-linux_udev-only-return-initialized-devices-during-en.patch b/0002-linux_udev-only-return-initialized-devices-during-en.patch new file mode 100644 index 0000000..fbc91c1 --- /dev/null +++ b/0002-linux_udev-only-return-initialized-devices-during-en.patch @@ -0,0 +1,120 @@ +From 04f1e292af11aa9b5faef4338d36716396beff1a Mon Sep 17 00:00:00 2001 +From: Benjamin Berg +Date: Thu, 20 Mar 2025 22:49:52 +0100 +Subject: [PATCH 2/2] linux_udev: only return initialized devices during + enumeration + +The udev enumerator can return devices for which udev has not finished +processing its rules. As udev has not finished handling its rules, the +application may not yet be permitted to open the device. + +For these devices a hotplug event will happen later on when udev sends +the "add" event. + +Change the enumerator to only return initialized devices when udev is +running. There is no nice API to check whether this is the case, so just +do the same access() call that udev_queue_get_udev_is_active() would +also do. + +Also add a umockdev test for this specific corner case. Note that other +tests need to be updated to work around poor umockdev emulation of the +udev behaviour. umockdev never writes udev database entries to its +/run/udev/data, which means that devices are only considered initialized +when an event has been received. + +Signed-off-by: Benjamin Berg +--- + libusb/os/linux_udev.c | 5 +++++ + tests/umockdev.c | 25 +++++++++++++++++++++++-- + 2 files changed, 28 insertions(+), 2 deletions(-) + +diff --git a/libusb/os/linux_udev.c b/libusb/os/linux_udev.c +index 9ec9eb17..fad4f2ca 100644 +--- a/libusb/os/linux_udev.c ++++ b/libusb/os/linux_udev.c +@@ -278,6 +278,11 @@ int linux_udev_scan_devices(struct libusb_context *ctx) + return LIBUSB_ERROR_OTHER; + } + ++ /* Only return initialized devices if udev is running. If it is not ++ * running, devices will never be marked as "initialized". ++ */ ++ if (access("/run/udev/control", F_OK) == 0) ++ udev_enumerate_add_match_is_initialized(enumerator); + udev_enumerate_add_match_subsystem(enumerator, "usb"); + udev_enumerate_add_match_property(enumerator, "DEVTYPE", "usb_device"); + udev_enumerate_scan_devices(enumerator); +diff --git a/tests/umockdev.c b/tests/umockdev.c +index f979ed8f..f09611f1 100644 +--- a/tests/umockdev.c ++++ b/tests/umockdev.c +@@ -409,6 +409,8 @@ test_fixture_add_canon(UMockdevTestbedFixture * fixture) + "E: DRIVER=usb\n" + "E: BUSNUM=001\n" + "E: DEVNUM=001\n" ++ "E: MAJOR=189\n" ++ "E: MINOR=128\n" + "E: DEVNAME=/dev/bus/usb/001/001\n" + "E: DEVTYPE=usb_device\n" + "A: bConfigurationValue=1\\n\n" +@@ -429,8 +431,7 @@ static void + test_fixture_setup_libusb(UMockdevTestbedFixture * fixture, int devcount) + { + libusb_device **devs = NULL; +- +- libusb_init_context(/*ctx=*/&fixture->ctx, /*options=*/NULL, /*num_options=*/0); ++ struct timeval zero_tv = { 0 }; + + /* Suppress global log messages completely + * (though, in some tests it might be interesting to check there are no real ones). +@@ -440,6 +441,11 @@ test_fixture_setup_libusb(UMockdevTestbedFixture * fixture, int devcount) + g_assert_cmpint(libusb_get_device_list(fixture->ctx, &devs), ==, devcount); + libusb_free_device_list(devs, TRUE); + libusb_set_log_cb (fixture->ctx, log_handler, LIBUSB_LOG_CB_CONTEXT); ++ ++ /* Get the udev "add" event out of the way, part of the workaround that ++ * umockdev does no create udev DB files. ++ */ ++ libusb_handle_events_timeout(fixture->ctx, &zero_tv); + } + + static void +@@ -464,6 +470,8 @@ test_fixture_setup_empty(UMockdevTestbedFixture * fixture, UNUSED_DATA) + { + test_fixture_setup_common(fixture); + ++ libusb_init_context(/*ctx=*/&fixture->ctx, /*options=*/NULL, /*num_options=*/0); ++ + test_fixture_setup_libusb(fixture, 0); + } + +@@ -472,6 +480,14 @@ test_fixture_setup_with_canon(UMockdevTestbedFixture * fixture, UNUSED_DATA) + { + test_fixture_setup_common(fixture); + ++ /* As of umockev 0.19.1 (and probably later) no entries in ++ * /run/udev/data are created. i.e. udev does not have a database, ++ * which means libudev will only consider devices as "initialized" if ++ * it received the uevent and does not need to read the database file. ++ * Creating the context first means the uevent will be received. ++ */ ++ libusb_init_context(/*ctx=*/&fixture->ctx, /*options=*/NULL, /*num_options=*/0); ++ + test_fixture_add_canon(fixture); + + test_fixture_setup_libusb(fixture, 1); +@@ -574,6 +590,11 @@ test_implicit_default(UMockdevTestbedFixture * fixture, UNUSED_DATA) + clear_libusb_log(fixture, LIBUSB_LOG_LEVEL_INFO); + + libusb_init_context(/*ctx=*/NULL, /*options=*/NULL, /*num_options=*/0); ++ /* Work around the missing udev DB in umockdev by sending a "add" ++ * notification. It is fine if this umockdev quirk changes in the ++ * future. ++ */ ++ umockdev_testbed_uevent(fixture->testbed, "/sys/devices/usb1", "add"); + g_assert_cmpint(libusb_get_device_list(NULL, &devs), ==, 1); + libusb_exit(NULL); + +-- +2.50.1 + diff --git a/libusb1.spec b/libusb1.spec index 256a12d..b934c1f 100644 --- a/libusb1.spec +++ b/libusb1.spec @@ -18,6 +18,10 @@ BuildRequires: gnupg2 Provides: libusbx = %{version}-%{release} Obsoletes: libusbx < %{version}-%{release} +Patch01: 0001-tests-umockdev-remove-UMOCKDEV_HOTPLUG-workaround.patch +Patch02: 0002-linux_udev-only-return-initialized-devices-during-en.patch + + %if %{with mingw} BuildRequires: mingw32-filesystem >= 95 BuildRequires: mingw32-gcc-c++ From 4a3bd6f9ecaa4e74bda93b1ad449fd51f00adf7b Mon Sep 17 00:00:00 2001 From: Kate Hsuan Date: Fri, 29 Aug 2025 10:43:28 +0800 Subject: [PATCH 3/3] Revert "Add patches to skip uninitialized devices" (rhbz#2390535) The fix brought a regression to mess storage so revert the fix and then trying to find the better fixes. This reverts commit 3c2e5bfcc55851d96c3beae4a1c3c0d01ea02a86. --- ...v-remove-UMOCKDEV_HOTPLUG-workaround.patch | 77 ----------- ...return-initialized-devices-during-en.patch | 120 ------------------ libusb1.spec | 4 - 3 files changed, 201 deletions(-) delete mode 100644 0001-tests-umockdev-remove-UMOCKDEV_HOTPLUG-workaround.patch delete mode 100644 0002-linux_udev-only-return-initialized-devices-during-en.patch diff --git a/0001-tests-umockdev-remove-UMOCKDEV_HOTPLUG-workaround.patch b/0001-tests-umockdev-remove-UMOCKDEV_HOTPLUG-workaround.patch deleted file mode 100644 index 2bbe1bd..0000000 --- a/0001-tests-umockdev-remove-UMOCKDEV_HOTPLUG-workaround.patch +++ /dev/null @@ -1,77 +0,0 @@ -From 76bbaf23ac3f1c91c93e82a6c0947c66fb246edc Mon Sep 17 00:00:00 2001 -From: Benjamin Berg -Date: Thu, 20 Mar 2025 22:38:39 +0100 -Subject: [PATCH 1/2] tests/umockdev: remove UMOCKDEV_HOTPLUG workaround - -umockdev 0.17.7 is old enough to rely on it for testing. Remove the -workaround as it might not be compatible with the follow up changes. - -Signed-off-by: Benjamin Berg ---- - configure.ac | 8 ++------ - tests/umockdev.c | 7 ------- - 2 files changed, 2 insertions(+), 13 deletions(-) - -diff --git a/configure.ac b/configure.ac -index 9b519234..abdbf0de 100644 ---- a/configure.ac -+++ b/configure.ac -@@ -209,14 +209,10 @@ linux) - AC_CHECK_HEADER([libudev.h], [], [AC_MSG_ERROR([udev support requested but libudev header not installed])]) - AC_CHECK_LIB([udev], [udev_new], [], [AC_MSG_ERROR([udev support requested but libudev not installed])]) - -- # We can build umockdev tests (if available) -+ # We can build umockdev tests (if available); need 0.17.7 because hotplug was racy before - m4_ifdef([PKG_PROG_PKG_CONFIG],[ - PKG_PROG_PKG_CONFIG -- PKG_CHECK_MODULES([UMOCKDEV], [umockdev-1.0 >= 0.16.0], [ac_have_umockdev=yes], [ac_have_umockdev=no]) -- PKG_CHECK_MODULES([UMOCKDEV_HOTPLUG], [umockdev-1.0 >= 0.17.7], [ac_umockdev_hotplug=yes], [ac_umockdev_hotplug=no]) -- if test $ac_umockdev_hotplug = yes; then -- AC_DEFINE([UMOCKDEV_HOTPLUG], [1], [UMockdev hotplug code is not racy]) -- fi -+ PKG_CHECK_MODULES([UMOCKDEV], [umockdev-1.0 >= 0.17.7], [ac_have_umockdev=yes], [ac_have_umockdev=no]) - ], []) - else - AC_CHECK_HEADERS([asm/types.h]) -diff --git a/tests/umockdev.c b/tests/umockdev.c -index a2d457ee..f979ed8f 100644 ---- a/tests/umockdev.c -+++ b/tests/umockdev.c -@@ -971,7 +971,6 @@ hotplug_count_arrival_cb(libusb_context *ctx, - return 0; - } - --#ifdef UMOCKDEV_HOTPLUG - static int LIBUSB_CALL - hotplug_count_removal_cb(libusb_context *ctx, - libusb_device *device, -@@ -987,7 +986,6 @@ hotplug_count_removal_cb(libusb_context *ctx, - - return 0; - } --#endif - - static void - test_hotplug_enumerate(UMockdevTestbedFixture * fixture, UNUSED_DATA) -@@ -1041,7 +1039,6 @@ test_hotplug_enumerate(UMockdevTestbedFixture * fixture, UNUSED_DATA) - static void - test_hotplug_add_remove(UMockdevTestbedFixture * fixture, UNUSED_DATA) - { --#ifdef UMOCKDEV_HOTPLUG - libusb_device **devs = NULL; - libusb_hotplug_callback_handle handle_add; - libusb_hotplug_callback_handle handle_remove; -@@ -1111,10 +1108,6 @@ test_hotplug_add_remove(UMockdevTestbedFixture * fixture, UNUSED_DATA) - - libusb_hotplug_deregister_callback(fixture->ctx, handle_add); - libusb_hotplug_deregister_callback(fixture->ctx, handle_remove); --#else -- (void) fixture; -- g_test_skip("UMockdev is too old to test hotplug"); --#endif - } - - int --- -2.50.1 - diff --git a/0002-linux_udev-only-return-initialized-devices-during-en.patch b/0002-linux_udev-only-return-initialized-devices-during-en.patch deleted file mode 100644 index fbc91c1..0000000 --- a/0002-linux_udev-only-return-initialized-devices-during-en.patch +++ /dev/null @@ -1,120 +0,0 @@ -From 04f1e292af11aa9b5faef4338d36716396beff1a Mon Sep 17 00:00:00 2001 -From: Benjamin Berg -Date: Thu, 20 Mar 2025 22:49:52 +0100 -Subject: [PATCH 2/2] linux_udev: only return initialized devices during - enumeration - -The udev enumerator can return devices for which udev has not finished -processing its rules. As udev has not finished handling its rules, the -application may not yet be permitted to open the device. - -For these devices a hotplug event will happen later on when udev sends -the "add" event. - -Change the enumerator to only return initialized devices when udev is -running. There is no nice API to check whether this is the case, so just -do the same access() call that udev_queue_get_udev_is_active() would -also do. - -Also add a umockdev test for this specific corner case. Note that other -tests need to be updated to work around poor umockdev emulation of the -udev behaviour. umockdev never writes udev database entries to its -/run/udev/data, which means that devices are only considered initialized -when an event has been received. - -Signed-off-by: Benjamin Berg ---- - libusb/os/linux_udev.c | 5 +++++ - tests/umockdev.c | 25 +++++++++++++++++++++++-- - 2 files changed, 28 insertions(+), 2 deletions(-) - -diff --git a/libusb/os/linux_udev.c b/libusb/os/linux_udev.c -index 9ec9eb17..fad4f2ca 100644 ---- a/libusb/os/linux_udev.c -+++ b/libusb/os/linux_udev.c -@@ -278,6 +278,11 @@ int linux_udev_scan_devices(struct libusb_context *ctx) - return LIBUSB_ERROR_OTHER; - } - -+ /* Only return initialized devices if udev is running. If it is not -+ * running, devices will never be marked as "initialized". -+ */ -+ if (access("/run/udev/control", F_OK) == 0) -+ udev_enumerate_add_match_is_initialized(enumerator); - udev_enumerate_add_match_subsystem(enumerator, "usb"); - udev_enumerate_add_match_property(enumerator, "DEVTYPE", "usb_device"); - udev_enumerate_scan_devices(enumerator); -diff --git a/tests/umockdev.c b/tests/umockdev.c -index f979ed8f..f09611f1 100644 ---- a/tests/umockdev.c -+++ b/tests/umockdev.c -@@ -409,6 +409,8 @@ test_fixture_add_canon(UMockdevTestbedFixture * fixture) - "E: DRIVER=usb\n" - "E: BUSNUM=001\n" - "E: DEVNUM=001\n" -+ "E: MAJOR=189\n" -+ "E: MINOR=128\n" - "E: DEVNAME=/dev/bus/usb/001/001\n" - "E: DEVTYPE=usb_device\n" - "A: bConfigurationValue=1\\n\n" -@@ -429,8 +431,7 @@ static void - test_fixture_setup_libusb(UMockdevTestbedFixture * fixture, int devcount) - { - libusb_device **devs = NULL; -- -- libusb_init_context(/*ctx=*/&fixture->ctx, /*options=*/NULL, /*num_options=*/0); -+ struct timeval zero_tv = { 0 }; - - /* Suppress global log messages completely - * (though, in some tests it might be interesting to check there are no real ones). -@@ -440,6 +441,11 @@ test_fixture_setup_libusb(UMockdevTestbedFixture * fixture, int devcount) - g_assert_cmpint(libusb_get_device_list(fixture->ctx, &devs), ==, devcount); - libusb_free_device_list(devs, TRUE); - libusb_set_log_cb (fixture->ctx, log_handler, LIBUSB_LOG_CB_CONTEXT); -+ -+ /* Get the udev "add" event out of the way, part of the workaround that -+ * umockdev does no create udev DB files. -+ */ -+ libusb_handle_events_timeout(fixture->ctx, &zero_tv); - } - - static void -@@ -464,6 +470,8 @@ test_fixture_setup_empty(UMockdevTestbedFixture * fixture, UNUSED_DATA) - { - test_fixture_setup_common(fixture); - -+ libusb_init_context(/*ctx=*/&fixture->ctx, /*options=*/NULL, /*num_options=*/0); -+ - test_fixture_setup_libusb(fixture, 0); - } - -@@ -472,6 +480,14 @@ test_fixture_setup_with_canon(UMockdevTestbedFixture * fixture, UNUSED_DATA) - { - test_fixture_setup_common(fixture); - -+ /* As of umockev 0.19.1 (and probably later) no entries in -+ * /run/udev/data are created. i.e. udev does not have a database, -+ * which means libudev will only consider devices as "initialized" if -+ * it received the uevent and does not need to read the database file. -+ * Creating the context first means the uevent will be received. -+ */ -+ libusb_init_context(/*ctx=*/&fixture->ctx, /*options=*/NULL, /*num_options=*/0); -+ - test_fixture_add_canon(fixture); - - test_fixture_setup_libusb(fixture, 1); -@@ -574,6 +590,11 @@ test_implicit_default(UMockdevTestbedFixture * fixture, UNUSED_DATA) - clear_libusb_log(fixture, LIBUSB_LOG_LEVEL_INFO); - - libusb_init_context(/*ctx=*/NULL, /*options=*/NULL, /*num_options=*/0); -+ /* Work around the missing udev DB in umockdev by sending a "add" -+ * notification. It is fine if this umockdev quirk changes in the -+ * future. -+ */ -+ umockdev_testbed_uevent(fixture->testbed, "/sys/devices/usb1", "add"); - g_assert_cmpint(libusb_get_device_list(NULL, &devs), ==, 1); - libusb_exit(NULL); - --- -2.50.1 - diff --git a/libusb1.spec b/libusb1.spec index b934c1f..256a12d 100644 --- a/libusb1.spec +++ b/libusb1.spec @@ -18,10 +18,6 @@ BuildRequires: gnupg2 Provides: libusbx = %{version}-%{release} Obsoletes: libusbx < %{version}-%{release} -Patch01: 0001-tests-umockdev-remove-UMOCKDEV_HOTPLUG-workaround.patch -Patch02: 0002-linux_udev-only-return-initialized-devices-during-en.patch - - %if %{with mingw} BuildRequires: mingw32-filesystem >= 95 BuildRequires: mingw32-gcc-c++