[PATCH v4] dl-load: add memory barrier before updating the next

[Maninder Singh <maninder1 dot s at samsung dot com>]

[BZ #21349]: race condition between dl_open and rtld lazy symbol resolve.

Issue Fix: race condition between add_name_to_object  & _dl_name_match_p.
One threads calling dlopen which further calls add_name_to_object &
other thread trying to resolve RTLD_LAZY symbols through
_dl_runtime_resolve which further calls.

_dl_name_match_p checks if libname->next is valid, then it assumes
libname->next->name to be valid. Also add_name_to_object initialized
name first and then sets valid next pointer.

This patch avoids any reorder of instruction when next is set before
name to avoid any race.

so as suggested by Torvald Riegel <triegel@redhat.com>:
"Add C11 memory barrier before updating the liblist next and add comments
for barriers."

Signed-off-by: Vaneet Narang <v.narang@samsung.com>
Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
---
v1 -> v2: use C11 atomics rather than direct memory barriers.
v2 -> v3: use comments for barriers and enter Bugzilla ID.
v3 -> v4: remove extra parens.

 elf/dl-load.c |    5 ++++-
 elf/dl-misc.c |    5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/elf/dl-load.c b/elf/dl-load.c
index a5318f9..03c6afb 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -418,7 +418,10 @@ 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;
+  /* We need release memory order here because we need to synchronize
+     with other thread doing _dl_runtime_resolve which calls _dl_name_match_p
+     to traverse all names added to libname_list */
+  atomic_store_release (&lastp->next, newname);
 }
 
 /* Standard search directories.  */
diff --git a/elf/dl-misc.c b/elf/dl-misc.c
index 1e9a6ee..a26d6f6 100644
--- a/elf/dl-misc.c
+++ b/elf/dl-misc.c
@@ -295,7 +295,10 @@ _dl_name_match_p (const char *name, const struct link_map *map)
     if (strcmp (name, runp->name) == 0)
       return 1;
     else
-      runp = runp->next;
+      /* We need to acquire memory order here because we need to synchronize
+         with other thread calling dlopen and adding new name to libname_list
+         through add_name_to_object */
+      runp = atomic_load_acquire (&runp->next);
 
   return 0;
 }
-- 
1.7.1