Ludovic, Would you mind also extending the documentation for pcsc_stringify_error to mention that: 1. The returned string is valid only while the thread on which is was obtained is alive. 2. The returned string is valid until the next call to this function on the same thread.
Thanks, Maksim On Wed, Jan 18, 2017 at 5:24 PM, Ludovic Rousseau <[email protected]> wrote: > > > > 2017-01-18 14:41 GMT+01:00 Ludovic Rousseau <[email protected]>: >> >> Hello, >> >> 2017-01-18 11:29 GMT+01:00 Nikos Mavrogiannopoulos <[email protected]>: >>> >>> On Tue, 2017-01-17 at 20:33 +0100, Maksim Ivanov wrote: >>> >>> > The pcsc_stringify_error function in the PC/SC-Lite implementation >>> > uses a statically allocated buffer. This means that the buffer may be >>> > used simultaneously when the function is called from multiple threads >>> > concurrently. >>> > Therefore, the returned message may be spoiled, e.g.: >>> > "Internal error.ul" >>> > or >>> > "Command cancell" >>> > In the worst-case scenario, the application may read an unbounded >>> > string (with the terminating null character missing). >>> >>> A possible fix is attached. That avoids copying strings which are >>> constant on global store, and ensures that the static buffer is on >>> thread local store when possible. >>> >>> Except compilation, the fix is completely untested. >> >> >> A really simple fix is: >> --- /var/folders/jb/2mvc64nx74b76qjg_5yk8zs00000gn/T//zsNKq9_error.c >> 2017-01-18 14:37:19.000000000 +0100 >> +++ src/error.c 2017-01-17 22:20:08.000000000 +0100 >> @@ -76,7 +76,7 @@ PCSC_API char* pcsc_stringify_error(cons >> */ >> PCSC_API char* pcsc_stringify_error(const LONG pcscError) >> { >> - static char strError[75]; >> + __thread static char strError[75]; >> const char *msg = NULL; >> >> switch (pcscError) >> >> I tested it with success. >> >> It looks like __thread is standard and not GNU C specific. So maybe your >> test to limit its use to GCC is not needed and, in fact, problematic. >> According to https://en.wikipedia.org/wiki/Thread-local_storage it is >> supported by Solaris Studio C/C++, IBM XL C/C++, GNU C, Clang and Intel C++ >> Compiler (Linux systems). > > > Fixed in > https://github.com/LudovicRousseau/PCSC/commit/eab1d67295e4e1d5c12bbca77bc57c50fd384a4e > > Then the fix was fixed in > https://github.com/LudovicRousseau/PCSC/commit/9726152f2c6767f0fa3103ad307dc28ef7923852 > Clang accepts "__thread static" but GCC does not and only accepts "static > __thread". > >> Adding const to the pcsc_stringify_error() protottype is also a good idea. >> I don't think it would break existing compilation. Any comment on that? > > > Applied in > https://github.com/LudovicRousseau/PCSC/commit/b27f0e54194dcfb4db8179dceede8a649141fea4 > > Thansk to all the participants in this thread. > > Bye > > -- > Dr. Ludovic Rousseau > > _______________________________________________ > Pcsclite-muscle mailing list > [email protected] > http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle _______________________________________________ Pcsclite-muscle mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pcsclite-muscle
