Message ID | 3237824fb057632817a8de508d1fcb1f2e6f5c7e.1618301209.git.szabolcs.nagy@arm.com |
---|---|
State | New |
Headers | show |
Series | Dynamic TLS related data race fixes | expand |
On 13/04/2021 05:18, Szabolcs Nagy via Libc-alpha wrote: > map is not valid to access here because it can be freed by a concurrent > dlclose: during tls access (via __tls_get_addr) _dl_update_slotinfo is > called without holding dlopen locks. So don't check the modid of map. > > The map == 0 and map != 0 code paths can be shared (avoiding the dtv > resize in case of map == 0 is just an optimization: larger dtv than > necessary would be fine too). > > -- > v2: > - commit message update LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > elf/dl-tls.c | 21 +++++---------------- > 1 file changed, 5 insertions(+), 16 deletions(-) > > diff --git a/elf/dl-tls.c b/elf/dl-tls.c > index 24d00c14ef..f8b32b3ecb 100644 > --- a/elf/dl-tls.c > +++ b/elf/dl-tls.c > @@ -743,6 +743,8 @@ _dl_update_slotinfo (unsigned long int req_modid) > { > for (size_t cnt = total == 0 ? 1 : 0; cnt < listp->len; ++cnt) > { > + size_t modid = total + cnt; > + > size_t gen = listp->slotinfo[cnt].gen; > > if (gen > new_gen) > @@ -758,25 +760,12 @@ _dl_update_slotinfo (unsigned long int req_modid) > > /* If there is no map this means the entry is empty. */ > struct link_map *map = listp->slotinfo[cnt].map; > - if (map == NULL) > - { > - if (dtv[-1].counter >= total + cnt) > - { > - /* If this modid was used at some point the memory > - might still be allocated. */ > - free (dtv[total + cnt].pointer.to_free); > - dtv[total + cnt].pointer.val = TLS_DTV_UNALLOCATED; > - dtv[total + cnt].pointer.to_free = NULL; > - } > - > - continue; > - } > - > /* Check whether the current dtv array is large enough. */ > - size_t modid = map->l_tls_modid; > - assert (total + cnt == modid); > if (dtv[-1].counter < modid) > { > + if (map == NULL) > + continue; > + > /* Resize the dtv. */ > dtv = _dl_resize_dtv (dtv); > >
diff --git a/elf/dl-tls.c b/elf/dl-tls.c index 24d00c14ef..f8b32b3ecb 100644 --- a/elf/dl-tls.c +++ b/elf/dl-tls.c @@ -743,6 +743,8 @@ _dl_update_slotinfo (unsigned long int req_modid) { for (size_t cnt = total == 0 ? 1 : 0; cnt < listp->len; ++cnt) { + size_t modid = total + cnt; + size_t gen = listp->slotinfo[cnt].gen; if (gen > new_gen) @@ -758,25 +760,12 @@ _dl_update_slotinfo (unsigned long int req_modid) /* If there is no map this means the entry is empty. */ struct link_map *map = listp->slotinfo[cnt].map; - if (map == NULL) - { - if (dtv[-1].counter >= total + cnt) - { - /* If this modid was used at some point the memory - might still be allocated. */ - free (dtv[total + cnt].pointer.to_free); - dtv[total + cnt].pointer.val = TLS_DTV_UNALLOCATED; - dtv[total + cnt].pointer.to_free = NULL; - } - - continue; - } - /* Check whether the current dtv array is large enough. */ - size_t modid = map->l_tls_modid; - assert (total + cnt == modid); if (dtv[-1].counter < modid) { + if (map == NULL) + continue; + /* Resize the dtv. */ dtv = _dl_resize_dtv (dtv);