|
Message-ID: <CAMe9rOpmiiBEZqLz-94_MEwgRky+EUsfd=X6Ue30H2c9R=dSKQ@mail.gmail.com> Date: Tue, 3 Nov 2020 04:36:42 -0800 From: "H.J. Lu" <hjl.tools@...il.com> To: Florian Weimer <fweimer@...hat.com> Cc: Szabolcs Nagy <szabolcs.nagy@....com>, GNU C Library <libc-alpha@...rceware.org>, Jeremy Linton <jeremy.linton@....com>, Catalin Marinas <catalin.marinas@....com>, Mark Rutland <mark.rutland@....com>, Will Deacon <will.deacon@....com>, Mark Brown <broonie@...nel.org>, Kees Cook <keescook@...omium.org>, Salvatore Mesoraca <s.mesoraca16@...il.com>, Lennart Poettering <mzxreary@...inter.de>, Topi Miettinen <toiwoton@...il.com>, LKML <linux-kernel@...r.kernel.org>, linux-arm-kernel@...ts.infradead.org, Kernel Hardening <kernel-hardening@...ts.openwall.com>, linux-hardening@...r.kernel.org Subject: Re: [PATCH 2/4] elf: Move note processing after l_phdr is updated [BZ #26831] On Tue, Nov 3, 2020 at 2:38 AM Florian Weimer <fweimer@...hat.com> wrote: > > * Szabolcs Nagy: > > > Program headers are processed in two pass: after the first pass > > load segments are mmapped so in the second pass target specific > > note processing logic can access the notes. > > > > The second pass is moved later so various link_map fields are > > set up that may be useful for note processing such as l_phdr. > > --- > > elf/dl-load.c | 30 +++++++++++++++--------------- > > 1 file changed, 15 insertions(+), 15 deletions(-) > > > > diff --git a/elf/dl-load.c b/elf/dl-load.c > > index ceaab7f18e..673cf960a0 100644 > > --- a/elf/dl-load.c > > +++ b/elf/dl-load.c > > @@ -1259,21 +1259,6 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, > > maplength, has_holes, loader); > > if (__glibc_unlikely (errstring != NULL)) > > goto call_lose; > > - > > - /* Process program headers again after load segments are mapped in > > - case processing requires accessing those segments. Scan program > > - headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY > > - exits. */ > > - for (ph = &phdr[l->l_phnum]; ph != phdr; --ph) > > - switch (ph[-1].p_type) > > - { > > - case PT_NOTE: > > - _dl_process_pt_note (l, fd, &ph[-1]); > > - break; > > - case PT_GNU_PROPERTY: > > - _dl_process_pt_gnu_property (l, fd, &ph[-1]); > > - break; > > - } > > } > > > > if (l->l_ld == 0) > > @@ -1481,6 +1466,21 @@ cannot enable executable stack as shared object requires"); > > /* Assign the next available module ID. */ > > l->l_tls_modid = _dl_next_tls_modid (); > > > > + /* Process program headers again after load segments are mapped in > > + case processing requires accessing those segments. Scan program > > + headers backward so that PT_NOTE can be skipped if PT_GNU_PROPERTY > > + exits. */ > > + for (ph = &l->l_phdr[l->l_phnum]; ph != l->l_phdr; --ph) > > + switch (ph[-1].p_type) > > + { > > + case PT_NOTE: > > + _dl_process_pt_note (l, fd, &ph[-1]); > > + break; > > + case PT_GNU_PROPERTY: > > + _dl_process_pt_gnu_property (l, fd, &ph[-1]); > > + break; > > + } > > + > > #ifdef DL_AFTER_LOAD > > DL_AFTER_LOAD (l); > > #endif > > Is this still compatible with the CET requirements? > > I hope it is because the CET magic happens in _dl_open_check, so after > the the code in elf/dl-load.c has run. > > _dl_process_pt_note and _dl_process_pt_gnu_property may call _dl_signal_error. Are we prepared to clean more things up when it happens? I am investigating: https://sourceware.org/bugzilla/show_bug.cgi?id=26825 I don't think cleanup of _dl_process_pt_gnu_property failure is done properly. -- H.J.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.