Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OpenUSBByName(): the device bus & addr must match #10

Closed
wants to merge 1 commit into from
Closed

OpenUSBByName(): the device bus & addr must match #10

wants to merge 1 commit into from

Conversation

santigimeno
Copy link
Contributor

In Linux, before this change, if there were multiple card readers of the same
model connected, a new Reader could be initialized by opening the wrong USB
device.

@santigimeno
OpenUSBByName(): the device bus & addr must match
a67c50c 
In Linux, before this change, if there were multiple card readers of the same
model connected, a new Reader could be initialized by opening the wrong USB
device.
@santigimeno
Copy link
Contributor Author

This patch fixes the issue for me in Debian Jessie. Not sure if it'll work for other environments.
I've assumed there can't be a bus_number and device_addr equal to 0. I'm not sure about it either.

@LudovicRousseau
Copy link
Owner

Fixed in 8b9f6f3*
thanks

@santigimeno santigimeno deleted the my_fix branch November 10, 2015 09:28
@santigimeno
Copy link
Contributor Author

Thanks @LudovicRousseau !

I've one question. I don't know what's your policy regarding security fixes in general, and specifically in Debian. In the event this bug were considered a "security bug" could it be added to a new version of libccid in the stable distribution (jessie)?

@LudovicRousseau
Copy link
Owner

I don't see how this bug can be a security issue.

To get an updated version in Debian stable the easiest may be to use backport http:https://backports.debian.org/ or build a new package yourself.

@santigimeno
Copy link
Contributor Author

@LudovicRousseau I understand it might not be. Let me explain my case: I have some boxes with 2 readers of the same model. I'm able to differentiate them by their SerialId. I allow certain operations on them depending on this id. This bug was causing that some operations were being allowed in the wrong reader. This is a security issue for my application. Of course I don't know this can be considered a security issue in the library itself.

I didn't know about the debian backports website. Thanks for the info!

LudovicRousseau added a commit that referenced this pull request Nov 13, 2015
@LudovicRousseau
CreateChannelByNameOrChannel(): call FreeChannel() in case of error
522ebb2 
If the creation of a channel fails then call FreeChannel() instead of
IFDHCloseChannel().
The ressources are unalocated but with no access at the lower layer.

Since the creation failed the channel is not correctly created and
only part of the reader stucture is initialised.

The problem was discovered with the folowing problem:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff6fe2700 (LWP 6120)]
0x00007ffff5dca8f2 in CmdPowerOff (reader_index=0) at commands.c:1076
1076		cmd[6] = (*ccid_descriptor->pbSeq)++;
(gdb) print ccid_descriptor
$1 = (_ccid_descriptor *) 0x7ffff5fe0468 <usbDevice+40>
(gdb) print ccid_descriptor->pbSeq
$2 = (unsigned char *) 0x0

The pbSeq pointer is NULL and can't be dereferenced.

(gdb) bt
 #0  0x00007ffff5dca8f2 in CmdPowerOff (reader_index=0) at commands.c:1076
 #1  0x00007ffff5dce556 in IFDHCloseChannel (Lun=0) at ifdhandler.c:269
 #2  0x00007ffff5dce773 in CreateChannelByNameOrChannel (Lun=0,
     lpcDevice=0x269 <error: Cannot access memory at address 0x269>, Channel=0)
     at ifdhandler.c:194
 #3  0x000000000040673a in IFDOpenIFD (rContext=rContext@entry=0x61e010)
     at ifdwrapper.c:136
 #4  0x0000000000408151 in RFInitializeReader (rContext=0x61e010)
     at readerfactory.c:1036
 #5  0x0000000000408a93 in RFAddReader (readerNameLong=<optimized out>,
     port=2097152, library=<optimized out>,
     device=0x7ffff6fe1d30 "usb:08e6/3437:libusb-1.0:1:2:0")
     at readerfactory.c:329
 #6  0x000000000040d9d4 in HPAddHotPluggable (dev=0x0,
     bus_device=0x61d660 <readerTracker+32> "", interface=0,
     driver=<optimized out>, driver=<optimized out>, desc=...)
     at hotplug_libusb.c:608
 #7  0x000000000040dd2a in HPRescanUsbBus () at hotplug_libusb.c:373
 #8  0x000000000040dfa1 in HPEstablishUSBNotifications (pipefd=0x7fffffffe440)
     at hotplug_libusb.c:429
 #9  0x00007ffff75a30a4 in start_thread (arg=0x7ffff6fe2700)
     at pthread_create.c:309
 #10 0x00007ffff72d804d in clone ()
     at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
LudovicRousseau added a commit that referenced this pull request Oct 23, 2023
@LudovicRousseau
Fix thread safe issue
765b338 
Declare ReaderIndex[] as _Atomic as it can be accessed from 2 different
threads.

==================
WARNING: ThreadSanitizer: data race (pid=4017)
  Write of size 4 at 0x7fd1b32be970 by main thread (mutexes: write M0, write M1):
    #0 ReleaseReaderIndex CCID/src/utils.c:81:21 (libccid.so+0xa672) (BuildId: 69b3a74d7af87f05c5edb71935a43b024a0fe968)
    #1 FreeChannel CCID/src/ifdhandler.c:92:2 (libccid.so+0xa672)
    #2 IFDHCloseChannel CCID/src/ifdhandler.c:302:2 (libccid.so+0xa672)
    #3 IFDCloseIFD PCSC/src/ifdwrapper.c:190:7 (pcscd+0xd9e30) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #4 RFUnInitializeReader PCSC/src/readerfactory.c:1153:9 (pcscd+0xd9e30)
    #5 removeReader PCSC/src/readerfactory.c:645:2 (pcscd+0xd9e30)
    #6 _UnrefReader PCSC/src/readerfactory.c:120:3 (pcscd+0xd9e30)
    #7 IFDCloseIFD PCSC/src/ifdwrapper.c:190:7 (pcscd+0xd9e30) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #8 RFUnInitializeReader PCSC/src/readerfactory.c:1153:9 (pcscd+0xd9e30)
    #9 removeReader PCSC/src/readerfactory.c:645:2 (pcscd+0xd9e30)
    #10 _UnrefReader PCSC/src/readerfactory.c:120:3 (pcscd+0xd9e30)
    #11 RFRemoveReader PCSC/src/readerfactory.c:624:5 (pcscd+0xdd282) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #12 RFRemoveReader PCSC/src/readerfactory.c:624:5 (pcscd+0xdd282) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #13 RFRemoveReader PCSC/src/readerfactory.c:624:5 (pcscd+0xdd282) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #14 RFCleanupReaders PCSC/src/readerfactory.c:1396:9 (pcscd+0xddb9e) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #15 SVCServiceRunLoop PCSC/src/pcscdaemon.c:123:4 (pcscd+0xd9574) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #16 main PCSC/src/pcscdaemon.c:801:2 (pcscd+0xd8810) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)

  Previous read of size 4 at 0x7fd1b32be970 by thread T7:
    #0 LunToReaderIndex CCID/src/utils.c:72:14 (libccid.so+0xf8aa) (BuildId: 69b3a74d7af87f05c5edb71935a43b024a0fe968)
    #1 IFDHPolling CCID/src/ifdhandler.c:313:28 (libccid.so+0xb1bf) (BuildId: 69b3a74d7af87f05c5edb71935a43b024a0fe968)
    #2 EHStatusHandlerThread PCSC/src/eventhandler.c:467:10 (pcscd+0xd6f9b) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)

  As if synchronized via sleep:
    #0 nanosleep <null> (pcscd+0x515ed) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #1 SYS_Sleep PCSC/src/sys_unix.c:69:9 (pcscd+0xdfbf9) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #2 SVCServiceRunLoop PCSC/src/pcscdaemon.c:117:10 (pcscd+0xd956a) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #3 main PCSC/src/pcscdaemon.c:801:2 (pcscd+0xd8810) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)

  Location is global 'ReaderIndex' of size 64 at 0x7fd1b32be970 (libccid.so+0x22970)

  Mutex M0 (0x7b0c00000ed0) created at:
    #0 pthread_mutex_init <null> (pcscd+0x555cf) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #1 RFAddReader PCSC/src/readerfactory.c:355:9 (pcscd+0xdb4b7) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #2 HPAddDevice PCSC/src/hotplug_libudev.c:512:8 (pcscd+0xe3409) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #3 HPScanUSB PCSC/src/hotplug_libudev.c:579:3 (pcscd+0xe2a1d) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #4 HPRegisterForHotplugEvents PCSC/src/hotplug_libudev.c:761:2 (pcscd+0xe2a1d)
    #5 main PCSC/src/pcscdaemon.c:768:7 (pcscd+0xd8717) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)

  Mutex M1 (0x7fd1b32be948) created at:
    #0 pthread_mutex_lock <null> (pcscd+0x71ada) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #1 CreateChannelByNameOrChannel CCID/src/ifdhandler.c:119:8 (libccid.so+0x82ae) (BuildId: 69b3a74d7af87f05c5edb71935a43b024a0fe968)
    #2 IFDHCreateChannelByName CCID/src/ifdhandler.c:233:9 (libccid.so+0x7e55) (BuildId: 69b3a74d7af87f05c5edb71935a43b024a0fe968)
    #3 IFDOpenIFD PCSC/src/ifdwrapper.c:136:9 (pcscd+0xdd0cb) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #4 RFInitializeReader PCSC/src/readerfactory.c:1121:8 (pcscd+0xdd0cb)
    #5 RFAddReader PCSC/src/readerfactory.c:366:7 (pcscd+0xdb52a) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #6 RFAddReader PCSC/src/readerfactory.c:366:7 (pcscd+0xdb52a) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #7 HPAddDevice PCSC/src/hotplug_libudev.c:512:8 (pcscd+0xe3409) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #8 HPScanUSB PCSC/src/hotplug_libudev.c:579:3 (pcscd+0xe2a1d) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #9 HPRegisterForHotplugEvents PCSC/src/hotplug_libudev.c:761:2 (pcscd+0xe2a1d)
    #10 main PCSC/src/pcscdaemon.c:768:7 (pcscd+0xd8717) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)

  Thread T7 (tid=4049, running) created by thread T4 at:
    #0 pthread_create <null> (pcscd+0x53dfd) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #1 ThreadCreate PCSC/src/utils.c:184:8 (pcscd+0xe3cab) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #2 EHSpawnEventHandler PCSC/src/eventhandler.c:233:7 (pcscd+0xd6840) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #3 RFAddReader PCSC/src/readerfactory.c:397:8 (pcscd+0xdb6ba) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #4 HPAddDevice PCSC/src/hotplug_libudev.c:512:8 (pcscd+0xe3409) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)
    #5 HPEstablishUSBNotifications PCSC/src/hotplug_libudev.c:646:6 (pcscd+0xe3783) (BuildId: 046d0874ce19c882d4f3b8ab5213aa4cd336cd5e)

SUMMARY: ThreadSanitizer: data race CCID/src/utils.c:81:21 in ReleaseReaderIndex
==================
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@santigimeno @LudovicRousseau