elf: Fix data race in _dl_name_match_p [BZ #21349]

dlopen updates libname_list by writing to lastp->next, but concurrent
reads in _dl_name_match_p were not synchronized when it was called
without holding GL(dl_load_lock), which can happen during lazy symbol
resolution.

This patch fixes the race between _dl_name_match_p reading lastp->next
and add_name_to_object writing to it. This could cause segfault on
targets with weak memory order when lastp->next->name is read, which
was observed on an arm system. Fixes bug 21349.

(Code is from Maninder Singh, comments and description is from Szabolcs
Nagy.)

Co-authored-by: Vaneet Narang <v.narang@samsung.com>
Co-authored-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
This commit is contained in:
Maninder Singh 2018-01-10 15:17:30 +00:00 committed by Szabolcs Nagy
parent 69499bb6ee
commit 395be7c218
2 changed files with 20 additions and 2 deletions

View File

@ -438,7 +438,23 @@ add_name_to_object (struct link_map *l, const char *name)
newname->name = memcpy (newname + 1, name, name_len);
newname->next = NULL;
newname->dont_free = 0;
lastp->next = newname;
/* CONCURRENCY NOTES:
Make sure the initialization of newname happens before its address is
read from the lastp->next store below.
GL(dl_load_lock) is held here (and by other writers, e.g. dlclose), so
readers of libname_list->next (e.g. _dl_check_caller or the reads above)
can use that for synchronization, however the read in _dl_name_match_p
may be executed without holding the lock during _dl_runtime_resolve
(i.e. lazy symbol resolution when a function of library l is called).
The release MO store below synchronizes with the acquire MO load in
_dl_name_match_p. Other writes need to synchronize with that load too,
however those happen either early when the process is single threaded
(dl_main) or when the library is unloaded (dlclose) and the user has to
synchronize library calls with unloading. */
atomic_store_release (&lastp->next, newname);
}
/* Standard search directories. */

View File

@ -347,7 +347,9 @@ _dl_name_match_p (const char *name, const struct link_map *map)
if (strcmp (name, runp->name) == 0)
return 1;
else
runp = runp->next;
/* Synchronize with the release MO store in add_name_to_object.
See CONCURRENCY NOTES in add_name_to_object in dl-load.c. */
runp = atomic_load_acquire (&runp->next);
return 0;
}