mbox series

[v2,0/4] fix ifunc with static pie [BZ #27072]

Message ID cover.1610121077.git.szabolcs.nagy@arm.com
Headers show
Series fix ifunc with static pie [BZ #27072] | expand

Message

Szabolcs Nagy Jan. 8, 2021, 4:19 p.m. UTC 
v2:
- check PI_STATIC_AND_HIDDEN for --enable-static-pie
- change string buffer sizes in the tunables
- fix env_alias == NULL logic in __tunables_init
- move __ehdr_start processing after self relocation

I think this is in a reasonable shape now, but there are still
some issues:
- tunables try to allocate memory (tunable_strdup) even if
  that's not necessary: only setuid binaries need this (in
  case there is a TUNABLE_SECLEVEL_SXID_ERASE tunable).
  this adds a lot of complexity and a failure path to the early
  init code. i think that if there is any such tunable then the
  entire GLIBC_TUNABLE= should just be dropped.
- tunable strings could be stored more compactly (and without
  arbitrary size limits) as an optimization. (can be done once
  there are too many tunables.)
- all symbols are forced hidden in libc.a, but i think lib*.a
  should do the same. (other than lib*_nonshared.a)

Szabolcs Nagy (4):
  configure: Require PI_STATIC_AND_HIDDEN for static pie
  Make libc symbols hidden in static PIE
  elf: Avoid RELATIVE relocs in __tunables_init
  csu: Move static pie self relocation later [BZ #27072]

 configure                | 14 +++++++++++++
 configure.ac             |  5 +++++
 csu/libc-start.c         | 44 +++++++++++++++++++++++-----------------
 elf/dl-tunables.c        |  2 +-
 elf/dl-tunables.h        |  4 ++--
 include/libc-symbols.h   |  8 ++++++--
 scripts/gen-tunables.awk |  2 +-
 7 files changed, 54 insertions(+), 25 deletions(-)

Comments

H.J. Lu Jan. 8, 2021, 5:04 p.m. UTC | #1 
On Fri, Jan 8, 2021 at 8:22 AM Szabolcs Nagy via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> v2:
> - check PI_STATIC_AND_HIDDEN for --enable-static-pie
> - change string buffer sizes in the tunables
> - fix env_alias == NULL logic in __tunables_init
> - move __ehdr_start processing after self relocation
>
> I think this is in a reasonable shape now, but there are still
> some issues:
> - tunables try to allocate memory (tunable_strdup) even if
>   that's not necessary: only setuid binaries need this (in
>   case there is a TUNABLE_SECLEVEL_SXID_ERASE tunable).
>   this adds a lot of complexity and a failure path to the early
>   init code. i think that if there is any such tunable then the
>   entire GLIBC_TUNABLE= should just be dropped.
> - tunable strings could be stored more compactly (and without
>   arbitrary size limits) as an optimization. (can be done once
>   there are too many tunables.)
> - all symbols are forced hidden in libc.a, but i think lib*.a
>   should do the same. (other than lib*_nonshared.a)
>
> Szabolcs Nagy (4):
>   configure: Require PI_STATIC_AND_HIDDEN for static pie
>   Make libc symbols hidden in static PIE
>   elf: Avoid RELATIVE relocs in __tunables_init
>   csu: Move static pie self relocation later [BZ #27072]
>

Can you push your patches into a branch? I'd like to add an x86 test
on top of your patches.

Thanks.
Szabolcs Nagy Jan. 11, 2021, 10:50 a.m. UTC | #2 
The 01/08/2021 09:04, H.J. Lu wrote:
> On Fri, Jan 8, 2021 at 8:22 AM Szabolcs Nagy via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > v2:
> > - check PI_STATIC_AND_HIDDEN for --enable-static-pie
> > - change string buffer sizes in the tunables
> > - fix env_alias == NULL logic in __tunables_init
> > - move __ehdr_start processing after self relocation
> >
> > I think this is in a reasonable shape now, but there are still
> > some issues:
> > - tunables try to allocate memory (tunable_strdup) even if
> >   that's not necessary: only setuid binaries need this (in
> >   case there is a TUNABLE_SECLEVEL_SXID_ERASE tunable).
> >   this adds a lot of complexity and a failure path to the early
> >   init code. i think that if there is any such tunable then the
> >   entire GLIBC_TUNABLE= should just be dropped.
> > - tunable strings could be stored more compactly (and without
> >   arbitrary size limits) as an optimization. (can be done once
> >   there are too many tunables.)
> > - all symbols are forced hidden in libc.a, but i think lib*.a
> >   should do the same. (other than lib*_nonshared.a)
> >
> > Szabolcs Nagy (4):
> >   configure: Require PI_STATIC_AND_HIDDEN for static pie
> >   Make libc symbols hidden in static PIE
> >   elf: Avoid RELATIVE relocs in __tunables_init
> >   csu: Move static pie self relocation later [BZ #27072]
> >
> 
> Can you push your patches into a branch? I'd like to add an x86 test
> on top of your patches.

i rebased and pushed them into nsz/bug27072