[Libusbx-devel] [PATCH 2/2] linux_usbfs: Work around a driver binding race in reset handling
I've been seeing these intermittent failures to reclaim an interface after
a device reset. After much debugging and inserting sleeps in strategic
places to make the race window larger I've found the following race:
1) A user is running some software using libusbx which will automatically
detect, and "bind" to, any newly plugged in USB-devices. For example
a virtual machine viewer with automatic USB-redirection
2) The user plugs in a new usb-storage device
3) The usb-storage driver is not yet loaded, udev spawns
"modprobe usb-storage", this blocks on disk-io
4) The libusbx app opens the device, claims all interfaces, does a device-reset
5) While the IOCTL_USBFS_RESET is running the modprobe completes
6) The driver registration blocks on an USB lock held by the reset code path
7) When the reset finishes the driver registration completes and the driver
binds itself to the device, before IOCTL_USBFS_RESET returns to userspace
8) libusbx tries to re-claim all interfaces it had claimed before the reset
9) libusbx fails as usb-storage is now bound to it
This patch works around this issue by simply unbinding the driver for all
interfaces which were claimed before the reset. Normally this is a no-op as
no driver (other then usbfs) can be bound for claimed interfaces before the
reset.
This patch also improves the error logging, and makes libusb_device_reset
properly return an error when re-claiming fails.
Signed-off-by: Hans de Goede
---
libusb/os/linux_usbfs.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index a737b2d..898af1c 100644
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -108,6 +108,10 @@ static int sysfs_can_relate_devices = 0;
/* do we have a descriptors file? */
static int sysfs_has_descriptors = 0;
+static int op_detach_kernel_driver_and_claim(
+ struct libusb_device_handle *handle, int interface,
+ const char *driver_name);
+
struct linux_device_priv {
char *sysfs_dir;
unsigned char *dev_descriptor;
@@ -1497,11 +1501,18 @@ static int op_reset_device(struct libusb_device_handle
*handle)
/* And re-claim any interfaces which were claimed before the reset */
for (i = 0; i < USB_MAXINTERFACES; i++) {
if (handle->claimed_interfaces & (1L << i)) {
- r = op_claim_interface(handle, i);
+ /*
+* A driver may have completed modprobing during
+* IOCTL_USBFS_RESET, and bound itself as soon as
+* IOCTL_USBFS_RESET released the device lock
+*/
+ r = op_detach_kernel_driver_and_claim(handle, i, NULL);
if (r) {
usbi_warn(HANDLE_CTX(handle),
- "failed to re-claim interface %d after
reset", i);
+ "failed to re-claim interface %d after
reset: %s",
+ i, libusb_error_name(r));
handle->claimed_interfaces &= ~(1L << i);
+ ret = LIBUSB_ERROR_NOT_FOUND;
}
}
}
--
1.7.12
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
[Libusbx-devel] [PATCH 0/2 for v1.0.13]: Add libusb_detach_kernel_driver_and_claim()
Is here a repost of my RFC from a few weeks ago, the Linux kernel part of this has just been reviewed and acked by Alan Stern, so it looks like we are good to go with this. Note this set does not yet hook up the new kernel ioctl, I would like to wait with that until it hits Linus' tree, as until then its API could still change (I don't expect that, but it could), for the actual hookup patch see the RFC patch-set. I know it is a bit late, but if possible I would like to get this in for v1.0.13, this would close issue 17, at least from our API pov (it will only actually be fixed when using a kernel with the new ioctl). And it will fix the driver binding race on reset I've been hitting occasionally. Regards, Hans -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
[Libusbx-devel] [PATCH 1/2] Add a new libusb_detach_kernel_driver_and_claim() function
Apps which deal with devices which also have a kernel driver, need to do
the following:
1) Check which driver is attached, so as to not detach the wrong driver
(ie detaching usbfs while another instance of the app is using the device)
2) Detach the kernel driver
3) Claim the interface
Where moving from one step to the next for both 1-2 and 2-3 consists of
a (small) race window. So currently such apps are racy and people just live
with it.
To fix this issue, a new ioctl is being added to the Linux kernel which does
steps 1-3 in one race-free call.
This patch adds a new libusbx function for exporting this functionality, note
that currently it just implements it using the old racy ioctls. A follow up
patch will add use of the new ioctl (where available).
Signed-off-by: Hans de Goede
---
libusb/core.c | 38 ++
libusb/libusb.h | 2 ++
libusb/libusbi.h| 16
libusb/os/linux_usbfs.c | 23 +++
4 files changed, 79 insertions(+)
diff --git a/libusb/core.c b/libusb/core.c
index 5f2cd34..9e044b5 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -1567,6 +1567,44 @@ int API_EXPORTED
libusb_attach_kernel_driver(libusb_device_handle *dev,
return LIBUSB_ERROR_NOT_SUPPORTED;
}
+/** \ingroup dev
+ * Detach the named kernel driver from an interface and claim it,
+ * in one atomic operation (if possible, fallback to non atomic
+ * where not). This call is only effective on Linux and returns
+ * LIBUSB_ERROR_NOT_SUPPORTED on all other platforms.
+ *
+ * If driver_name is NULL, any attached driver will be detached, if
+ * driver_name is not NULL, and another driver then named is attached,
+ * the call will fail with a LIBUSB_ERROR_BUSY return.
+ *
+ * This functionality is not available on Darwin, Windows or BSD.
+ *
+ * \param dev a device handle
+ * \param interface_number the interface to detach the driver from
+ * \param driver_name name of the driver to detach, or NULL to detach any
+ * \returns 0 on success
+ * \returns LIBUSB_ERROR_BUSY if the interface is busy
+ * \returns LIBUSB_ERROR_INVALID_PARAM if the interface does not exist
+ * \returns LIBUSB_ERROR_NO_DEVICE if the device has been disconnected
+ * \returns LIBUSB_ERROR_NOT_SUPPORTED on platforms where the functionality
+ * is not available
+ * \returns another LIBUSB_ERROR code on other failure
+ * \see libusb_detach_kernel_driver()
+ * \see libusb_claim_interface()
+ */
+int API_EXPORTED libusb_detach_kernel_driver_and_claim(
+ libusb_device_handle *dev, int interface_number,
+ const char *driver_name)
+{
+ usbi_dbg("interface %d, driver_name %s", interface_number,
+driver_name ? driver_name : "NULL");
+ if (usbi_backend->detach_kernel_driver_and_claim)
+ return usbi_backend->detach_kernel_driver_and_claim(
+ dev, interface_number, driver_name);
+ else
+ return LIBUSB_ERROR_NOT_SUPPORTED;
+}
+
/** \ingroup lib
* Set log message verbosity.
*
diff --git a/libusb/libusb.h b/libusb/libusb.h
index de31a56..3fdf6dd 100644
--- a/libusb/libusb.h
+++ b/libusb/libusb.h
@@ -1032,6 +1032,8 @@ int LIBUSB_CALL
libusb_detach_kernel_driver(libusb_device_handle *dev,
int interface_number);
int LIBUSB_CALL libusb_attach_kernel_driver(libusb_device_handle *dev,
int interface_number);
+int LIBUSB_CALL libusb_detach_kernel_driver_and_claim(
+ libusb_device_handle *dev, int interface_number, const char *driver);
/* async I/O */
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index 5ec0761..d195a36 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -792,6 +792,22 @@ struct usbi_os_backend {
int (*attach_kernel_driver)(struct libusb_device_handle *handle,
int interface_number);
+ /* Detach the named kernel driver from an interface, then claim it,
+* in one atomic operation (if possible, fallback to non atomic
+* where not). Optional.
+*
+* Return:
+* - 0 on success
+* - LIBUSB_ERROR_BUSY if the interface is busy
+* - LIBUSB_ERROR_INVALID_PARAM if the interface does not exist
+* - LIBUSB_ERROR_NO_DEVICE if the device has been disconnected since
+* it was opened
+* - another LIBUSB_ERROR code on other failure
+*/
+ int (*detach_kernel_driver_and_claim)(
+ struct libusb_device_handle *handle, int interface_number,
+ const char *driver_name);
+
/* Destroy a device. Optional.
*
* This function is called when the last reference to a device is
diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index 5093745..a737b2d 100644
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -1599,6 +1599,28 @@ static int op_attach_kernel_driver(struct
libusb_device_handle *handle,
return 0;
}
+static int op_detach_kernel_driver_
Re: [Libusbx-devel] libusb segfaults - causes pcscd to crash
On 06.09.2012 00:45, Pete Batard wrote: > We haven't seen crash reports from you since we pushed the fixes that > (we hoped) could address the issue. Can you confirm that you've been > happily running pcscd ever since? Hello Pete, sorry for my delayed response. I have been out of office for a week. The issue seems to be fixed by your patches. Thanks! I'll contact you, if the segmentation fault occurs again or any other problems arise. = Regards Sebastian = -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
Re: [Libusbx-devel] Libusb++ is available.
On Wed, Sep 5, 2012 at 9:22 PM, Ludovic Rousseau wrote: > Also the LibusbTest.cpp do not compile on Mac OS X. Bug filed in github. I installed gcc-4.7 (enable c++) using Homebrew (dupes gcc formula) and now it complains about conio.h, bug commented in github. https://github.com/zarthcode/Libusbpp/issues/2 -- Xiaofan -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
Re: [Libusbx-devel] [PATCH 0/2 for v1.0.13]: Add libusb_detach_kernel_driver_and_claim()
On 2012.09.10 08:56, Hans de Goede wrote:
> Note this set does not yet hook up the new kernel ioctl, I would like to
> wait with that until it hits Linus' tree
and
> I know it is a bit late, but if possible I would like to get this in for
> v1.0.13
So this is really 1/3 and 2/3, with 3/3 ("A follow up patch will add use
of the new ioctl") to be produced later, right?
I don't have an issue adding these 2 for 1.0.13 if others want it, but
somehow I feel this is exactly the kind of OS specific calls we want to
avoid adding an extra public API for (since, if the kernel ever
implements something that obsoletes it, we'll have to publicly carry
that call forever). Instead, I see it better suited into a new single &
generic libusb_perform_os_specific_op() or something, that would take OS
specific actions/parameters, but through an API generic that all OSes
can have some use for. I.e. something along the lines of what I proposed
previously (though thinking about it some more, I think an union of
single op's would be better suited than a struct of all of them).
Do we expect any other platforms, besides Linux to ever need
libusb_detach_kernel_driver_and_claim()?
Granted, we already have libusb_detach_kernel_driver(), so yeah, it's a
special case, but from the comments, the call looks awfully OS specific
to me, and therefore something that doesn't seem extraordinarily well
suited to introduce in an API that is intended to be as cross-platform
as possible.
Thus, I would prefer making it less conspicuous, and at the same time
try to sort a once and for all approach for OS specific calls.
I can even use that proposal to elaborate further what I have in mind,
but that's not gonna happen in time for 1.0.13...
Regards,
/Pete
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
Re: [Libusbx-devel] libusb-win32 and libusbK support has now been pushed to mainline
On 2012.09.10 06:14, Xiaofan Chen wrote: > On the other hand, if we can not fix the issue of the > filter driver with regard to USB composite device, I > think it is still okay to release 1.0.13, just need to > put this limitation in the release notes. That was the plan, as I'd like to go RC tomorrow, and I'm not gonna have a chance to look into it before then. Regards, /Pete -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
[Libusbx-devel] [PATCH] Doc: Update logging documentation regarding stderr usage
As mentioned when we discussed the pcscd issue.
The existing doc mentions that stderr/stdout can be closed without
worry, which is not true unless libusbx is compiled with --disable-log
(nondefault).
Regards,
/Pete
>From c0e21eba6190c246dc7b1fddcbd07393f6ceb5c2 Mon Sep 17 00:00:00 2001
From: Pete Batard
Date: Tue, 11 Sep 2012 00:17:39 +0100
Subject: [PATCH] Doc: Update logging documentation regarding stderr usage
---
libusb/core.c | 29 -
1 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/libusb/core.c b/libusb/core.c
index 5f2cd34..3964e3d 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -106,15 +106,18 @@ static struct timeval timestamp_origin = { 0, 0 };
*
* \section msglog Debug message logging
*
- * libusbx does not log any messages by default. Your application is therefore
- * free to close stdout/stderr and those descriptors may be reused without
- * worry.
- *
- * The libusb_set_debug() function can be used to enable stderr logging of
- * certain messages. Under standard configuration, libusbx doesn't really
- * log much at all, so you are advised to use this function to enable all
- * error/warning/informational messages. It will help you debug problems with
- * your software.
+ * libusbx uses stderr for all logging. By default, logging is set to NONE,
+ * which means that no output will be produced. However, unless the library
+ * has been compiled with logging disabled, then any application calls to
+ * libusb_set_debug(), or the setting of the environmental variable
+ * LIBUSB_DEBUG outside of the application, can result in logging being
+ * produced. Your application should therefore not close stderr, but instead
+ * direct it to the null device if its output is undesireable.
+ *
+ * The libusb_set_debug() function can be used to enable logging of certain
+ * messages. Under standard configuration, libusbx doesn't really log much
+ * so you are advised to use this function to enable all error/warning/
+ * informational messages. It will help debug problems with your software.
*
* The logged messages are unstructured. There is no one-to-one correspondence
* between messages being logged and success or failure return codes from
@@ -137,10 +140,10 @@ static struct timeval timestamp_origin = { 0, 0 };
* systems. In this case, libusb_set_debug() and the LIBUSB_DEBUG environment
* variable have no effects.
*
- * libusbx can also be compiled with verbose debugging messages. When the
- * library is compiled in this way, all messages of all verbosities are always
- * logged. libusb_set_debug() and the LIBUSB_DEBUG environment variable have
- * no effects.
+ * libusbx can also be compiled with verbose debugging messages always. When
+ * the library is compiled in this way, all messages of all verbosities are
+ * always logged. libusb_set_debug() and the LIBUSB_DEBUG environment variable
+ * have no effects.
*
* \section remarks Other remarks
*
--
1.7.11.msysgit.0
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
