Re: GNU C Library master sources branch master updated. glibc-2.19-214-ga42faf5

[Paul Pluzhnikov <ppluzhnikov at google dot com>]

On Mon, Mar 24, 2014 at 6:07 PM, Siddhesh Poyarekar
<siddhesh.poyarekar@gmail.com> wrote:

> Oh, I had expected that you'd repost the patch with the test case.

Sorry. All fixed.

>> +2014-03-24  Paul Pluzhnikov  <ppluzhnikov@google.com>
>> +
>> +       [BZ #16634]
>> +
>
> Incorrect extra newline.
>
>> +       * elf/dl-load.c (open_verify): Add mode parameter.
>> +        Error early when ET_EXEC and mode does not have __RTLD_OPENEXEC.
>> +        (open_path): Change from boolean 'secure' to complete flag 'mode'
>> +        (_dl_map_object): Adjust.
>> +       * elf/Makefile (tests): Add tst-dlopen-aout.
>> +       * elf/tst-dlopen-aout.c: New test.
>> +
>>  2014-03-24  Stefan Liebler <stli@linux.vnet.ibm.com>
>>
>>         [BZ #16714]
>> diff --git a/NEWS b/NEWS
>> index a5540f1..79a298f 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -10,9 +10,9 @@ Version 2.20
>>  * The following bugs are resolved with this release:
>>
>>    15347, 15804, 15894, 16002, 16284, 16447, 16532, 16545, 16574, 16600,
>> -  16609, 16610, 16611, 16613, 16623, 16632, 16639, 16642, 16649, 16670,
>> -  16674, 16677, 16680, 16683, 16689, 16695, 16701, 16706, 16707, 16731,
>> -  16743.
>> +  16609, 16610, 16611, 16613, 16623, 16632, 16634, 16639, 16642, 16649,
>> +  16670, 16674, 16677, 16680, 16683, 16689, 16695, 16701, 16706, 16707,
>> +  16731, 16743.
>>
>>  * Running the testsuite no longer terminates as soon as a test fails.
>>    Instead, a file tests.sum (xtests.sum from "make xcheck") is generated,
>> diff --git a/elf/Makefile b/elf/Makefile
>> index df138fc..d96cd40 100644
>> --- a/elf/Makefile
>> +++ b/elf/Makefile
>> @@ -148,7 +148,7 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
>>          tst-stackguard1 tst-addr1 tst-thrlock \
>>          tst-unique1 tst-unique2 tst-unique3 tst-unique4 \
>>          tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
>> -        tst-ptrguard1
>> +        tst-ptrguard1 tst-dlopen-aout
>>  #       reldep9
>>  test-srcs = tst-pathopt
>>  selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
>> @@ -1056,6 +1056,7 @@ tst-leaks1-static-ENV = MALLOC_TRACE=$(objpfx)tst-leaks1-static.mtrace
>>  $(objpfx)tst-addr1: $(libdl)
>>
>>  $(objpfx)tst-thrlock: $(libdl) $(shared-thread-library)
>> +$(objpfx)tst-dlopen-aout: $(libdl) $(shared-thread-library)
>>
>>  CFLAGS-ifuncmain1pic.c += $(pic-ccflag)
>>  CFLAGS-ifuncmain1picstatic.c += $(pic-ccflag)
>> diff --git a/elf/dl-load.c b/elf/dl-load.c
>> index 8ebc128..9455df2 100644
>> --- a/elf/dl-load.c
>> +++ b/elf/dl-load.c
>> @@ -1667,7 +1667,7 @@ print_search_path (struct r_search_path_elem **list,
>>     user might want to know about this.  */
>>  static int
>>  open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>> -            int whatcode, bool *found_other_class, bool free_name)
>> +            int whatcode, int mode, bool *found_other_class, bool free_name)
>>  {
>>    /* This is the expected ELF header.  */
>>  #define ELF32_CLASS ELFCLASS32
>> @@ -1843,6 +1843,17 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>>           errstring = N_("only ET_DYN and ET_EXEC can be loaded");
>>           goto call_lose;
>>         }
>> +      else if (__glibc_unlikely (ehdr->e_type == ET_EXEC
>> +                                && (mode & __RTLD_OPENEXEC) == 0))
>> +       {
>> +         /* BZ #16634. It is an error to dlopen ET_EXEC (unless
>> +            __RTLD_OPENEXEC is explicitly set).  We return error here
>> +            so that code in _dl_map_object_from_fd does not try to set
>> +            l_tls_modid for this module.  */
>> +
>> +         errstring = N_("cannot dynamically load executable");
>> +         goto call_lose;
>> +       }
>>        else if (__builtin_expect (ehdr->e_phentsize, sizeof (ElfW(Phdr)))
>>                != sizeof (ElfW(Phdr)))
>>         {
>> @@ -1928,7 +1939,7 @@ open_verify (const char *name, struct filebuf *fbp, struct link_map *loader,
>>     if MAY_FREE_DIRS is true.  */
>>
>>  static int
>> -open_path (const char *name, size_t namelen, int secure,
>> +open_path (const char *name, size_t namelen, int mode,
>>            struct r_search_path_struct *sps, char **realname,
>>            struct filebuf *fbp, struct link_map *loader, int whatcode,
>>            bool *found_other_class)
>> @@ -1980,8 +1991,8 @@ open_path (const char *name, size_t namelen, int secure,
>>           if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_LIBS))
>>             _dl_debug_printf ("  trying file=%s\n", buf);
>>
>> -         fd = open_verify (buf, fbp, loader, whatcode, found_other_class,
>> -                           false);
>> +         fd = open_verify (buf, fbp, loader, whatcode, mode,
>> +                           found_other_class, false);
>>           if (this_dir->status[cnt] == unknown)
>>             {
>>               if (fd != -1)
>> @@ -2010,7 +2021,7 @@ open_path (const char *name, size_t namelen, int secure,
>>           /* Remember whether we found any existing directory.  */
>>           here_any |= this_dir->status[cnt] != nonexisting;
>>
>> -         if (fd != -1 && __builtin_expect (secure, 0)
>> +         if (fd != -1 && __builtin_expect (mode & __RTLD_SECURE, 0)
>>               && INTUSE(__libc_enable_secure))
>>             {
>>               /* This is an extra security effort to make sure nobody can
>> @@ -2184,7 +2195,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>>           for (l = loader; l; l = l->l_loader)
>>             if (cache_rpath (l, &l->l_rpath_dirs, DT_RPATH, "RPATH"))
>>               {
>> -               fd = open_path (name, namelen, mode & __RTLD_SECURE,
>> +               fd = open_path (name, namelen, mode,
>>                                 &l->l_rpath_dirs,
>>                                 &realname, &fb, loader, LA_SER_RUNPATH,
>>                                 &found_other_class);
>> @@ -2200,7 +2211,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>>               && main_map != NULL && main_map->l_type != lt_loaded
>>               && cache_rpath (main_map, &main_map->l_rpath_dirs, DT_RPATH,
>>                               "RPATH"))
>> -           fd = open_path (name, namelen, mode & __RTLD_SECURE,
>> +           fd = open_path (name, namelen, mode,
>>                             &main_map->l_rpath_dirs,
>>                             &realname, &fb, loader ?: main_map, LA_SER_RUNPATH,
>>                             &found_other_class);
>> @@ -2208,7 +2219,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>>
>>        /* Try the LD_LIBRARY_PATH environment variable.  */
>>        if (fd == -1 && env_path_list.dirs != (void *) -1)
>> -       fd = open_path (name, namelen, mode & __RTLD_SECURE, &env_path_list,
>> +       fd = open_path (name, namelen, mode, &env_path_list,
>>                         &realname, &fb,
>>                         loader ?: GL(dl_ns)[LM_ID_BASE]._ns_loaded,
>>                         LA_SER_LIBPATH, &found_other_class);
>> @@ -2217,7 +2228,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>>        if (fd == -1 && loader != NULL
>>           && cache_rpath (loader, &loader->l_runpath_dirs,
>>                           DT_RUNPATH, "RUNPATH"))
>> -       fd = open_path (name, namelen, mode & __RTLD_SECURE,
>> +       fd = open_path (name, namelen, mode,
>>                         &loader->l_runpath_dirs, &realname, &fb, loader,
>>                         LA_SER_RUNPATH, &found_other_class);
>>
>> @@ -2267,7 +2278,8 @@ _dl_map_object (struct link_map *loader, const char *name,
>>                 {
>>                   fd = open_verify (cached,
>>                                     &fb, loader ?: GL(dl_ns)[nsid]._ns_loaded,
>> -                                   LA_SER_CONFIG, &found_other_class, false);
>> +                                   LA_SER_CONFIG, mode, &found_other_class,
>> +                                   false);
>>                   if (__glibc_likely (fd != -1))
>>                     {
>>                       realname = local_strdup (cached);
>> @@ -2287,7 +2299,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>>           && ((l = loader ?: GL(dl_ns)[nsid]._ns_loaded) == NULL
>>               || __builtin_expect (!(l->l_flags_1 & DF_1_NODEFLIB), 1))
>>           && rtld_search_dirs.dirs != (void *) -1)
>> -       fd = open_path (name, namelen, mode & __RTLD_SECURE, &rtld_search_dirs,
>> +       fd = open_path (name, namelen, mode, &rtld_search_dirs,
>>                         &realname, &fb, l, LA_SER_DEFAULT, &found_other_class);
>>
>>        /* Add another newline when we are tracing the library loading.  */
>> @@ -2305,7 +2317,7 @@ _dl_map_object (struct link_map *loader, const char *name,
>>        else
>>         {
>>           fd = open_verify (realname, &fb,
>> -                           loader ?: GL(dl_ns)[nsid]._ns_loaded, 0,
>> +                           loader ?: GL(dl_ns)[nsid]._ns_loaded, 0, mode,
>>                             &found_other_class, true);
>>           if (__builtin_expect (fd, 0) == -1)
>>             free (realname);
>> diff --git a/elf/tst-dlopen-aout.c b/elf/tst-dlopen-aout.c
>> new file mode 100644
>> index 0000000..d353425
>> --- /dev/null
>> +++ b/elf/tst-dlopen-aout.c
>> @@ -0,0 +1,61 @@
>> +/* Test case for BZ #16634.
>
> More descriptive one-liner.  Suggest:
>
>     Verify that incorrectly opening an executable without
> __RTLD_OPENEXEC does not affect dl_tls_max_dtv_idx.
>
>> +
>> +   Copyright (C) 2014 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <http://www.gnu.org/licenses/>.
>> +
>> +   Note: this test currently only fails when glibc is configured with
>> +   --enable-hardcoded-path-in-tests.  */
>> +
>> +#include <assert.h>
>> +#include <dlfcn.h>
>> +#include <stdio.h>
>> +#include <pthread.h>
>> +
>> +__thread int x;
>> +
>> +void *
>> +fn (void *p)
>> +{
>> +  return p;
>> +}
>> +
>> +int
>> +main (int argc, char *argv[])
>
> Is there a reason why this doesn't use test-skeleton.c?
>
>> +{
>> +  int j;
>> +
>> +  for (j = 0; j < 100; ++j)
>> +    {
>> +      pthread_t thr;
>> +      void *p;
>> +      int rc;
>> +
>> +      p = dlopen (argv[0], RTLD_LAZY);
>> +      if (p != NULL)
>> +        {
>> +          fprintf (stderr, "dlopen unexpectedly succeeded\n");
>> +          return 1;
>> +        }
>> +      rc = pthread_create (&thr, NULL, fn, NULL);
>> +      assert (rc == 0);
>> +
>> +      rc = pthread_join (thr, NULL);
>> +      assert (rc == 0);
>> +    }
>> +
>> +  return 0;
>> +}
>>
>> -----------------------------------------------------------------------
>>
>> Summary of changes:
>>  ChangeLog                                     |   11 ++++
>>  NEWS                                          |    6 +-
>>  elf/Makefile                                  |    3 +-
>>  elf/dl-load.c                                 |   36 +++++++++-----
>>  libio/test-freopen.c => elf/tst-dlopen-aout.c |   63 +++++++++++++-----------
>>  5 files changed, 74 insertions(+), 45 deletions(-)
>>  copy libio/test-freopen.c => elf/tst-dlopen-aout.c (51%)
>>
>>
>> hooks/post-receive
>> --
>> GNU C Library master sources
>
>
>
> --
> http://siddhesh.in



-- 
Paul Pluzhnikov