misc SunOS patches #7
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LudovicRousseau
pushed a commit
that referenced
this pull request
Dec 15, 2021
Problem:
The thread running EHStatusHandlerThread() will race all the
rContext->readerState->fields as well as the powerState and some other
flags. These flags are both consumed and set by the thread running the
ccid driver as well.
Solution:
Make the flow unidirectional*: the status thread will always set its own
copy and "stash" a unique copy ready to be consumed by the driver
thread. That copy is consumed and rcontext->readerState is updated in
RFCheckReaderStatus() and a few other functions in readerfactory.c.
Whenever a copy is consumed it is also freed. Note that even though this
adds dynamic allocation it's at most 2 instances of READER_STATE.
This PR uses the C11 _Atomic feature. I'm not very familiar with all the
compilers pcsc is built with, but if you deem this to be incompatible,
please review the overall approach and I can #ifdef and do some mutexes
for platforms which do not provide _Atomic
* the flow is unidirectional except for the readerName which is logged
at the beginning of the status thread. That should not be a problem
since the name is set from the driver thread before the status thread
is spawned.
==================
WARNING: ThreadSanitizer: data race (pid=28786)
Write of size 4 at 0x000000f55c10 by thread T3:
#0 EHStatusHandlerThread <null> (pcscd+0x4c3aed)
Previous read of size 4 at 0x000000f55c10 by main thread:
#0 RFWaitForReaderInit <null> (pcscd+0x4d0e27)
#1 main <null> (pcscd+0x4c70c6)
Location is global 'readerStates' of size 2944 at 0x000000f55b60 (pcscd+0x000000f55c10)
Thread T3 (tid=28790, running) created by main thread at:
#0 pthread_create <null> (pcscd+0x42be9b)
#1 ThreadCreate <null> (pcscd+0x4e1c79)
#2 EHSpawnEventHandler <null> (pcscd+0x4c2e9f)
#3 RFAddReader <null> (pcscd+0x4cac07)
#4 HPAddDevice <null> (pcscd+0x4e0a2e)
#5 HPScanUSB <null> (pcscd+0x4dfe65)
#6 HPRegisterForHotplugEvents <null> (pcscd+0x4dfd3a)
#7 main <null> (pcscd+0x4c705c)
SUMMARY: ThreadSanitizer: data race (/home/rousseau/sc/costa/PCSC/src/pcscd+0x4c3aed) in EHStatusHandlerThread
==================
LudovicRousseau
added a commit
that referenced
this pull request
Nov 19, 2023
The field .cardAtrLength is used to know if a card is inserted and is
accessed from different threads.
We define it as _Atomic to have atomic accesses to the field and fix a
TSAN warning:
==================
WARNING: ThreadSanitizer: data race (pid=14987)
Read of size 4 at 0x557bacd39cb0 by main thread:
#0 RFWaitForReaderInit PCSC/src/readerfactory.c:1430:43 (pcscd+0xdda24) (BuildId: b43179fc4c418df0dad65d97dc7a0d9cc226d42f)
#1 main PCSC/src/pcscdaemon.c:773:2 (pcscd+0xd87d4) (BuildId: b43179fc4c418df0dad65d97dc7a0d9cc226d42f)
Previous write of size 8 at 0x557bacd39cb0 by thread T3:
#0 EHStatusHandlerThread PCSC/src/eventhandler.c:314:40 (pcscd+0xd6a47) (BuildId: b43179fc4c418df0dad65d97dc7a0d9cc226d42f)
Location is global 'readerStates' of size 2944 at 0x557bacd39c00 (pcscd+0x1496cb0)
Thread T3 (tid=14992, running) created by main thread at:
#0 pthread_create <null> (pcscd+0x53dfd) (BuildId: b43179fc4c418df0dad65d97dc7a0d9cc226d42f)
#1 ThreadCreate PCSC/src/utils.c:184:8 (pcscd+0xe38cb) (BuildId: b43179fc4c418df0dad65d97dc7a0d9cc226d42f)
#2 EHSpawnEventHandler PCSC/src/eventhandler.c:233:7 (pcscd+0xd6960) (BuildId: b43179fc4c418df0dad65d97dc7a0d9cc226d42f)
#3 RFAddReader PCSC/src/readerfactory.c:397:8 (pcscd+0xdb632) (BuildId: b43179fc4c418df0dad65d97dc7a0d9cc226d42f)
#4 HPAddDevice PCSC/src/hotplug_libudev.c:512:8 (pcscd+0xe3029) (BuildId: b43179fc4c418df0dad65d97dc7a0d9cc226d42f)
#5 HPScanUSB PCSC/src/hotplug_libudev.c:579:3 (pcscd+0xe263d) (BuildId: b43179fc4c418df0dad65d97dc7a0d9cc226d42f)
#6 HPRegisterForHotplugEvents PCSC/src/hotplug_libudev.c:761:2 (pcscd+0xe263d)
#7 main PCSC/src/pcscdaemon.c:766:7 (pcscd+0xd87c7) (BuildId: b43179fc4c418df0dad65d97dc7a0d9cc226d42f)
SUMMARY: ThreadSanitizer: data race PCSC/src/readerfactory.c:1430:43 in RFWaitForReaderInit
==================
Thanks to Stefan Ehmann for the bug report.
LudovicRousseau
added a commit
that referenced
this pull request
Nov 19, 2023
The struct timeval used in log_line() can't be accessed in a atomic way
so we should protect it using a mutex.
==================
WARNING: ThreadSanitizer: data race (pid=5391)
Read of size 8 at 0x55b3bba7b950 by thread T3 (mutexes: write M17):
#0 log_line /PCSC-debug/src/debuglog.c:217 (pcscd+0x9d7e)
#1 log_msg /PCSC-debug/src/debuglog.c:148 (pcscd+0x9a85)
#2 IFDHPowerICC <null> (libccid.so+0x8a54)
#3 EHStatusHandlerThread /PCSC-debug/src/eventhandler.c:395 (pcscd+0xbafe)
Previous write of size 8 at 0x55b3bba7b950 by main thread:
#0 log_line /PCSC-debug/src/debuglog.c:232 (pcscd+0x9e77)
#1 log_msg /PCSC-debug/src/debuglog.c:148 (pcscd+0x9a85)
#2 get_driver /PCSC-debug/src/hotplug_libudev.c:298 (pcscd+0x1f75d)
#3 HPAddDevice /PCSC-debug/src/hotplug_libudev.c:394 (pcscd+0x1ff32)
#4 HPScanUSB /PCSC-debug/src/hotplug_libudev.c:579 (pcscd+0x209db)
#5 HPRegisterForHotplugEvents /PCSC-debug/src/hotplug_libudev.c:761
(pcscd+0x21255)
#6 main /PCSC-debug/src/pcscdaemon.c:766 (pcscd+0xe6fa)
Location is global 'last_time.3' of size 16 at 0x55b3bba7b950
(pcscd+0x35950)
Mutex M17 (0x7b0c000014d0) created at:
#0 pthread_mutex_init ../../../../src/libsanitizer/tsan/
tsan_interceptors_posix.cpp:1295 (libtsan.so.2+0x50468)
#1 RFAddReader /PCSC-debug/src/readerfactory.c:355 (pcscd+0x10b8c)
#2 HPAddDevice /PCSC-debug/src/hotplug_libudev.c:512 (pcscd+0x205b0)
#3 HPScanUSB /PCSC-debug/src/hotplug_libudev.c:579 (pcscd+0x209db)
#4 HPRegisterForHotplugEvents /PCSC-debug/src/hotplug_libudev.c:761
(pcscd+0x21255)
#5 main /PCSC-debug/src/pcscdaemon.c:766 (pcscd+0xe6fa)
Thread T3 (tid=5395, running) created by main thread at:
#0 pthread_create ../../../../src/libsanitizer/tsan/
tsan_interceptors_posix.cpp:1001 (libtsan.so.2+0x5e686)
#1 ThreadCreate /PCSC-debug/src/utils.c:184 (pcscd+0x21755)
#2 EHSpawnEventHandler /PCSC-debug/src/eventhandler.c:233 (pcscd+0xb2d9)
#3 RFAddReader /PCSC-debug/src/readerfactory.c:397 (pcscd+0x10f1a)
#4 HPAddDevice /PCSC-debug/src/hotplug_libudev.c:512 (pcscd+0x205b0)
#5 HPScanUSB /PCSC-debug/src/hotplug_libudev.c:579 (pcscd+0x209db)
#6 HPRegisterForHotplugEvents /PCSC-debug/src/hotplug_libudev.c:761
(pcscd+0x21255)
#7 main /PCSC-debug/src/pcscdaemon.c:766 (pcscd+0xe6fa)
SUMMARY: ThreadSanitizer: data race /PCSC-debug/src/debuglog.c:217 in log_line
==================
Thanks to Stefan Ehmann for the bug report and patch.
LudovicRousseau
added a commit
that referenced
this pull request
Nov 19, 2023
The field .vHandle is used to check the validity of the reader.
It is accessed in write mode in RFUnloadReader() from a thread A while
it is also accessed in read mode in RFWaitForReaderInit() from thread B.
==================
WARNING: ThreadSanitizer: data race (pid=23997)
Write of size 8 at 0x7b4c000002d8 by thread T5:
#0 RFUnloadReader PCSC/src/readerfactory.c:1005:20 (pcscd+0xda017) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
#1 RFUnInitializeReader PCSC/src/readerfactory.c:1151:8 (pcscd+0xda017)
#2 removeReader PCSC/src/readerfactory.c:645:2 (pcscd+0xda017)
#3 _UnrefReader PCSC/src/readerfactory.c:120:3 (pcscd+0xda017)
#4 RFRemoveReader PCSC/src/readerfactory.c:624:5 (pcscd+0xdd11b) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
#5 RFRemoveReader PCSC/src/readerfactory.c:624:5 (pcscd+0xdd11b) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
#6 HPRemoveDevice PCSC/src/hotplug_libudev.c:348:4 (pcscd+0xe35af) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
#7 HPEstablishUSBNotifications PCSC/src/hotplug_libudev.c:640:6 (pcscd+0xe35af)
Previous read of size 8 at 0x7b4c000002d8 by main thread:
#0 RFWaitForReaderInit PCSC/src/readerfactory.c:1426:29 (pcscd+0xddabe) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
#1 main PCSC/src/pcscdaemon.c:773:2 (pcscd+0xd8864) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
Location is heap block of size 400 at 0x7b4c000001c0 allocated by main thread:
#0 malloc <null> (pcscd+0x52691) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
#1 RFAllocateReaderSpace PCSC/src/readerfactory.c:135:25 (pcscd+0xda3b9) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
#2 main PCSC/src/pcscdaemon.c:643:7 (pcscd+0xd85a5) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
Thread T5 (tid=24004, running) created by main thread at:
#0 pthread_create <null> (pcscd+0x53dfd) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
#1 ThreadCreate PCSC/src/utils.c:184:8 (pcscd+0xe39bb) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
#2 HPRegisterForHotplugEvents PCSC/src/hotplug_libudev.c:763:6 (pcscd+0xe2768) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
#3 main PCSC/src/pcscdaemon.c:766:7 (pcscd+0xd8857) (BuildId: db0b27e1c3b409153327b14d9e501205ed34fb6e)
SUMMARY: ThreadSanitizer: data race PCSC/src/readerfactory.c:1005:20 in RFUnloadReader
==================
LudovicRousseau
added a commit
that referenced
this pull request
Nov 19, 2023
The field .readerSharing can be used from different theads at the same
time.
We define it as _Atomic to have atomic accesses to the field and fix a
TSAN warning:
==================
WARNING: ThreadSanitizer: data race (pid=63123)
Write of size 4 at 0x5575884d8cb8 by thread T3:
#0 EHStatusHandlerThread PCSC/src/eventhandler.c:449:41 (pcscd+0xd6f12) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
Previous write of size 4 at 0x5575884d8cb8 by thread T8:
#0 SCardDisconnect PCSC/src/winscard.c:1034:39 (pcscd+0xe4779) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#1 ContextThread PCSC/src/winscard_svc.c:575:16 (pcscd+0xe6507) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
As if synchronized via sleep:
#0 nanosleep <null> (pcscd+0x515ed) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#1 SYS_USleep PCSC/src/sys_unix.c:87:9 (pcscd+0xd6fca) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#2 EHStatusHandlerThread PCSC/src/eventhandler.c (pcscd+0xd6fca)
#3 __tsan_thread_start_func <null> (pcscd+0x53d66) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
Location is global 'readerStates' of size 2944 at 0x5575884d8c30 (pcscd+0x1497cb8)
Thread T3 (tid=63127, running) created by main thread at:
#0 pthread_create <null> (pcscd+0x53dfd) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#1 ThreadCreate PCSC/src/utils.c:184:8 (pcscd+0xe3cab) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#2 EHSpawnEventHandler PCSC/src/eventhandler.c:233:7 (pcscd+0xd6840) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#3 RFAddReader PCSC/src/readerfactory.c:397:8 (pcscd+0xdb6dd) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#4 HPAddDevice PCSC/src/hotplug_libudev.c:512:8 (pcscd+0xe3409) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#5 HPScanUSB PCSC/src/hotplug_libudev.c:579:3 (pcscd+0xe2a1d) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#6 HPRegisterForHotplugEvents PCSC/src/hotplug_libudev.c:761:2 (pcscd+0xe2a1d)
#7 main PCSC/src/pcscdaemon.c:768:7 (pcscd+0xd8717) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
Thread T8 (tid=63221, finished) created by main thread at:
#0 pthread_create <null> (pcscd+0x53dfd) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#1 ThreadCreate PCSC/src/utils.c:184:8 (pcscd+0xe3cab) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#2 CreateContextThread PCSC/src/winscard_svc.c:256:7 (pcscd+0xd924e) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#3 SVCServiceRunLoop PCSC/src/pcscdaemon.c:133:9 (pcscd+0xd924e)
#4 main PCSC/src/pcscdaemon.c:801:2 (pcscd+0xd8810) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
#5 main PCSC/src/pcscdaemon.c:801:2 (pcscd+0xd8810) (BuildId: b2f519c8807f458b6282d631fe17086d4f6da420)
SUMMARY: ThreadSanitizer: data race PCSC/src/eventhandler.c:449:41 in EHStatusHandlerThread
==================
LudovicRousseau
added a commit
that referenced
this pull request
Nov 19, 2023
The field .LockCount can be used from different theads at the same
time.
We define it as _Atomic to have atomic accesses to the field and fix a
TSAN warning:
==================
WARNING: ThreadSanitizer: data race (pid=64069)
Write of size 4 at 0x7b4c00000138 by thread T6 (mutexes: write M0):
#0 RFUnlockAllSharing PCSC/src/readerfactory.c:1080:23 (pcscd+0xe3f15) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#1 SCardDisconnect PCSC/src/winscard.c:863:7 (pcscd+0xe3f15)
#2 ContextThread PCSC/src/winscard_svc.c:575:16 (pcscd+0xe64f7) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#3 ContextThread PCSC/src/winscard_svc.c:575:16 (pcscd+0xe64f7) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
Previous write of size 4 at 0x7b4c00000138 by thread T3:
#0 RFSetReaderEventState PCSC/src/readerfactory.c:1304:23 (pcscd+0xdd8f2) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#1 EHStatusHandlerThread PCSC/src/eventhandler.c:362:11 (pcscd+0xd6c9c) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
Location is heap block of size 400 at 0x7b4c00000000 allocated by main thread:
#0 malloc <null> (pcscd+0x52691) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#1 RFAllocateReaderSpace PCSC/src/readerfactory.c:135:25 (pcscd+0xda2e9) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#2 main PCSC/src/pcscdaemon.c:645:7 (pcscd+0xd8465) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
Mutex M0 (0x55dc20366908) created at:
#0 pthread_mutex_lock <null> (pcscd+0x71ada) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#1 RFUnlockAllSharing PCSC/src/readerfactory.c:1076:8 (pcscd+0xe3ecf) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#2 SCardDisconnect PCSC/src/winscard.c:863:7 (pcscd+0xe3ecf)
#3 ContextThread PCSC/src/winscard_svc.c:575:16 (pcscd+0xe64f7) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#4 ContextThread PCSC/src/winscard_svc.c:575:16 (pcscd+0xe64f7) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
Thread T6 (tid=64077, running) created by main thread at:
#0 pthread_create <null> (pcscd+0x53dfd) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#1 ThreadCreate PCSC/src/utils.c:184:8 (pcscd+0xe3c9b) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#2 CreateContextThread PCSC/src/winscard_svc.c:256:7 (pcscd+0xd924e) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#3 SVCServiceRunLoop PCSC/src/pcscdaemon.c:133:9 (pcscd+0xd924e)
#4 main PCSC/src/pcscdaemon.c:801:2 (pcscd+0xd8810) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#5 main PCSC/src/pcscdaemon.c:801:2 (pcscd+0xd8810) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
Thread T3 (tid=64073, running) created by main thread at:
#0 pthread_create <null> (pcscd+0x53dfd) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#1 ThreadCreate PCSC/src/utils.c:184:8 (pcscd+0xe3c9b) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#2 EHSpawnEventHandler PCSC/src/eventhandler.c:233:7 (pcscd+0xd6840) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#3 RFAddReader PCSC/src/readerfactory.c:397:8 (pcscd+0xdb6cd) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#4 HPAddDevice PCSC/src/hotplug_libudev.c:512:8 (pcscd+0xe33f9) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#5 HPScanUSB PCSC/src/hotplug_libudev.c:579:3 (pcscd+0xe2a0d) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
#6 HPRegisterForHotplugEvents PCSC/src/hotplug_libudev.c:761:2 (pcscd+0xe2a0d)
#7 main PCSC/src/pcscdaemon.c:768:7 (pcscd+0xd8717) (BuildId: ca24a31c13ced4613b52730b820229170aba90d6)
SUMMARY: ThreadSanitizer: data race PCSC/src/readerfactory.c:1080:23 in RFUnlockAllSharing
==================
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.