This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH v5][BZ #21349] fix data race during lazy resolution in _dl_name_match_p
- From: Szabolcs Nagy <szabolcs dot nagy at arm dot com>
- To: GNU C Library <libc-alpha at sourceware dot org>, Maninder Singh <maninder1 dot s at samsung dot com>, "triegel at redhat dot com" <triegel at redhat dot com>, Vaneet Narang <v dot narang at samsung dot com>, PANKAJ MISHRA <pankaj dot m at samsung dot com>, AMIT SAHRAWAT <a dot sahrawat at samsung dot com>, Lalit Mohan Tripathi <lalit dot mohan at samsung dot com>, AKHILESH KUMAR <akhilesh dot k at samsung dot com>, Hakbong Lee <hakbong5 dot lee at samsung dot com>
- Cc: nd at arm dot com
- Date: Wed, 17 Jan 2018 17:37:38 +0000
- Subject: [PATCH v5][BZ #21349] fix data race during lazy resolution in _dl_name_match_p
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Szabolcs dot Nagy at arm dot com;
- Nodisclaimer: True
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
only comments and description update compared to
https://sourceware.org/ml/libc-alpha/2017-04/msg00160.html
(Code is from Maninder Singh and Vaneet Narang, comments and description
is from Szabolcs Nagy.)
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.
2018-01-17 Maninder Singh <maninder1.s@samsung.com>
Vaneet Narang <v.narang@samsung.com>
Szabolcs Nagy <szabolcs.nagy@arm.com>
[BZ #21349]
* elf/dl-load.c (add_name_to_object): Use atomic_store_release.
* elf/dl-misc.c (_dl_name_match_p): Use atomic_load_acquire.
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 7554a99b5a..d3e59952e8 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -375,7 +375,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 b7174994cd..03a8feb0c6 100644
--- a/elf/dl-misc.c
+++ b/elf/dl-misc.c
@@ -289,7 +289,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;
}