From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106256 invoked by alias); 22 Dec 2016 20:06:24 -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 106234 invoked by uid 89); 22 Dec 2016 20:06:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 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=wherever, H*MI:sk:6D39441, detailing, initialise X-HELO: mailapp01.imgtec.com From: Matthew Fortune To: "libc-alpha@sourceware.org" CC: Petar Jovanovic , Sandra Loosemore , Joseph Myers Subject: RE: [PATCH,resend] fix uninitialized variable in dynamic linker Date: Thu, 22 Dec 2016 20:06:00 -0000 Message-ID: <6D39441BF12EF246A7ABCE6654B0235380B07804@HHMAIL01.hh.imgtec.org> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-SW-Source: 2016-12/txt/msg00889.txt.bz2 Ping^2 >=20 > Matthew > > -----Original Message----- > > From: libc-alpha-owner@sourceware.org [mailto:libc-alpha-owner@sourcewa= re.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 > > > > 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 exam= ple > > > >> - 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 th= is > > > > is directly related to the reject_phdr feature for MIPS and only MI= PS > > > > 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? > > > > (updated subject to follow Sandra's original thread) > > > > 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. > > > > 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. > > > > 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. > > > > I'm re-running tests but Sandra's original testing should still be vali= d. > > > > Thanks, > > Matthew > > > > 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 > > > > - /* 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_i= nfo[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 > > > > -- > > 2.7.4