Protection against concurrent access to USBHost

Fixed regression: Protection against concurrent access to USBHost using
locking class
Use Lock class within USBHost
pull/202/head
Donatien Garnier 2014-03-07 15:01:57 +00:00
parent 8c6ca15be8
commit 876134ec22
3 changed files with 213 additions and 203 deletions

View File

@ -79,133 +79,136 @@ void USBHost::usb_process() {
too_many_hub = false; too_many_hub = false;
buf[4] = 0; buf[4] = 0;
usb_mutex.lock(); do
{
Lock lock(this);
for (i = 0; i < MAX_DEVICE_CONNECTED; i++) { for (i = 0; i < MAX_DEVICE_CONNECTED; i++) {
if (!deviceInUse[i]) { if (!deviceInUse[i]) {
USB_DBG_EVENT("new device connected: %p\r\n", &devices[i]); USB_DBG_EVENT("new device connected: %p\r\n", &devices[i]);
devices[i].init(usb_msg->hub, usb_msg->port, usb_msg->lowSpeed); devices[i].init(usb_msg->hub, usb_msg->port, usb_msg->lowSpeed);
deviceReset[i] = false; deviceReset[i] = false;
deviceInited[i] = true; deviceInited[i] = true;
break; break;
} }
} }
if (i == MAX_DEVICE_CONNECTED) { if (i == MAX_DEVICE_CONNECTED) {
USB_ERR("Too many device connected!!\r\n"); USB_ERR("Too many device connected!!\r\n");
usb_mutex.unlock(); continue;
continue; }
}
if (!controlEndpointAllocated) { if (!controlEndpointAllocated) {
control = newEndpoint(CONTROL_ENDPOINT, OUT, 0x08, 0x00); control = newEndpoint(CONTROL_ENDPOINT, OUT, 0x08, 0x00);
addEndpoint(NULL, 0, (USBEndpoint*)control); addEndpoint(NULL, 0, (USBEndpoint*)control);
controlEndpointAllocated = true; controlEndpointAllocated = true;
} }
#if MAX_HUB_NB #if MAX_HUB_NB
if (usb_msg->hub_parent) if (usb_msg->hub_parent)
devices[i].setHubParent((USBHostHub *)(usb_msg->hub_parent)); devices[i].setHubParent((USBHostHub *)(usb_msg->hub_parent));
#endif #endif
for (j = 0; j < timeout_set_addr; j++) { for (j = 0; j < timeout_set_addr; j++) {
resetDevice(&devices[i]); resetDevice(&devices[i]);
// set size of control endpoint // set size of control endpoint
devices[i].setSizeControlEndpoint(8); devices[i].setSizeControlEndpoint(8);
devices[i].activeAddress(false); devices[i].activeAddress(false);
// get first 8 bit of device descriptor // get first 8 bit of device descriptor
// and check if we deal with a hub // and check if we deal with a hub
USB_DBG("usb_thread read device descriptor on dev: %p\r\n", &devices[i]); USB_DBG("usb_thread read device descriptor on dev: %p\r\n", &devices[i]);
res = getDeviceDescriptor(&devices[i], buf, 8); res = getDeviceDescriptor(&devices[i], buf, 8);
if (res != USB_TYPE_OK) { if (res != USB_TYPE_OK) {
USB_ERR("usb_thread could not read dev descr"); USB_ERR("usb_thread could not read dev descr");
continue; continue;
} }
// set size of control endpoint // set size of control endpoint
devices[i].setSizeControlEndpoint(buf[7]); devices[i].setSizeControlEndpoint(buf[7]);
// second step: set an address to the device // second step: set an address to the device
res = setAddress(&devices[i], devices[i].getAddress()); res = setAddress(&devices[i], devices[i].getAddress());
if (res != USB_TYPE_OK) { if (res != USB_TYPE_OK) {
USB_ERR("SET ADDR FAILED"); USB_ERR("SET ADDR FAILED");
continue; continue;
} }
devices[i].activeAddress(true); devices[i].activeAddress(true);
USB_DBG("Address of %p: %d", &devices[i], devices[i].getAddress()); USB_DBG("Address of %p: %d", &devices[i], devices[i].getAddress());
// try to read again the device descriptor to check if the device // try to read again the device descriptor to check if the device
// answers to its new address // answers to its new address
res = getDeviceDescriptor(&devices[i], buf, 8); res = getDeviceDescriptor(&devices[i], buf, 8);
if (res == USB_TYPE_OK) { if (res == USB_TYPE_OK) {
break; break;
} }
Thread::wait(100); Thread::wait(100);
} }
USB_INFO("New device connected: %p [hub: %d - port: %d]", &devices[i], usb_msg->hub, usb_msg->port); USB_INFO("New device connected: %p [hub: %d - port: %d]", &devices[i], usb_msg->hub, usb_msg->port);
#if MAX_HUB_NB #if MAX_HUB_NB
if (buf[4] == HUB_CLASS) { if (buf[4] == HUB_CLASS) {
for (k = 0; k < MAX_HUB_NB; k++) { for (k = 0; k < MAX_HUB_NB; k++) {
if (hub_in_use[k] == false) { if (hub_in_use[k] == false) {
for (uint8_t j = 0; j < MAX_TRY_ENUMERATE_HUB; j++) { for (uint8_t j = 0; j < MAX_TRY_ENUMERATE_HUB; j++) {
if (hubs[k].connect(&devices[i])) { if (hubs[k].connect(&devices[i])) {
devices[i].hub = &hubs[k]; devices[i].hub = &hubs[k];
hub_in_use[k] = true; hub_in_use[k] = true;
break; break;
} }
} }
if (hub_in_use[k] == true) if (hub_in_use[k] == true)
break; break;
} }
} }
if (k == MAX_HUB_NB) { if (k == MAX_HUB_NB) {
USB_ERR("Too many hubs connected!!\r\n"); USB_ERR("Too many hubs connected!!\r\n");
too_many_hub = true; too_many_hub = true;
} }
} }
if (usb_msg->hub_parent) if (usb_msg->hub_parent)
((USBHostHub *)(usb_msg->hub_parent))->deviceConnected(&devices[i]); ((USBHostHub *)(usb_msg->hub_parent))->deviceConnected(&devices[i]);
#endif #endif
if ((i < MAX_DEVICE_CONNECTED) && !too_many_hub) { if ((i < MAX_DEVICE_CONNECTED) && !too_many_hub) {
deviceInUse[i] = true; deviceInUse[i] = true;
} }
usb_mutex.unlock(); } while(0);
break; break;
// a device has been disconnected // a device has been disconnected
case DEVICE_DISCONNECTED_EVENT: case DEVICE_DISCONNECTED_EVENT:
usb_mutex.lock(); do
{
Lock lock(this);
controlListState = disableList(CONTROL_ENDPOINT); controlListState = disableList(CONTROL_ENDPOINT);
bulkListState = disableList(BULK_ENDPOINT); bulkListState = disableList(BULK_ENDPOINT);
interruptListState = disableList(INTERRUPT_ENDPOINT); interruptListState = disableList(INTERRUPT_ENDPOINT);
idx = findDevice(usb_msg->hub, usb_msg->port, (USBHostHub *)(usb_msg->hub_parent)); idx = findDevice(usb_msg->hub, usb_msg->port, (USBHostHub *)(usb_msg->hub_parent));
if (idx != -1) { if (idx != -1) {
freeDevice((USBDeviceConnected*)&devices[idx]); freeDevice((USBDeviceConnected*)&devices[idx]);
} }
if (controlListState) enableList(CONTROL_ENDPOINT); if (controlListState) enableList(CONTROL_ENDPOINT);
if (bulkListState) enableList(BULK_ENDPOINT); if (bulkListState) enableList(BULK_ENDPOINT);
if (interruptListState) enableList(INTERRUPT_ENDPOINT); if (interruptListState) enableList(INTERRUPT_ENDPOINT);
usb_mutex.unlock(); } while(0);
break; break;
@ -278,6 +281,15 @@ USBHost::USBHost() : usbThread(USBHost::usb_process_static, (void *)this, osPrio
#endif #endif
} }
USBHost::Lock::Lock(USBHost* pHost) : m_pHost(pHost)
{
m_pHost->usb_mutex.lock();
}
USBHost::Lock::~Lock()
{
m_pHost->usb_mutex.unlock();
}
void USBHost::transferCompleted(volatile uint32_t addr) void USBHost::transferCompleted(volatile uint32_t addr)
{ {
@ -810,83 +822,81 @@ USB_TYPE USBHost::enumerate(USBDeviceConnected * dev, IUSBEnumerator* pEnumerato
uint16_t total_conf_descr_length = 0; uint16_t total_conf_descr_length = 0;
USB_TYPE res; USB_TYPE res;
usb_mutex.lock(); do
{
Lock lock(this);
// don't enumerate a device which all interfaces are registered to a specific driver // don't enumerate a device which all interfaces are registered to a specific driver
int index = findDevice(dev); int index = findDevice(dev);
if (index == -1) { if (index == -1) {
usb_mutex.unlock(); return USB_TYPE_ERROR;
return USB_TYPE_ERROR; }
}
uint8_t nb_intf_attached = numberDriverAttached(dev); uint8_t nb_intf_attached = numberDriverAttached(dev);
USB_DBG("dev: %p nb_intf: %d", dev, dev->getNbIntf()); USB_DBG("dev: %p nb_intf: %d", dev, dev->getNbIntf());
USB_DBG("dev: %p nb_intf_attached: %d", dev, nb_intf_attached); USB_DBG("dev: %p nb_intf_attached: %d", dev, nb_intf_attached);
if ((nb_intf_attached != 0) && (dev->getNbIntf() == nb_intf_attached)) { if ((nb_intf_attached != 0) && (dev->getNbIntf() == nb_intf_attached)) {
USB_DBG("Don't enumerate dev: %p because all intf are registered with a driver", dev); USB_DBG("Don't enumerate dev: %p because all intf are registered with a driver", dev);
usb_mutex.unlock(); return USB_TYPE_OK;
return USB_TYPE_OK; }
}
USB_DBG("Enumerate dev: %p", dev); USB_DBG("Enumerate dev: %p", dev);
// third step: get the whole device descriptor to see vid, pid // third step: get the whole device descriptor to see vid, pid
res = getDeviceDescriptor(dev, data, DEVICE_DESCRIPTOR_LENGTH); res = getDeviceDescriptor(dev, data, DEVICE_DESCRIPTOR_LENGTH);
if (res != USB_TYPE_OK) { if (res != USB_TYPE_OK) {
USB_DBG("GET DEV DESCR FAILED"); USB_DBG("GET DEV DESCR FAILED");
usb_mutex.unlock(); return res;
return res; }
}
dev->setClass(data[4]); dev->setClass(data[4]);
dev->setSubClass(data[5]); dev->setSubClass(data[5]);
dev->setProtocol(data[6]); dev->setProtocol(data[6]);
dev->setVid(data[8] | (data[9] << 8)); dev->setVid(data[8] | (data[9] << 8));
dev->setPid(data[10] | (data[11] << 8)); dev->setPid(data[10] | (data[11] << 8));
USB_DBG("CLASS: %02X \t VID: %04X \t PID: %04X", data[4], data[8] | (data[9] << 8), data[10] | (data[11] << 8)); USB_DBG("CLASS: %02X \t VID: %04X \t PID: %04X", data[4], data[8] | (data[9] << 8), data[10] | (data[11] << 8));
pEnumerator->setVidPid( data[8] | (data[9] << 8), data[10] | (data[11] << 8) ); pEnumerator->setVidPid( data[8] | (data[9] << 8), data[10] | (data[11] << 8) );
res = getConfigurationDescriptor(dev, data, sizeof(data), &total_conf_descr_length); res = getConfigurationDescriptor(dev, data, sizeof(data), &total_conf_descr_length);
if (res != USB_TYPE_OK) { if (res != USB_TYPE_OK) {
usb_mutex.unlock(); return res;
return res; }
}
#if (DEBUG > 3) #if (DEBUG > 3)
USB_DBG("CONFIGURATION DESCRIPTOR:\r\n"); USB_DBG("CONFIGURATION DESCRIPTOR:\r\n");
for (int i = 0; i < total_conf_descr_length; i++) for (int i = 0; i < total_conf_descr_length; i++)
printf("%02X ", data[i]); printf("%02X ", data[i]);
printf("\r\n\r\n"); printf("\r\n\r\n");
#endif #endif
// Parse the configuration descriptor // Parse the configuration descriptor
parseConfDescr(dev, data, total_conf_descr_length, pEnumerator); parseConfDescr(dev, data, total_conf_descr_length, pEnumerator);
// only set configuration if not enumerated before // only set configuration if not enumerated before
if (!dev->isEnumerated()) { if (!dev->isEnumerated()) {
USB_DBG("Set configuration 1 on dev: %p", dev); USB_DBG("Set configuration 1 on dev: %p", dev);
// sixth step: set configuration (only 1 supported) // sixth step: set configuration (only 1 supported)
res = setConfiguration(dev, 1); res = setConfiguration(dev, 1);
if (res != USB_TYPE_OK) { if (res != USB_TYPE_OK) {
USB_DBG("SET CONF FAILED"); USB_DBG("SET CONF FAILED");
usb_mutex.unlock(); return res;
return res; }
} }
}
dev->setEnumerated(); dev->setEnumerated();
// Now the device is enumerated! // Now the device is enumerated!
USB_DBG("dev %p is enumerated\r\n", dev); USB_DBG("dev %p is enumerated\r\n", dev);
usb_mutex.unlock();
} while(0);
// Some devices may require this delay // Some devices may require this delay
wait_ms(100); Thread::wait(100);
return USB_TYPE_OK; return USB_TYPE_OK;
} }
@ -987,38 +997,33 @@ USB_TYPE USBHost::generalTransfer(USBDeviceConnected * dev, USBEndpoint * ep, ui
USB_DBG_TRANSFER("----- %s %s [dev: %p - %s - hub: %d - port: %d - addr: %d - ep: %02X]------", type_str, (write) ? "WRITE" : "READ", dev, dev->getName(ep->getIntfNb()), dev->getHub(), dev->getPort(), dev->getAddress(), ep->getAddress()); USB_DBG_TRANSFER("----- %s %s [dev: %p - %s - hub: %d - port: %d - addr: %d - ep: %02X]------", type_str, (write) ? "WRITE" : "READ", dev, dev->getName(ep->getIntfNb()), dev->getHub(), dev->getPort(), dev->getAddress(), ep->getAddress());
#endif #endif
usb_mutex.lock(); Lock lock(this);
USB_TYPE res; USB_TYPE res;
ENDPOINT_DIRECTION dir = (write) ? OUT : IN; ENDPOINT_DIRECTION dir = (write) ? OUT : IN;
if (dev == NULL) { if (dev == NULL) {
USB_ERR("dev NULL"); USB_ERR("dev NULL");
usb_mutex.unlock();
return USB_TYPE_ERROR; return USB_TYPE_ERROR;
} }
if (ep == NULL) { if (ep == NULL) {
USB_ERR("ep NULL"); USB_ERR("ep NULL");
usb_mutex.unlock();
return USB_TYPE_ERROR; return USB_TYPE_ERROR;
} }
if (ep->getState() != USB_TYPE_IDLE) { if (ep->getState() != USB_TYPE_IDLE) {
USB_WARN("[ep: %p - dev: %p - %s] NOT IDLE: %s", ep, ep->dev, ep->dev->getName(ep->getIntfNb()), ep->getStateString()); USB_WARN("[ep: %p - dev: %p - %s] NOT IDLE: %s", ep, ep->dev, ep->dev->getName(ep->getIntfNb()), ep->getStateString());
usb_mutex.unlock();
return ep->getState(); return ep->getState();
} }
if ((ep->getDir() != dir) || (ep->getType() != type)) { if ((ep->getDir() != dir) || (ep->getType() != type)) {
USB_ERR("[ep: %p - dev: %p] wrong dir or bad USBEndpoint type", ep, ep->dev); USB_ERR("[ep: %p - dev: %p] wrong dir or bad USBEndpoint type", ep, ep->dev);
usb_mutex.unlock();
return USB_TYPE_ERROR; return USB_TYPE_ERROR;
} }
if (dev->getAddress() != ep->getDeviceAddress()) { if (dev->getAddress() != ep->getDeviceAddress()) {
USB_ERR("[ep: %p - dev: %p] USBEndpoint addr and device addr don't match", ep, ep->dev); USB_ERR("[ep: %p - dev: %p] USBEndpoint addr and device addr don't match", ep, ep->dev);
usb_mutex.unlock();
return USB_TYPE_ERROR; return USB_TYPE_ERROR;
} }
@ -1040,15 +1045,12 @@ USB_TYPE USBHost::generalTransfer(USBDeviceConnected * dev, USBEndpoint * ep, ui
USB_DBG_TRANSFER("%s TRANSFER res: %s on ep: %p\r\n", type_str, ep->getStateString(), ep); USB_DBG_TRANSFER("%s TRANSFER res: %s on ep: %p\r\n", type_str, ep->getStateString(), ep);
if (res != USB_TYPE_IDLE) { if (res != USB_TYPE_IDLE) {
usb_mutex.unlock();
return res; return res;
} }
usb_mutex.unlock();
return USB_TYPE_OK; return USB_TYPE_OK;
} }
usb_mutex.unlock();
return USB_TYPE_PROCESSING; return USB_TYPE_PROCESSING;
} }
@ -1064,7 +1066,7 @@ USB_TYPE USBHost::controlWrite(USBDeviceConnected * dev, uint8_t requestType, ui
USB_TYPE USBHost::controlTransfer(USBDeviceConnected * dev, uint8_t requestType, uint8_t request, uint32_t value, uint32_t index, uint8_t * buf, uint32_t len, bool write) USB_TYPE USBHost::controlTransfer(USBDeviceConnected * dev, uint8_t requestType, uint8_t request, uint32_t value, uint32_t index, uint8_t * buf, uint32_t len, bool write)
{ {
usb_mutex.lock(); Lock lock(this);
USB_DBG_TRANSFER("----- CONTROL %s [dev: %p - hub: %d - port: %d] ------", (write) ? "WRITE" : "READ", dev, dev->getHub(), dev->getPort()); USB_DBG_TRANSFER("----- CONTROL %s [dev: %p - hub: %d - port: %d] ------", (write) ? "WRITE" : "READ", dev, dev->getHub(), dev->getPort());
int length_transfer = len; int length_transfer = len;
@ -1098,7 +1100,6 @@ USB_TYPE USBHost::controlTransfer(USBDeviceConnected * dev, uint8_t requestType,
USB_DBG_TRANSFER("CONTROL setup stage %s", control->getStateString()); USB_DBG_TRANSFER("CONTROL setup stage %s", control->getStateString());
if (res != USB_TYPE_IDLE) { if (res != USB_TYPE_IDLE) {
usb_mutex.unlock();
return res; return res;
} }
@ -1126,7 +1127,6 @@ USB_TYPE USBHost::controlTransfer(USBDeviceConnected * dev, uint8_t requestType,
#endif #endif
if (res != USB_TYPE_IDLE) { if (res != USB_TYPE_IDLE) {
usb_mutex.unlock();
return res; return res;
} }
} }
@ -1139,7 +1139,6 @@ USB_TYPE USBHost::controlTransfer(USBDeviceConnected * dev, uint8_t requestType,
res = control->getState(); res = control->getState();
USB_DBG_TRANSFER("CONTROL ack stage %s", control->getStateString()); USB_DBG_TRANSFER("CONTROL ack stage %s", control->getStateString());
usb_mutex.unlock();
if (res != USB_TYPE_IDLE) if (res != USB_TYPE_IDLE)
return res; return res;

View File

@ -187,6 +187,18 @@ public:
} }
} }
/**
* Instantiate to protect USB thread from accessing shared objects (USBConnectedDevices and Interfaces)
*/
class Lock
{
public:
Lock(USBHost* pHost);
~Lock();
private:
USBHost* m_pHost;
};
friend class USBHostHub; friend class USBHostHub;
protected: protected:

View File

@ -20,11 +20,6 @@
#ifdef USBHOST_3GMODULE #ifdef USBHOST_3GMODULE
#define __DEBUG__ 0
#ifndef __MODULE__
#define __MODULE__ "WANDongle.cpp"
#endif
#include "dbg.h" #include "dbg.h"
#include <stdint.h> #include <stdint.h>
#include "rtos.h" #include "rtos.h"
@ -50,11 +45,15 @@ bool WANDongle::tryConnect()
USB_DBG("Trying to connect device"); USB_DBG("Trying to connect device");
if (dev_connected) { if (dev_connected) {
USB_DBG("Device is already connected!");
return true; return true;
} }
m_pInitializer = NULL; m_pInitializer = NULL;
//Protect from concurrent access from USB thread
USBHost::Lock lock(host);
for (int i = 0; i < MAX_DEVICE_CONNECTED; i++) for (int i = 0; i < MAX_DEVICE_CONNECTED; i++)
{ {
if ((dev = host->getDevice(i)) != NULL) if ((dev = host->getDevice(i)) != NULL)