Re: [PATCH][BZ #19329] Fix race between tls allocation at thread creation and dlopen

[Szabolcs Nagy <szabolcs dot nagy at arm dot com>]

On 06/01/16 18:33, Szabolcs Nagy wrote:
> At least the following objects may have conflicting access:
>
>    GL(dl_tls_max_dtv_idx)
>    GL(dl_tls_generation)
>    listp->slotinfo[i].map
>    listp->slotinfo[i].gen
>    listp->next
...
> I think various other accesses to the objects in the list above should
> be atomic as well, but that rabbit hole is deeper than what a quick
> fix could patch up.

i thought i can get away without making listp->next
access atomic, but now i see that can fail too

>     /* Check if the current dtv is big enough.   */
> -  if (dtv[-1].counter < GL(dl_tls_max_dtv_idx))
> +  dtv_slots = atomic_load_relaxed (&GL(dl_tls_max_dtv_idx));
> +  if (dtv[-1].counter < dtv_slots)
>       {
>         /* Resize the dtv.  */
>         dtv = _dl_resize_dtv (dtv);
> @@ -470,6 +473,7 @@ _dl_allocate_tls_init (void *result)
>        TLS.  For those which are dynamically loaded we add the values
>        indicating deferred allocation.  */
>     listp = GL(dl_tls_dtv_slotinfo_list);
> +  gen_count = atomic_load_acquire (&GL(dl_tls_generation));
>     while (1)
>       {
>         size_t cnt;
> @@ -480,18 +484,22 @@ _dl_allocate_tls_init (void *result)
>   	  void *dest;
>
>   	  /* Check for the total number of used slots.  */
> -	  if (total + cnt > GL(dl_tls_max_dtv_idx))
> +	  if (total + cnt > dtv_slots)
>   	    break;
....
>         total += cnt;
> -      if (total >= GL(dl_tls_max_dtv_idx))
> +      if (total >= dtv_slots)
>   	break;
>
>         listp = listp->next;

here.

a single listp node can hold 64 slotinfo entries, if more
libraries are loaded with tls then GL(dl_tls_max_dtv_idx) > 64,
but the updated listp->next pointer might not be visible here.

i will update the patch.