This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/1] dl-load: add memory barrier before updating the next.


On Thu, 2017-03-16 at 10:42 +0530, Maninder Singh wrote:
> This patch adds memory barrier before updating the liblist next.
> 
> 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.

Thanks for trying to fix concurrency issues.  I don't have the details
of synchronization in the loader paged in, so I'll just reply quickly
for now with a few general comments.

If you haven't yet read it, please have a look at
https://sourceware.org/glibc/wiki/Concurrency

If we fix concurrent code, we generally want to do so by transforming
the code to using C11 atomics (if necessary) and being valid in the C11
memory model.  For example, we want to make sure that it is
data-race-free as defined by C11 (e.g., with your patch and as you
describe the concurrent exceutions that are possible, there would need
to be a data-race involving the store to lastp->next).

If you add something like a C11 release action (ie, the write barrier
you add), then it must be paired with an acquire action somewhere else
to take effect.

We want to document concurrent code better.  In particular, we want to
say which acquire operation a release operation is supposed to
synchronize with, and why we need the happens-before this results in.  

Also, its often useful to say what the "input" concurrency is.  For
example, in this case, which concurrent accesses to the link_map data
structure are possible in a valid program?

If you have further questions about using the C11-like atomics we have,
please ask.  Looking at other concurrent code, for example
nptl/pthread_once.c, can also be helpful to get an idea of what we're
looking for.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]