While doing some smartcard testing on Solaris 10 I ran across two bugs
with the open source pcsc-lite and ccid code as it interacts with the
Solaris libusb.

The first fix was accepted by pcsc-lite, but the second was rejected,
as a problem with libusb, not pcsc-lite or ccid.

I have attached the original messsge with the patches as I know there
has been a lot of interest within Sun with using smart cards, Kerberos
PKINIT, opensc and pcscd and a number of messages on how to get these
to compile on Solaris and Open Solaris.

I would hope that someone within Sun would find this interesting and look
at it closer to see if it is indeed a bug on libusb or not and if it also
fails on Open Solaris. For my testing I can continue to apply the ccid
patch.


2010/2/19 Douglas E. Engert <[email protected]>:
> Two fixes:
>
> GCC
>
> The gcc on Solaris 10 combined with the Sun loader appears to not handle
> the gcc visibility attribute correctly. The sparc version says it is
> ignored, the x86 version gives linker error. The attached patch
> sun.gcc.1.5.6-svn-477.txt tries to test for gcc on Sun and not use
> the visibility attribute.  If on a sun and the compiler is not GCC,
> try and use the Sun __global and __hidden instead. (I did not try the Sun
> Studio compiler with this.)

Committed in revision 4766.
Thanks

> PCSCD CRASH
>
> pcscd would crash sometimes when a reader was unplugged on Solaris 10
> or would not recognize the reader if it was plugged back in. The problem
> appears to be that once the libusb returns errno == ENODEV no further
> calls should be made other then to usb_close.

The crash is not in pcscd but in libusb on Solaris.
libusb should not crash if called on a device that disappeared. libusb
should return an error code instead.
I reject this patch, sorry.

Bye




--

 Douglas E. Engert  <[email protected]>
 Argonne National Laboratory
 9700 South Cass Avenue
 Argonne, Illinois  60439
 (630) 252-5444
--- Begin Message ---
Two fixes:

GCC

The gcc on Solaris 10 combined with the Sun loader appears to not handle
the gcc visibility attribute correctly. The sparc version says it is
ignored, the x86 version gives linker error. The attached patch
sun.gcc.1.5.6-svn-477.txt tries to test for gcc on Sun and not use
the visibility attribute.  If on a sun and the compiler is not GCC,
try and use the Sun __global and __hidden instead. (I did not try the Sun
Studio compiler with this.)

PCSCD CRASH

pcscd would crash sometimes when a reader was unplugged on Solaris 10
or would not recognize the reader if it was plugged back in. The problem
appears to be that once the libusb returns errno == ENODEV no further
calls should be made other then to usb_close.

The attached patch enodev.1.43.11-svn-4750.txt in ccid_usb.c defines
errno_enodev for each reader. If any of the usb_* calls return ENODEV
errno_enodev will be set. Any further read, write or control operations
to the device will not be attempted.

The ifdhandler.c was also modified to remove the CmdPowerOff call, since
this can be called via HPRemoveHotPlugable->RFRemoveReader->
RFUninitializingReader->IFDCloseIFD->(ccid)IFDCloseChannel

This path leads to trying to power off a reader that is not present,
and can cause a crash.

There is comment in RFUninitializeReader

    /*
     * If the reader is getting uninitialized then it is being unplugged
     * so I can't send a IFDPowerICC call to it
     *
     * IFDPowerICC( rContext, IFD_POWER_DOWN, Atr, &AtrLen );
     */

If it was not a good idea to call IFDPowerICC here, it is not a
good idea to call it from IFDCloseChannel either. i.e. if the
hotplug_libusb.c says the reader is not there, no operations should be
attempted. (There may be some other code path where IFDCloseIFD
should call IFDPowerICC, but I did not see that. I also did not see
and easy way set the errno_enodev to avoid this call either.)


With these changes I can plug and unplug readers, without a crash,
and with out losing the reader.

I may have missed something, but it looks like these changes should
work for any OS.


--

 Douglas E. Engert  <[email protected]>
 Argonne National Laboratory
 9700 South Cass Avenue
 Argonne, Illinois  60439
 (630) 252-5444
--- ./src/,misc.h       Wed Nov 18 10:32:33 2009
+++ ./src/misc.h        Tue Feb 16 15:42:19 2010
@@ -24,9 +24,13 @@
  * see 
http://gcc.gnu.org/onlinedocs/gcc-3.3.5/gcc/Function-Attributes.html#Function-Attributes
  * see http://www.nedprod.com/programs/gccvisibility.html
  */
-#if defined __GNUC__ && (__GNUC__ >= 4 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 
3))
+#if defined __GNUC__ && (! defined (__sun)) && (__GNUC__ >= 4 || (__GNUC__ == 
3 && __GNUC_MINOR__ >= 3))
 #define INTERNAL __attribute__ ((visibility("hidden")))
 #define PCSC_API __attribute__ ((visibility("default")))
+#elif (! defined __GNUC__ ) && defined (__sun) 
+/* 
http://wikis.sun.com/display/SunStudio/Macros+for+Shared+Library+Symbol+Visibility
 */
+#define INTERNAL __hidden
+#define PCSC_API __global
 #else
 #define INTERNAL
 #define PCSC_API
--- ./src/,ccid_usb.c   Tue Feb  9 15:31:38 2010
+++ ./src/ccid_usb.c    Fri Feb 19 11:12:00 2010
@@ -62,6 +62,7 @@
        char *dirname;
        char *filename;
        int interface;
+       int errno_enodev;
 
        /*
         * Endpoints
@@ -524,6 +525,7 @@
                                        usbDevice[reader_index].handle = 
dev_handle;
                                        usbDevice[reader_index].dirname = 
strdup(bus->dirname);
                                        usbDevice[reader_index].filename = 
strdup(dev->filename);
+                                       usbDevice[reader_index].errno_enodev = 
0;
                                        usbDevice[reader_index].interface = 
interface;
                                        
usbDevice[reader_index].real_nb_opened_slots = 1;
                                        usbDevice[reader_index].nb_opened_slots 
= &usbDevice[reader_index].real_nb_opened_slots;
@@ -584,6 +586,13 @@
 
        DEBUG_XXD(debug_header, buffer, length);
 
+       /* If we previously received errno == ENODEV  return failed */
+       if (usbDevice[reader_index].errno_enodev == 1) 
+       {
+               DEBUG_CRITICAL("WriteUSB errno_enodev == 1 %d");
+               return STATUS_NO_SUCH_DEVICE;
+       }
+
        rv = usb_bulk_write(usbDevice[reader_index].handle,
                usbDevice[reader_index].bulk_out, (char *)buffer, length,
                USB_WRITE_TIMEOUT);
@@ -595,7 +604,11 @@
                        usb_strerror());
 
                if (ENODEV == errno)
+               {
+                       usbDevice[reader_index].errno_enodev = 1;
+                       DEBUG_CRITICAL("usb_bulk_write errno == ENODEV");
                        return STATUS_NO_SUCH_DEVICE;
+               }
 
                return STATUS_UNSUCCESSFUL;
        }
@@ -621,6 +634,13 @@
        (void)snprintf(debug_header, sizeof(debug_header), "<- %06X ",
                (int)reader_index);
 
+       /* If we previously received errno == ENODEV  return failed */
+       if (usbDevice[reader_index].errno_enodev == 1) 
+       {
+               DEBUG_CRITICAL("ReadUSB errno_enodev == 1 %d");
+               return STATUS_NO_SUCH_DEVICE;
+       }
+
        rv = usb_bulk_read(usbDevice[reader_index].handle,
                usbDevice[reader_index].bulk_in, (char *)buffer, *length,
                usbDevice[reader_index].ccid.readTimeout * 1000);
@@ -633,7 +653,11 @@
                        usb_strerror());
 
                if (ENODEV == errno)
+               {
+                       usbDevice[reader_index].errno_enodev = 1;
+                       DEBUG_CRITICAL("usb_bulk_read errno == ENODEV");
                        return STATUS_NO_SUCH_DEVICE;
+               }
 
                return STATUS_UNSUCCESSFUL;
        }
@@ -706,6 +730,7 @@
        usbDevice[reader_index].handle = NULL;
        usbDevice[reader_index].dirname = NULL;
        usbDevice[reader_index].filename = NULL;
+       usbDevice[reader_index].errno_enodev = 0;
        usbDevice[reader_index].interface = 0;
 
        return STATUS_SUCCESS;
@@ -950,9 +975,23 @@
        if (0 == (requesttype & 0x80))
                DEBUG_XXD("send: ", bytes, size);
 
+       /* If we previously received errno == ENODEV  return failed */
+       if (usbDevice[reader_index].errno_enodev == 1)
+       {
+               DEBUG_CRITICAL("ControlUSB errno_enodev == 1 %d");
+               return STATUS_NO_SUCH_DEVICE;
+       }
+
        ret = usb_control_msg(usbDevice[reader_index].handle, requesttype,
                request, value, usbDevice[reader_index].interface, (char 
*)bytes, size,
                usbDevice[reader_index].ccid.readTimeout * 1000);
+
+       if (ENODEV == errno)
+       {
+               usbDevice[reader_index].errno_enodev = 1;
+               DEBUG_CRITICAL("usb_control_msg errno == ENODEV");
+               return STATUS_NO_SUCH_DEVICE;
+       }
 
        if (requesttype & 0x80)
                DEBUG_XXD("receive: ", bytes, ret);
--- ./src/,ifdhandler.c Fri Jan  8 07:59:27 2010
+++ ./src/ifdhandler.c  Wed Feb 17 16:35:12 2010
@@ -269,7 +269,7 @@
         * No need to wait too long if the reader disapeared */
        get_ccid_descriptor(reader_index)->readTimeout = 
DEFAULT_COM_READ_TIMEOUT;
 
-       (void)CmdPowerOff(reader_index);
+//DEE  (void)CmdPowerOff(reader_index);
        /* No reader status check, if it failed, what can you do ? :) */
 
 #ifdef HAVE_PTHREAD

--- End Message ---
_______________________________________________
Muscle mailing list
[email protected]
http://lists.drizzle.com/mailman/listinfo/muscle

Reply via email to