[Pcsclite-muscle] Fix libudev hotplug
Stefani Seibold
stefani at seibold.net
Mon Jul 7 12:22:50 UTC 2014
Hi,
the current libudev hotplug code have some issues.
The HPRegisterForHotplugEvents() does first a HPRescanUsbBus() and than
start the HPEstablishUSBNotifications() thread. This could lead in a
race condition, where a add event occurred between the scan and the
start of the thread. I moved the scan after the
udev_monitor_enable_receiving() call, so the race is fixed.
Next: The HPRescanUsbBus() is broken too. It will set all
readerTracker.status to READER_ABSENT. This make it very hard to track
reader where added twice. This can happen due the fix above and due the
fact that any udev event in the origin code could lead to a add, because
the "remove" event whill call HPRescanUsbBus() which calls
HPAddDevice(). This problem could be fixed using a temporary array of
status bytes.
And at last the HPAddDevice() is broken because a SCARD_E_UNKNOWN_READER
status could never overwriten. But the "remove" event handling which
also do device adds it could happen that a HPRescanUsbBus() will try to
add a device via HPAddDevice() which is still in use by other processes
or not ready. So a device should not permanently marked as
SCARD_E_UNKNOWN_READER because when the udev "add" event arrives than
the device should be normaly accessible.
The following small patch fix all this issues.
Greetings,
Stefani
diff -u -N -r -p pcsc-lite-1.8.11.orig/src/hotplug_libudev.c pcsc-lite-1.8.11/src/hotplug_libudev.c
--- pcsc-lite-1.8.11.orig/src/hotplug_libudev.c 2014-02-14 17:15:44.000000000 +0100
+++ pcsc-lite-1.8.11/src/hotplug_libudev.c 2014-07-07 13:33:37.830000506 +0200
@@ -321,7 +321,7 @@ static LONG HPReadBundleValues(void)
}
-static void HPAddDevice(struct udev_device *dev, struct udev_device *parent,
+static int HPAddDevice(struct udev_device *dev, struct udev_device *parent,
const char *devpath)
{
int i;
@@ -341,7 +341,7 @@ static void HPAddDevice(struct udev_devi
Log2(PCSC_LOG_DEBUG, "%s is not a supported smart card reader",
devpath);
#endif
- return;
+ return -1;
}
Log2(PCSC_LOG_INFO, "Adding USB device: %s", driver->readerName);
@@ -357,8 +357,6 @@ static void HPAddDevice(struct udev_devi
bInterfaceNumber, devpath);
deviceName[sizeof(deviceName) -1] = '\0';
- (void)pthread_mutex_lock(&usbNotifierMutex);
-
/* find a free entry */
for (i=0; i<PCSCLITE_MAX_READERS_CONTEXTS; i++)
{
@@ -370,8 +368,7 @@ static void HPAddDevice(struct udev_devi
{
Log2(PCSC_LOG_ERROR,
"Not enough reader entries. Already found %d readers", i);
- (void)pthread_mutex_unlock(&usbNotifierMutex);
- return;
+ return -1;
}
if (Add_Interface_In_Name)
@@ -411,17 +408,17 @@ static void HPAddDevice(struct udev_devi
ret = RFAddReader(fullname, PCSCLITE_HP_BASE_PORT + i,
driver->libraryPath, deviceName);
- if ((SCARD_S_SUCCESS != ret) && (SCARD_E_UNKNOWN_READER != ret))
+ if (SCARD_S_SUCCESS != ret)
{
Log2(PCSC_LOG_ERROR, "Failed adding USB device: %s",
driver->readerName);
- if (classdriver && driver != classdriver)
+ if (SCARD_E_UNKNOWN_READER != ret && classdriver && driver != classdriver)
{
/* the reader can also be used by the a class driver */
ret = RFAddReader(fullname, PCSCLITE_HP_BASE_PORT + i,
classdriver->libraryPath, deviceName);
- if ((SCARD_S_SUCCESS != ret) && (SCARD_E_UNKNOWN_READER != ret))
+ if (SCARD_S_SUCCESS != ret)
{
Log2(PCSC_LOG_ERROR, "Failed adding USB device: %s",
driver->readerName);
@@ -439,7 +436,7 @@ static void HPAddDevice(struct udev_devi
}
}
- (void)pthread_mutex_unlock(&usbNotifierMutex);
+ return i;
} /* HPAddDevice */
@@ -448,10 +445,11 @@ static void HPRescanUsbBus(struct udev *
int i, j;
struct udev_enumerate *enumerate;
struct udev_list_entry *devices, *dev_list_entry;
+ readerState_t readerTrackerStatus[PCSCLITE_MAX_READERS_CONTEXTS];
/* all reader are marked absent */
for (i=0; i < PCSCLITE_MAX_READERS_CONTEXTS; i++)
- readerTracker[i].status = READER_ABSENT;
+ readerTrackerStatus[i] = READER_ABSENT;
/* Create a list of the devices in the 'usb' subsystem. */
enumerate = udev_enumerate_new(udev);
@@ -459,6 +457,8 @@ static void HPRescanUsbBus(struct udev *
udev_enumerate_scan_devices(enumerate);
devices = udev_enumerate_get_list_entry(enumerate);
+ pthread_mutex_lock(&usbNotifierMutex);
+
/* For each item enumerated */
udev_list_entry_foreach(dev_list_entry, devices)
{
@@ -510,12 +510,13 @@ static void HPRescanUsbBus(struct udev *
/* Check if the reader is a new one */
for (j=0; j<PCSCLITE_MAX_READERS_CONTEXTS; j++)
{
- if (readerTracker[j].devpath
+ if ((readerTrackerStatus[j] == READER_ABSENT)
+ && (readerTracker[j].devpath)
&& (strcmp(readerTracker[j].devpath, devpath) == 0)
&& (bInterfaceNumber == readerTracker[j].bInterfaceNumber))
{
/* The reader is already known */
- readerTracker[j].status = READER_PRESENT;
+ readerTrackerStatus[j] = READER_PRESENT;
newreader = FALSE;
#ifdef DEBUG_HOTPLUG
Log2(PCSC_LOG_DEBUG, "Refresh USB device: %s", devpath);
@@ -525,8 +526,12 @@ static void HPRescanUsbBus(struct udev *
}
/* New reader found */
- if (newreader)
- HPAddDevice(dev, parent, devpath);
+ if (newreader) {
+Log2(PCSC_LOG_DEBUG, "Add USB device: %s", devpath);
+ i = HPAddDevice(dev, parent, devpath);
+ if (i >= 0 && readerTracker[i].status == READER_PRESENT)
+ readerTrackerStatus[i] = READER_PRESENT;
+ }
/* free device */
udev_device_unref(dev);
@@ -535,11 +540,11 @@ static void HPRescanUsbBus(struct udev *
/* Free the enumerator object */
udev_enumerate_unref(enumerate);
- pthread_mutex_lock(&usbNotifierMutex);
/* check if all the previously found readers are still present */
for (i=0; i<PCSCLITE_MAX_READERS_CONTEXTS; i++)
{
- if ((READER_ABSENT == readerTracker[i].status)
+ if ((readerTracker[i].status != readerTrackerStatus[i])
+ && (READER_ABSENT != readerTracker[i].status)
&& (readerTracker[i].fullName != NULL))
{
Log4(PCSC_LOG_INFO, "Removing USB device[%d]: %s at %s", i,
@@ -584,13 +589,15 @@ static void HPEstablishUSBNotifications(
return;
}
+ HPRescanUsbBus(udev);
+
/* udev monitor file descriptor */
fd = udev_monitor_get_fd(udev_monitor);
while (!AraKiriHotPlug)
{
- struct udev_device *dev, *parent;
- const char *action, *devpath;
+ struct udev_device *dev;
+ const char *action;
#ifdef DEBUG_HOTPLUG
Log0(PCSC_LOG_INFO);
@@ -622,24 +629,14 @@ static void HPEstablishUSBNotifications(
continue;
}
- if (strcmp("add", action))
- continue;
-
- parent = udev_device_get_parent_with_subsystem_devtype(dev, "usb",
- "usb_device");
- devpath = udev_device_get_devnode(parent);
- if (!devpath)
- {
- /* the device disapeared? */
- Log1(PCSC_LOG_ERROR, "udev_device_get_devnode() failed");
+ if (0 == strcmp("add", action)) {
+ Log1(PCSC_LOG_INFO, "Device added");
+ HPRescanUsbBus(udev);
continue;
}
-
- HPAddDevice(dev, parent, devpath);
-
+Log2(PCSC_LOG_INFO, "action: %s", action);
/* free device */
udev_device_unref(dev);
-
}
for (i=0; i<driverSize; i++)
@@ -710,8 +707,6 @@ ULONG HPRegisterForHotplugEvents(void)
return 0;
}
- HPRescanUsbBus(udev);
-
(void)ThreadCreate(&usbNotifyThread, THREAD_ATTR_DETACHED,
(PCSCLITE_THREAD_FUNCTION( )) HPEstablishUSBNotifications, udev);
More information about the Pcsclite-muscle
mailing list