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

[Yury Gribov]

On 05/29/2015 08:38 PM, Pavel Kopyl wrote:
> On 05/29/2015 12:31 AM, Pavel Kopyl wrote:
>> On 05/28/2015 01:06 AM, H.J. Lu wrote:
>>> On Wed, May 27, 2015 at 2:38 PM, Pavel Kopyl <p.kopyl@samsung.com>
>>> wrote:
>>>>>>>> AFAIU DF_1_NODELETE change is necessary. Otherwise unique
>>>>>>>> symbols force
>>>>>>>> library to remain loaded in half-initialized state (with missing
>>>>>>>> dependencies). This usecase is demonstrated by the testcase in
>>>>>>>> patch.
>>>>>>>>
>>>>>>> But unique symbols != DF_1_NODELETE:
>>>>>>
>>>>>> DF_1_NODELETE is set in do_lookup_unique at runtime (Paul, am I
>>>>>> right?):
>>>>>>
>>>>>>         enter_unique_sym (entries, size,
>>>>>>                           new_hash, strtab + sym->st_name, sym, map);
>>>>>>
>>>>>>         if (map->l_type == lt_loaded)
>>>>>>           /* Make sure we don't unload this object by
>>>>>>              setting the appropriate flag.  */
>>>>>>           ((struct link_map *) map)->l_flags_1 |= DF_1_NODELETE;
>>>>>>
>>>>>> -Y
>>>>> Should we set DF_1_NODELETE when dlopen fails?  What happens
>>>>> when dlopen a  DF_1_NODELETE fails?  Do we keep it in memory
>>>>> even when dopen fails?
>>>>>
>>>> As I know there are following ways how to set DF_1_NODELETE flag for a
>>>> library.
>>>>
>>>> 1. Unique symbols.
>>>> 2, Load with RTLD_NODELETE flag.
>>>> 3. Link with "-z nodelete" option.
>>>>
>>>> As for RTLD_NODELETE, there is no problem: dlopen exits in case of
>>>> error
>>>> before DF_1_NODELETE is set.
>>>> If dlopen fails both in first and third cases this leads to locking
>>>> libraries in half-initialized state. I think this is a bug.
>>>> Suggested patch fixes both usecases.
>>> Assume there is a problem with DF_1_NODELETE, which can be
>>> set explicitly as well as implicitly.  Your testcase only covers
>>> implicitly DF_1_NODELETE, not explicitly  DF_1_NODELETE.
>>> I think we should verify the status of explicitly DF_1_NODELETE.
>>>
>>> If explicitly  DF_1_NODELETE works fine, then we only have a
>>> problem with implicitly DF_1_NODELETE.
>>>
>>> If explicitly  DF_1_NODELETE doesn't work, we need to fix it
>>> first and implicitly DF_1_NODELETE may just work.
>>
>> Ok, I prepared the following testcase.
>>
>> tst-nodelete.cc:
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <dlfcn.h>
>>
>> int
>> main (void)
>> {
>>   int ret = 0;
>>
>>   /* This is a test for correct handling of dlopen failures for
>> library that
>>      is loaded with RTLD_NODELETE flag. The first dlopen should fail
>> because
>>      of undefined symbols in shared library. The second dlopen then
>> verifies
>>      that library was properly unloaded.  */
>>   if (dlopen ("./tst-nodeletelib.so", RTLD_NOW | RTLD_NODELETE) != NULL
>>       || dlopen ("./tst-nodeletelib.so", RTLD_LAZY | RTLD_NOLOAD) !=
>> NULL)
>>     {
>>       printf("RTLD_NOLOAD test failed\n");
>>       ret = 1;
>>     }
>>
>>   /* This is a test for correct handling of dlopen failures for
>> library that
>>      is linked with '-z nodelete' option and hence has DF_1_NODELETE
>> flag.
>>      The first dlopen should fail because of undefined symbols in shared
>>      library. The second dlopen then verifies that library was properly
>>      unloaded.  */
>>   if (dlopen ("./tst-znodeletelib.so", RTLD_NOW) != NULL
>>       || dlopen ("./tst-znodeletelib.so", RTLD_LAZY | RTLD_NOLOAD) !=
>> NULL)
>>     {
>>       printf("-z \'nodelete\' test failed\n");
>>       ret = 1;
>>     }
>>
>>    /* This is a test for correct handling of dlopen failures for library
>>      with unique symbols. The first dlopen should fail because of
>> undefined
>>      symbols in shared library. The second dlopen then verifies that
>> library
>>      was properly unloaded.  */
>>   if (dlopen ("./tst-unique5lib.so", RTLD_NOW) != NULL
>>       || dlopen ("./tst-unique5lib.so", RTLD_LAZY | RTLD_NOLOAD) != NULL)
>>     {
>>       printf("Unique symbols test failed\n");
>>       ret = 1;
>>     }
>>
>>   if (!ret)
>>     printf ("SUCCESS\n");
>>
>>   return ret;
>> }
>>
>>
>> tst-unique5lib.cc:
>> extern int not_exist (void);
>>
>> inline int make_unique (void)
>> {
>> /* Static variables in inline functions and classes
>>    generate STB_GNU_UNIQUE symbols.  */
>>   static int unique;
>>   return ++unique;
>> }
>>
>> int foo (void)
>> {
>>   return make_unique () + not_exist ();
>> }
>>
>>
>> tst-nodeletelib.cc:
>> extern int not_exist (void);
>>
>> int foo (void)
>> {
>>   return  not_exist ();
>> }
>>
>>
>> g++ -o tst-nodelete tst-nodelete.cc -ldl
>> g++ -shared -fPIC -o tst-nodeletelib.so tst-nodeletelib.cc
>> g++ -shared -fPIC -Wl,-z,nodelete -o tst-znodeletelib.so
>> tst-nodeletelib.cc
>> ./tst-nodelete
>> -z 'nodelete' test failed
>> Unique symbols test failed
>>
>>
>> Thus, besides unique symbols we have a problem with explicitly
>> DF_1_NODELETE when library is built with "-Wl,-z,nodelete".
>> I checked that patch fixes this usecase too. Now I'm going to update
>> testcase in make check.
>
> This is new patch with updated testcase.
>
> -Pavel

Ping.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: delete_inconsistent_objs_v5.patch
Type: text/x-patch
Size: 8717 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20150605/7c51c827/attachment.bin>