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][BZ #17833] _dl_close_worker() does not release inconsistent objects.


On 01/19/2015 01:39 PM, Pavel Kopyl wrote:
> Hello,
> 
> I faced with the following problem. [See test case:
> https://sourceware.org/bugzilla/show_bug.cgi?id=17833]
> 
> I've a shared library that contains both undefined and unique
> symbols. Then I try to call the following sequence of dlopen:
> 
> 1. dlopen("./libfoo.so", RTLD_NOW)
> 2. dlopen("./libfoo.so", RTLD_LAZY | RTLD_GLOBAL)
> 
> First dlopen call terminates with error because of undefined symbols,
> but STB_GNU_UNIQUE ones set DF_1_NODELETE flag and hence block
> library in the memory. The library goes into inconsistent state as
> several structures remain uninitialized. For instance, relocations
> for GOT table were not performed. By the time of second dlopen call
> this library looks like as it would be fully initialized but this is
> not true: any call through incorrect GOT table leads to segmentation
> fault. On some systems this inconsistency triggers assertions in the
> dynamic linker.
>
> Suggested patch forces object deletion in case of dlopen() fail:
> 
> 1. Clears DF_1_NODELETE flag if dlopen() fails, allowing library to be
>    deleted from memory.
> 2. For each unique symbol that is defined in this object clears
>    appropriate entry in _ns_unique_sym_table.

>From a high level perspective your patch looks plausibly correct.

I expect no other threads can have possibly seen the result of the
dlopen() because it's not yet complete, and so it's OK to clear
the STB_GNU_UNIQUE symbols list because none are yet used. Did you
verify this?

The solution you've chosen looks good, failing at dlopen time and
cleaning up the resulting changes is good.

This change should go into glibc 2.22 (February when branch opens).
I don't want to make this kind of internals change in dlopen right
now without having a full release to make sure we didn't break anything.

As another reviewer points out, we need a test case for this.

Please repost a v2 with a regression test.

Good work on the patch.

Cheers,
Carlos.


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