[libmtp] Test out a patch to make device probing more careful.
Linus Walleij
snirkel at fedoraproject.org
Wed Jan 19 18:45:39 UTC 2011
commit 4f8940654495c808ff093298102dcc2f879863fc
Author: Linus Walleij <linus.walleij at stericsson.com>
Date: Wed Jan 19 19:45:03 2011 +0100
Test out a patch to make device probing more careful.
0001-Fixup-device-OS-descriptor-probe-code.patch | 266 ++++++++++++++++++++++
libmtp.spec | 7 +-
2 files changed, 272 insertions(+), 1 deletions(-)
---
diff --git a/0001-Fixup-device-OS-descriptor-probe-code.patch b/0001-Fixup-device-OS-descriptor-probe-code.patch
new file mode 100644
index 0000000..95a60d4
--- /dev/null
+++ b/0001-Fixup-device-OS-descriptor-probe-code.patch
@@ -0,0 +1,266 @@
+From 9db8a316a880caddf82227f69d7a8607fb674146 Mon Sep 17 00:00:00 2001
+From: Linus Walleij <linus.walleij at stericsson.com>
+Date: Wed, 19 Jan 2011 19:20:34 +0100
+Subject: [PATCH] Fixup device OS descriptor probe code
+
+This was to dangerous, so now we only probe:
+
+- Devices that are of known classes to conatin MTP extensions:
+ COMM, PTP, CUSTOM and per-interface.
+- Unless the device is CUSTOM, it needs to contain atleast one
+ CUSTOM interface.
+- Clear halt on EP 0 after probe if it fails with error.
+
+Signed-off-by: Linus Walleij <triad at df.lth.se>
+---
+ src/libusb-glue.c | 208 +++++++++++++++++++++++++++++++----------------------
+ 1 files changed, 121 insertions(+), 87 deletions(-)
+
+diff --git a/src/libusb-glue.c b/src/libusb-glue.c
+index ea22384..ca8c47d 100644
+--- a/src/libusb-glue.c
++++ b/src/libusb-glue.c
+@@ -229,13 +229,18 @@ static int probe_device_descriptor(struct usb_device *dev, FILE *dumpfile)
+ unsigned char buf[1024], cmd;
+ int i;
+ int ret;
++ /* This is to indicate if we find some vendor interface */
++ int found_vendor_spec_interface = 0;
+
+ /*
+- * Don't examine hubs or HID devices, no point in that.
+- * (Some Kensington mice really don't like this.)
++ * Don't examine devices that are not likely to
++ * contain any MTP interface, update this the day
++ * you find some weird combination...
+ */
+- if (dev->descriptor.bDeviceClass == USB_CLASS_HUB ||
+- dev->descriptor.bDeviceClass == USB_CLASS_HID) {
++ if (dev->descriptor.bDeviceClass != USB_CLASS_PER_INTERFACE &&
++ dev->descriptor.bDeviceClass != USB_CLASS_COMM &&
++ dev->descriptor.bDeviceClass != USB_CLASS_PTP &&
++ dev->descriptor.bDeviceClass != USB_CLASS_VENDOR_SPEC) {
+ return 0;
+ }
+
+@@ -270,6 +275,15 @@ static int probe_device_descriptor(struct usb_device *dev, FILE *dumpfile)
+ &dev->config[i].interface[j].altsetting[k];
+
+ /*
++ * We only want to probe for the OS descriptor if the
++ * device is USB_CLASS_VENDOR_SPEC or one of the interfaces
++ * in it is, so flag if we find an interface like this.
++ */
++ if (intf->bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
++ found_vendor_spec_interface = 1;
++ }
++
++ /*
+ * Check for Still Image Capture class with PIMA 15740 protocol,
+ * also known as PTP
+ */
+@@ -329,98 +343,118 @@ static int probe_device_descriptor(struct usb_device *dev, FILE *dumpfile)
+ LIBMTP_INFO("dev->config is NULL in probe_device_descriptor yet dev->descriptor.bNumConfigurations > 0\n");
+ }
+
+- /* Read the special descriptor */
+- ret = usb_get_descriptor(devh, 0x03, 0xee, buf, sizeof(buf));
++ /*
++ * Only probe for OS descriptor if the device is vendor specific
++ * or one of the interfaces found is.
++ */
++ if (dev->descriptor.bDeviceClass == USB_CLASS_VENDOR_SPEC ||
++ found_vendor_spec_interface) {
+
+- // Dump it, if requested
+- if (dumpfile != NULL && ret > 0) {
+- fprintf(dumpfile, "Microsoft device descriptor 0xee:\n");
+- data_dump_ascii(dumpfile, buf, ret, 16);
+- }
++ /* Read the special descriptor */
++ ret = usb_get_descriptor(devh, 0x03, 0xee, buf, sizeof(buf));
+
+- /* Check if descriptor length is at least 10 bytes */
+- if (ret < 10) {
+- usb_close(devh);
+- return 0;
+- }
++ /*
++ * If something failed we're probably stalled to we need
++ * to clear the stall off the endpoint and say this is not
++ * MTP.
++ */
++ if (ret < 0) {
++ /* EP0 is the default control endpoint */
++ usb_clear_halt(devh, 0);
++ usb_close(devh);
++ return 0;
++ }
+
+- /* Check if this device has a Microsoft Descriptor */
+- if (!((buf[2] == 'M') && (buf[4] == 'S') &&
+- (buf[6] == 'F') && (buf[8] == 'T'))) {
+- usb_close(devh);
+- return 0;
+- }
++ // Dump it, if requested
++ if (dumpfile != NULL && ret > 0) {
++ fprintf(dumpfile, "Microsoft device descriptor 0xee:\n");
++ data_dump_ascii(dumpfile, buf, ret, 16);
++ }
+
+- /* Check if device responds to control message 1 or if there is an error */
+- cmd = buf[16];
+- ret = usb_control_msg (devh,
+- USB_ENDPOINT_IN | USB_RECIP_DEVICE | USB_TYPE_VENDOR,
+- cmd,
+- 0,
+- 4,
+- (char *) buf,
+- sizeof(buf),
+- USB_TIMEOUT_DEFAULT);
+-
+- // Dump it, if requested
+- if (dumpfile != NULL && ret > 0) {
+- fprintf(dumpfile, "Microsoft device response to control message 1, CMD 0x%02x:\n", cmd);
+- data_dump_ascii(dumpfile, buf, ret, 16);
+- }
++ /* Check if descriptor length is at least 10 bytes */
++ if (ret < 10) {
++ usb_close(devh);
++ return 0;
++ }
+
+- /* If this is true, the device either isn't MTP or there was an error */
+- if (ret <= 0x15) {
+- /* TODO: If there was an error, flag it and let the user know somehow */
+- /* if(ret == -1) {} */
+- usb_close(devh);
+- return 0;
+- }
++ /* Check if this device has a Microsoft Descriptor */
++ if (!((buf[2] == 'M') && (buf[4] == 'S') &&
++ (buf[6] == 'F') && (buf[8] == 'T'))) {
++ usb_close(devh);
++ return 0;
++ }
+
+- /* Check if device is MTP or if it is something like a USB Mass Storage
+- device with Janus DRM support */
+- if ((buf[0x12] != 'M') || (buf[0x13] != 'T') || (buf[0x14] != 'P')) {
+- usb_close(devh);
+- return 0;
+- }
++ /* Check if device responds to control message 1 or if there is an error */
++ cmd = buf[16];
++ ret = usb_control_msg (devh,
++ USB_ENDPOINT_IN | USB_RECIP_DEVICE | USB_TYPE_VENDOR,
++ cmd,
++ 0,
++ 4,
++ (char *) buf,
++ sizeof(buf),
++ USB_TIMEOUT_DEFAULT);
++
++ // Dump it, if requested
++ if (dumpfile != NULL && ret > 0) {
++ fprintf(dumpfile, "Microsoft device response to control message 1, CMD 0x%02x:\n", cmd);
++ data_dump_ascii(dumpfile, buf, ret, 16);
++ }
+
+- /* After this point we are probably dealing with an MTP device */
++ /* If this is true, the device either isn't MTP or there was an error */
++ if (ret <= 0x15) {
++ /* TODO: If there was an error, flag it and let the user know somehow */
++ /* if(ret == -1) {} */
++ usb_close(devh);
++ return 0;
++ }
+
+- /*
+- * Check if device responds to control message 2, which is
+- * the extended device parameters. Most devices will just
+- * respond with a copy of the same message as for the first
+- * message, some respond with zero-length (which is OK)
+- * and some with pure garbage. We're not parsing the result
+- * so this is not very important.
+- */
+- ret = usb_control_msg (devh,
+- USB_ENDPOINT_IN | USB_RECIP_DEVICE | USB_TYPE_VENDOR,
+- cmd,
+- 0,
+- 5,
+- (char *) buf,
+- sizeof(buf),
+- USB_TIMEOUT_DEFAULT);
+-
+- // Dump it, if requested
+- if (dumpfile != NULL && ret > 0) {
+- fprintf(dumpfile, "Microsoft device response to control message 2, CMD 0x%02x:\n", cmd);
+- data_dump_ascii(dumpfile, buf, ret, 16);
+- }
++ /* Check if device is MTP or if it is something like a USB Mass Storage
++ device with Janus DRM support */
++ if ((buf[0x12] != 'M') || (buf[0x13] != 'T') || (buf[0x14] != 'P')) {
++ usb_close(devh);
++ return 0;
++ }
++
++ /* After this point we are probably dealing with an MTP device */
++
++ /*
++ * Check if device responds to control message 2, which is
++ * the extended device parameters. Most devices will just
++ * respond with a copy of the same message as for the first
++ * message, some respond with zero-length (which is OK)
++ * and some with pure garbage. We're not parsing the result
++ * so this is not very important.
++ */
++ ret = usb_control_msg (devh,
++ USB_ENDPOINT_IN | USB_RECIP_DEVICE | USB_TYPE_VENDOR,
++ cmd,
++ 0,
++ 5,
++ (char *) buf,
++ sizeof(buf),
++ USB_TIMEOUT_DEFAULT);
++
++ // Dump it, if requested
++ if (dumpfile != NULL && ret > 0) {
++ fprintf(dumpfile, "Microsoft device response to control message 2, CMD 0x%02x:\n", cmd);
++ data_dump_ascii(dumpfile, buf, ret, 16);
++ }
+
+- /* If this is true, the device errored against control message 2 */
+- if (ret == -1) {
+- /* TODO: Implement callback function to let managing program know there
+- was a problem, along with description of the problem */
+- LIBMTP_ERROR("Potential MTP Device with VendorID:%04x and "
+- "ProductID:%04x encountered an error responding to "
+- "control message 2.\n"
+- "Problems may arrise but continuing\n",
+- dev->descriptor.idVendor, dev->descriptor.idProduct);
+- } else if (dumpfile != NULL && ret == 0) {
+- fprintf(dumpfile, "Zero-length response to control message 2 (OK)\n");
+- } else if (dumpfile != NULL) {
+- fprintf(dumpfile, "Device responds to control message 2 with some data.\n");
++ /* If this is true, the device errored against control message 2 */
++ if (ret == -1) {
++ /* TODO: Implement callback function to let managing program know there
++ was a problem, along with description of the problem */
++ LIBMTP_ERROR("Potential MTP Device with VendorID:%04x and "
++ "ProductID:%04x encountered an error responding to "
++ "control message 2.\n"
++ "Problems may arrise but continuing\n",
++ dev->descriptor.idVendor, dev->descriptor.idProduct);
++ } else if (dumpfile != NULL && ret == 0) {
++ fprintf(dumpfile, "Zero-length response to control message 2 (OK)\n");
++ } else if (dumpfile != NULL) {
++ fprintf(dumpfile, "Device responds to control message 2 with some data.\n");
++ }
+ }
+
+ /* Close the USB device handle */
+--
+1.7.3.4
+
diff --git a/libmtp.spec b/libmtp.spec
index 574ac2e..d5dd4d1 100644
--- a/libmtp.spec
+++ b/libmtp.spec
@@ -3,12 +3,13 @@
Name: libmtp
Version: 1.0.4
-Release: 1%{?dist}
+Release: 2%{?dist}
Summary: A software library for MTP media players
URL: http://libmtp.sourceforge.net/
Group: System Environment/Libraries
Source0: http://download.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
+Patch0: 0001-Fixup-device-OS-descriptor-probe-code.patch
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
License: LGPLv2+
Requires: udev
@@ -51,6 +52,7 @@ library for MTP media players.
%prep
%setup -q
+%patch0 -p1
%build
%configure --disable-static
@@ -126,6 +128,9 @@ rm -rf $RPM_BUILD_ROOT
%changelog
+* Wed Jan 19 2011 Linus Walleij <triad at df.lth.se> 1.0.4-2
+- Testing out a patch to be more careful when probing devices.
+
* Sun Jan 9 2011 Linus Walleij <triad at df.lth.se> 1.0.4-1
- New upstream version with mucho udev fixes.
More information about the scm-commits
mailing list