This is my patch proposal to address the issue.
I could verify that redundant code was also being called on for
composite devices (with set_composite_interface() - Unlike HID, it won't
crash the app, but unwarranted warnings are produced), so my proposal is
to just use a flag that indicates whether we've already initialized the
private interface data, through the (renamed) composite_api_flags attribute.
I also took this opportunity to reorder some of the attributes in
windows_device_priv, as well as add the check for table overflow from
Toby's patch.
Regards,
/Pete
PS: I also just pushed the CloseHandle fix from Simon Haggett that was
promoted by Toby [1]. Toby, feel free to delete your original winCE
branch, as I have now updated the reference in issue #35.
[1]
https://github.com/libusbx/libusbx/commit/ab1b3843bfd544daa5f907d7b9b4c35dce9411dc
>From 9f46beaf5b2a4b41e125125ebb77b1db9bef63ea Mon Sep 17 00:00:00 2001
From: Pete Batard <p...@akeo.ie>
Date: Tue, 17 Jul 2012 17:36:13 +0100
Subject: [PATCH] Windows: Fix overflow when handling HID or composite devices
* When libusb_get_device_list() is called mutliple times, the HID device
path was unconditionally duplicated in the list of device's interfaces.
* Because array boundaries were not checked, this caused overflow and crash.
* This patch adds an out of bound check and also ensures that duplication
of data, for HID and composite, does not occur
* It also renames the private composite_api_flags to api_flags, as well as
reorganizes the private attributes
* Bug report and part of the fix provided by Toby Gray
---
libusb/os/windows_usb.c | 17 +++++++++++++----
libusb/os/windows_usb.h | 10 ++++++----
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/libusb/os/windows_usb.c b/libusb/os/windows_usb.c
index 3b90d18..8615039 100644
--- a/libusb/os/windows_usb.c
+++ b/libusb/os/windows_usb.c
@@ -1149,6 +1149,8 @@ static int set_composite_interface(struct libusb_context*
ctx, struct libusb_dev
struct windows_device_priv *priv = _device_priv(dev);
int interface_number;
+ if (priv->api_flags & USB_API_SET)
+ return LIBUSB_SUCCESS;
if (priv->apib->id != USB_API_COMPOSITE) {
usbi_err(ctx, "program assertion failed: '%s' is not
composite", device_id);
return LIBUSB_ERROR_NO_DEVICE;
@@ -1191,7 +1193,7 @@ static int set_composite_interface(struct libusb_context*
ctx, struct libusb_dev
if (priv->hid == NULL)
return LIBUSB_ERROR_NO_MEM;
}
- priv->composite_api_flags |= 1<<api;
+ priv->api_flags |= USB_API_SET | (1<<api);
return LIBUSB_SUCCESS;
}
@@ -1201,14 +1203,21 @@ static int set_hid_interface(struct libusb_context*
ctx, struct libusb_device* d
{
struct windows_device_priv *priv = _device_priv(dev);
+ if (priv->api_flags & USB_API_SET)
+ return LIBUSB_SUCCESS;
if (priv->hid == NULL) {
usbi_err(ctx, "program assertion failed: parent is not HID");
return LIBUSB_ERROR_NO_DEVICE;
}
+ if (priv->hid->nb_interfaces == USB_MAXINTERFACES) {
+ usbi_err(ctx, "program assertion failed: max USB interfaces
reached for HID device");
+ return LIBUSB_ERROR_NO_DEVICE;
+ }
priv->usb_interface[priv->hid->nb_interfaces].path = dev_interface_path;
priv->usb_interface[priv->hid->nb_interfaces].apib =
&usb_api_backend[USB_API_HID];
usbi_dbg("interface[%d] = %s", priv->hid->nb_interfaces,
dev_interface_path);
priv->hid->nb_interfaces++;
+ priv->api_flags |= USB_API_SET;
return LIBUSB_SUCCESS;
}
@@ -3983,7 +3992,7 @@ static int composite_open(struct libusb_device_handle
*dev_handle)
uint8_t flag = 1<<USB_API_WINUSB;
for (api=USB_API_WINUSB; api<USB_API_MAX; api++) {
- if (priv->composite_api_flags & flag) {
+ if (priv->api_flags & flag) {
r = usb_api_backend[api].open(dev_handle);
if (r != LIBUSB_SUCCESS) {
return r;
@@ -4001,7 +4010,7 @@ static void composite_close(struct libusb_device_handle
*dev_handle)
uint8_t flag = 1<<USB_API_WINUSB;
for (api=USB_API_WINUSB; api<USB_API_MAX; api++) {
- if (priv->composite_api_flags & flag) {
+ if (priv->api_flags & flag) {
usb_api_backend[api].close(dev_handle);
}
flag <<= 1;
@@ -4126,7 +4135,7 @@ static int composite_reset_device(struct
libusb_device_handle *dev_handle)
uint8_t flag = 1<<USB_API_WINUSB;
for (api=USB_API_WINUSB; api<USB_API_MAX; api++) {
- if (priv->composite_api_flags & flag) {
+ if (priv->api_flags & flag) {
r = usb_api_backend[api].reset_device(dev_handle);
if (r != LIBUSB_SUCCESS) {
return r;
diff --git a/libusb/os/windows_usb.h b/libusb/os/windows_usb.h
index 331f75c..cc5c398 100644
--- a/libusb/os/windows_usb.h
+++ b/libusb/os/windows_usb.h
@@ -120,6 +120,8 @@ const GUID GUID_NULL = { 0x00000000, 0x0000, 0x0000, {0x00,
0x00, 0x00, 0x00, 0x
#define USB_API_WINUSB 3
#define USB_API_HID 4
#define USB_API_MAX 5
+// The following is used to indicate if the HID or composite extra props have
already been set.
+#define USB_API_SET (1<<USB_API_MAX)
#define CLASS_GUID_UNSUPPORTED GUID_NULL
const GUID CLASS_GUID_HID = { 0x745A17A0, 0x74D3, 0x11D0, {0xB6,
0xFE, 0x00, 0xA0, 0xC9, 0x0F, 0x57, 0xDA} };
@@ -229,9 +231,11 @@ typedef struct libusb_device_descriptor
USB_DEVICE_DESCRIPTOR, *PUSB_DEVICE_DESC
struct windows_device_priv {
uint8_t depth; // distance to
HCD
uint8_t port; // port number
on the hub
+ uint8_t active_config;
+ uint8_t api_flags; // HID and
composite devices require additional data
struct libusb_device *parent_dev; // access to parent is required
for usermode ops
- char *path; //
device interface path
struct windows_usb_api_backend const *apib;
+ char *path; //
device interface path
struct {
char *path; // each
interface needs a device interface path,
struct windows_usb_api_backend const *apib; // an API backend
(multiple drivers support),
@@ -240,9 +244,7 @@ struct windows_device_priv {
bool restricted_functionality; // indicates if the interface
functionality is restricted
// by Windows (eg. HID keyboards or mice cannot do R/W)
} usb_interface[USB_MAXINTERFACES];
- uint8_t composite_api_flags; // HID and composite devices
require additional data
struct hid_device_priv *hid;
- uint8_t active_config;
USB_DEVICE_DESCRIPTOR dev_descriptor;
unsigned char **config_descriptor; // list of pointers to the
cached config descriptors
};
@@ -259,7 +261,7 @@ static inline void windows_device_priv_init(libusb_device*
dev) {
p->parent_dev = NULL;
p->path = NULL;
p->apib = &usb_api_backend[USB_API_UNSUPPORTED];
- p->composite_api_flags = 0;
+ p->api_flags = 0;
p->hid = NULL;
p->active_config = 0;
p->config_descriptor = NULL;
--
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
