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

[Pavel Kopyl]

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.
>
>
> -Pavel

This is new patch with updated testcase.

-Pavel
-------------- 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/20150529/72a8b351/attachment.bin>