diff options
author | Maninder Singh <maninder1.s@samsung.com> | 2018-01-10 15:17:30 +0000 |
---|---|---|
committer | Szabolcs Nagy <szabolcs.nagy@arm.com> | 2021-02-15 11:49:15 +0000 |
commit | 3721784a79c9d2040297304b2a7216d7072ea838 (patch) | |
tree | 459fb973dc4f42660b19a2eacdfd30aa7355fce9 | |
parent | 90458da8b7b46e806f756c2715c87fc9d2c1be95 (diff) | |
download | glibc-3721784a79c9d2040297304b2a7216d7072ea838.zip glibc-3721784a79c9d2040297304b2a7216d7072ea838.tar.gz glibc-3721784a79c9d2040297304b2a7216d7072ea838.tar.bz2 |
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: Szabolcs Nagy <szabolcs.nagy@arm.com>
-rw-r--r-- | elf/dl-load.c | 18 | ||||
-rw-r--r-- | elf/dl-misc.c | 4 |
2 files changed, 20 insertions, 2 deletions
diff --git a/elf/dl-load.c b/elf/dl-load.c index 9e2089c..be54baf 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -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. */ diff --git a/elf/dl-misc.c b/elf/dl-misc.c index 082f75f..d4803bb 100644 --- a/elf/dl-misc.c +++ b/elf/dl-misc.c @@ -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; } |