From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 118595 invoked by alias); 16 Nov 2016 13:20:03 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 117950 invoked by uid 89); 16 Nov 2016 13:20:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,SPF_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=detailing, rtld.c, dl-machine.h, rtldc X-HELO: mailapp01.imgtec.com From: Matthew Fortune To: Matthew Fortune , Sandra Loosemore , Joseph Myers CC: "libc-alpha@sourceware.org" , Petar Jovanovic Subject: RE: [PATCH,resend] fix uninitialized variable in dynamic linker Date: Wed, 16 Nov 2016 13:20:00 -0000 Message-ID: <6D39441BF12EF246A7ABCE6654B0235380ACD5C0@HHMAIL01.hh.imgtec.org> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-SW-Source: 2016-11/txt/msg00544.txt.bz2 Ping Matthew > -----Original Message----- > From: libc-alpha-owner@sourceware.org [mailto:libc-alpha-owner@sourceware= .org] On Behalf > Of Matthew Fortune > Sent: 01 November 2016 13:01 > To: Sandra Loosemore; Joseph Myers > Cc: libc-alpha@sourceware.org; Petar Jovanovic > Subject: [PATCH,resend] fix uninitialized variable in dynamic linker >=20 > Sandra Loosemore writes: > > On 10/26/2016 03:01 PM, Matthew Fortune wrote: > > > Joseph Myers writes: > > >> On Wed, 26 Oct 2016, Matthew Fortune wrote: > > >> > > >>> +# ifdef ELF_MACHINE_INIT_MAP > > >>> + ELF_MACHINE_INIT_MAP (bootstrap_map); # endif > > >> > > >> We don't encourage use of #ifdef like that. It's better to have an > > >> inline function defined everywhere and used unconditionally, for > > >> which most systems have a dummy definition (see > > >> dl-machine-reject-phdr.h and elf_machine_reject_phdr_p for an example > > >> - if you have a header for a single function, you don't need to > > >> update lots of dl-machine.h headers, just add a generic version - > > >> which has the comments detailing the semantics of the function and > > when it's needed - and a MIPS version). > > > > > > Thanks Joseph. It's been a while since I did a glibc patch and > > > couldn't remember the recommended approach. > > > > > > Do you think I should add a whole new header for this? Or, since this > > > is directly related to the reject_phdr feature for MIPS and only MIPS > > > is affected then I could just add it to dl-machine-reject-phdr.h? > > > > Wouldn't it be easier and more maintainable just to unconditionally > > zero-initialize the structure, as I did in the original patch? >=20 > (updated subject to follow Sandra's original thread) >=20 > OK, given no further comments and your recommendation to stick with the > original. I'm reposting your original patch updated to remove minor > fuzz on applying. >=20 > Roland originally suggested leaving the compiler to initialise the stack > allocated link_map structure but doing so would lead to an implicit > memset function call. Since calls are not possible until the end of the > _dl_start function then we can't do that. >=20 > The data that gets zero'd increases from 1040 bytes to 1560 bytes for > MIPS n64 but the benefit of knowing the link_map is zero initialised > wherever it is created. I expect similar increases for the other > architectures that do not define PI_STATIC_AND_HIDDEN. >=20 > I'm re-running tests but Sandra's original testing should still be valid. >=20 > Thanks, > Matthew >=20 > diff --git a/elf/rtld.c b/elf/rtld.c > index 647661c..a1b3a39 100644 > --- a/elf/rtld.c > +++ b/elf/rtld.c > @@ -357,7 +357,7 @@ _dl_start (void *arg) > HP_TIMING_NOW (info.start_time); > #endif >=20 > - /* Partly clean the `bootstrap_map' structure up. Don't use > + /* Zero-initialize the `bootstrap_map' structure. Don't use > `memset' since it might not be built in or inlined and we cannot > make function calls at this point. Use '__builtin_memset' if we > know it is available. We do not have to clear the memory if we > @@ -365,12 +365,14 @@ _dl_start (void *arg) > are initialized to zero by default. */ > #ifndef DONT_USE_BOOTSTRAP_MAP > # ifdef HAVE_BUILTIN_MEMSET > - __builtin_memset (bootstrap_map.l_info, '\0', sizeof (bootstrap_map.l_= info)); > + __builtin_memset (&bootstrap_map, '\0', sizeof (struct link_map)); > # else > - for (size_t cnt =3D 0; > - cnt < sizeof (bootstrap_map.l_info) / sizeof (bootstrap_map.l_inf= o[0]); > - ++cnt) > - bootstrap_map.l_info[cnt] =3D 0; > + { > + char *p =3D (char *) &bootstrap_map; > + char *pend =3D p + sizeof (struct link_map); > + while (p < pend) > + *(p++) =3D '\0'; > + } > # endif > #endif >=20 > -- > 2.7.4