[Pcsclite-muscle] pcsc_stringify_error thread safety

[Ludovic Rousseau]

Hello,

2017-01-18 11:29 GMT+01:00 Nikos Mavrogiannopoulos <nmav at redhat.com>:

> 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).

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?

Bye

--
 Dr. Ludovic Rousseau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/pcsclite-muscle/attachments/20170118/251d46d8/attachment.html>