[PATCH] Async signal safe TLS accesses

[Paul Pluzhnikov]

On Thu, Jan 9, 2014 at 8:42 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:

> Maybe have the signal handler outside the loaded module call the function
> from the loaded module, but with sem_post in the function outside the
> module?  I haven't tested whether this fixes the powerpc problem, but it
> should avoid the identified race with the module being dlclosed while code
> from it is executing.  (There would of course then be the need to have
> memory barriers to ensure the current pointer obtained from dlsym is
> available from the thread calling the signal handler - and it would be
> necessary to ensure that the test does still show up the non-signal-safety
> if run with older glibc.)

Could you please test attached proposed fix?

It passes on x86_64 (ran 1000 times), and immediately self-deadlocks using
system (i.e. older) glibc-2.15 like so:

(gdb) bt
#0  __lll_lock_wait_private () at
../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:93
#1  0x00007f2dc9adf231 in _L_lock_10574 () at malloc.c:5241
#2  0x00007f2dc9adcf87 in __GI___libc_malloc (bytes=139833800392736)
at malloc.c:2921
#3  0x00007f2dca23c365 in allocate_and_init (map=0xc78bb0) at dl-tls.c:526
#4  tls_get_addr_tail (dtv=<optimized out>, the_map=0xc78bb0,
module=<optimized out>) at dl-tls.c:739
#5  0x00007f2dca24d5d0 in __tls_get_addr (ti=0x7f2dc524ffb0) at dl-tls.c:764
#6  0x00007f2dc504f870 in action (signo=<optimized out>,
info=0x7f2dc9a58b30, ignored=<optimized out>) at tst-tls7mod.c:32
#7  0x0000000000401913 in action (signo=<optimized out>,
info=<optimized out>, ignored=<optimized out>) at tst-tls7.c:53
#8  <signal handler called>
#9  __memset_sse2 () at ../sysdeps/x86_64/multiarch/../memset.S:44
#10 0x00007f2dc9ad9f6a in _int_free (av=0x7f2d98000020,
p=0x7f2d980008b0, have_lock=0) at malloc.c:4101
#11 0x000000000040174a in spin (ignored=<optimized out>) at tst-tls7.c:37
#12 0x00007f2dc9e21e9a in start_thread (arg=0x7f2dc9a59700) at
pthread_create.c:308
#13 0x00007f2dc9b4e3fd in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:112


Thanks,
-- 
Paul Pluzhnikov
-------------- next part --------------
diff --git a/nptl/tst-tls7.c b/nptl/tst-tls7.c
index 3bb3f7d..583d5b4 100644
--- a/nptl/tst-tls7.c
+++ b/nptl/tst-tls7.c
@@ -19,6 +19,8 @@
 /* This test checks that TLS in a dlopened object works when first accessed
    from a signal handler.  */
 
+#include <assert.h>
+#include <atomic.h>
 #include <dlfcn.h>
 #include <pthread.h>
 #include <semaphore.h>
@@ -39,6 +41,23 @@ spin (void *ignored)
   return NULL;
 }
 
+static void (*tls7mod_action) (int, siginfo_t *, void *);
+
+static void
+action (int signo, siginfo_t *info, void *ignored)
+{
+  sem_t *sem = info->si_value.sival_ptr;
+
+  atomic_read_barrier ();
+  assert (tls7mod_action != NULL);
+  (*tls7mod_action) (signo, info, ignored);
+
+  /* This sem_post may trigger dlclose, which will invalidate tls7mod_action.
+     It is important to do that only after tls7mod_action is no longer
+     active.  */
+  sem_post (sem);
+}
+
 int
 do_test (void)
 {
@@ -63,12 +82,13 @@ do_test (void)
           exit (1);
         }
 
-      void (*action) (int, siginfo_t *, void *) = dlsym (h, "action");
-      if (action == NULL)
+      tls7mod_action = dlsym (h, "action");
+      if (tls7mod_action == NULL)
         {
           puts ("dlsym for action failed");
           exit (1);
         }
+      atomic_write_barrier ();
 
       struct sigaction sa;
       sa.sa_sigaction = action;
@@ -105,6 +125,9 @@ do_test (void)
           }
         }
 
+      /* Paranoia.  */
+      tls7mod_action = NULL;
+
       if (dlclose (h))
         {
           puts ("dlclose failed");
diff --git a/nptl/tst-tls7mod.c b/nptl/tst-tls7mod.c
index aff29b9..da5af56 100644
--- a/nptl/tst-tls7mod.c
+++ b/nptl/tst-tls7mod.c
@@ -29,7 +29,6 @@ static __thread intptr_t tls_data = 0xdeadbeef;
 void
 action (int signo, siginfo_t *info, void *ignored)
 {
-  sem_t *sem = info->si_value.sival_ptr;
   if (tls_data != 0xdeadbeef)
     {
       write (STDOUT_FILENO, "wrong TLS value\n", 17);
@@ -38,6 +37,4 @@ action (int signo, siginfo_t *info, void *ignored)
 
   /* arbitrary choice, just write something unique-ish. */
   tls_data = (intptr_t) info;
-
-  sem_post (sem);
 }