[PATCH][V2][BZ #17833][PING] _dl_close_worker() does not release inconsistent objects.

[Pavel Kopyl]

On 02/12/2015 12:04 AM, Pavel Kopyl wrote:
>
> On 01/21/2015 03:52 AM, Carlos O'Donell 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.
>
> Thanks for review.
> Here is patch v2 with regression test

Ping.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: delete_inconsistent_objs.patch
Type: text/x-patch
Size: 5754 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20150227/3589db2a/attachment.bin>