On Feb 22, 2010, at 7:37 AM, Douglas E. Engert wrote: > 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. >
It is interesting, and something that will be addressed as the supported platforms intersect with this problem. We've seen some memory-corruption related crashing in libusb with PC/SC-lite, but since memory corruption can originate out of scope of where symptoms manifest we haven't been able to blame libusb, although we've run memory diagnostics on the live code at various times. We have our plate full right now dealing with other hot issues with supported combinations of components and those have to take priority, along with other porting work we're doing. One of the issues we're working with now, where libusb isn't directly involved (no CCID card use anyway) is showing an elusive problem with what looks like a double-free or memory smash and we're trying to track it down, but so far can't reproduce the problem locally. It is possible that it is related to what you're seeing. If it is, when we fix that, the problem you're finding may be simultaneously resolved. Anyway, it may be a little while before we can focus on the problem you're working with, but not due to disinterest. Paul > >> 2010/2/19 Douglas E. Engert <deengert at anl.gov>: >>> > 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 <DEEngert at anl.gov> > Argonne National Laboratory > 9700 South Cass Avenue > Argonne, Illinois 60439 > (630) 252-5444 > > From: "Douglas E. Engert" <deengert at anl.gov> > Date: February 19, 2010 11:44:13 AM PST > To: MUSCLE <muscle at lists.musclecard.com> > Cc: Kevin Reinholz <kreinholz at gmail.com> > Subject: Re: [Muscle] new BETA versions of pcsc-lite and libccid > > > 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 <DEEngert at anl.gov> > 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 > > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/crypto-discuss/attachments/20100222/4fa5634c/attachment-0001.html>
