commit-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [hurd,commited] hurd: Fix static-PIE startup


From: H.J. Lu
Subject: Re: [hurd,commited] hurd: Fix static-PIE startup
Date: Mon, 31 Jan 2022 16:14:59 -0800

 On Tue, Dec 28, 2021 at 1:43 AM Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
>
> hurd initialization stages use RUN_HOOK to run various initialization
> functions.  That is however using absolute addresses which need to be
> relocated, which is done later by csu.  We can however easily make the
> linker compute relative addresses which thus don't need a relocation.
> The new SET_RELHOOK and RUN_RELHOOK macros implement this.
> ---
>  hurd/dtable.c                       |  8 +++-----
>  hurd/hurdid.c                       |  7 +++----
>  hurd/hurdinit.c                     |  6 +++---
>  hurd/hurdmalloc.c                   | 10 +++-------
>  hurd/hurdpid.c                      |  7 +++----
>  hurd/hurdrlimit.c                   |  7 +++----
>  hurd/hurdsock.c                     |  7 +++----
>  include/set-hooks.h                 | 25 +++++++++++++++++++++++
>  sysdeps/generic/set-hooks-arch.h    | 31 +++++++++++++++++++++++++++++
>  sysdeps/i386/set-hooks-arch.h       | 28 ++++++++++++++++++++++++++
>  sysdeps/mach/hurd/brk.c             |  8 ++++----
>  sysdeps/mach/hurd/check_fds.c       |  6 ++----
>  sysdeps/mach/hurd/i386/init-first.c |  2 +-
>  sysdeps/x86_64/set-hooks-arch.h     | 28 ++++++++++++++++++++++++++
>  14 files changed, 140 insertions(+), 40 deletions(-)
>  create mode 100644 sysdeps/generic/set-hooks-arch.h
>  create mode 100644 sysdeps/i386/set-hooks-arch.h
>  create mode 100644 sysdeps/x86_64/set-hooks-arch.h
>
> diff --git a/hurd/dtable.c b/hurd/dtable.c
> index bbd3bfc892..4f081f09fd 100644
> --- a/hurd/dtable.c
> +++ b/hurd/dtable.c
> @@ -36,7 +36,7 @@ DEFINE_HOOK (_hurd_fd_subinit, (void));
>
>  /* Initialize the file descriptor table at startup.  */
>
> -static void
> +static void attribute_used_retain
>  init_dtable (void)
>  {
>    int i;
> @@ -91,12 +91,10 @@ init_dtable (void)
>
>    /* Run things that want to run after the file descriptor table
>       is initialized.  */
> -  RUN_HOOK (_hurd_fd_subinit, ());
> -
> -  (void) &init_dtable;         /* Avoid "defined but not used" warning.  */
> +  RUN_RELHOOK (_hurd_fd_subinit, ());
>  }
>
> -text_set_element (_hurd_subinit, init_dtable);
> +SET_RELHOOK (_hurd_subinit, init_dtable);
>
>  /* XXX when the linker supports it, the following functions should all be
>     elsewhere and just have text_set_elements here.  */
> diff --git a/hurd/hurdid.c b/hurd/hurdid.c
> index 70c46c0740..b220532c35 100644
> --- a/hurd/hurdid.c
> +++ b/hurd/hurdid.c
> @@ -17,6 +17,7 @@
>
>  #include <hurd.h>
>  #include <hurd/id.h>
> +#include "set-hooks.h"
>
>  struct hurd_id_data _hurd_id;
>
> @@ -74,7 +75,7 @@ _hurd_check_ids (void)
>    return 0;
>  }
>
> -static void
> +static void attribute_used_retain
>  init_id (void)
>  {
>    __mutex_init (&_hurd_id.lock);
> @@ -84,7 +85,5 @@ init_id (void)
>    _hurd_id.gen.nuids = _hurd_id.aux.nuids = 0;
>    _hurd_id.gen.gids = _hurd_id.aux.gids = NULL;
>    _hurd_id.gen.ngids = _hurd_id.aux.ngids = 0;
> -
> -  (void) &init_id;             /* Avoid "defined but not used" warning.  */
>  }
> -text_set_element (_hurd_preinit_hook, init_id);
> +SET_RELHOOK (_hurd_preinit_hook, init_id);
> diff --git a/hurd/hurdinit.c b/hurd/hurdinit.c
> index d4d219a083..9377d902d2 100644
> --- a/hurd/hurdinit.c
> +++ b/hurd/hurdinit.c
> @@ -108,7 +108,7 @@ _hurd_init (int flags, char **argv,
>    /* Call other things which want to do some initialization.  These are not
>       on the __libc_subinit hook because things there like to be able to
>       assume the availability of the POSIX.1 services we provide.  */
> -  RUN_HOOK (_hurd_subinit, ());
> +  RUN_RELHOOK (_hurd_subinit, ());
>  }
>  libc_hidden_def (_hurd_init)
>
> @@ -190,7 +190,7 @@ _hurd_new_proc_init (char **argv,
>    /* Call other things which want to do some initialization.  These are not
>       on the _hurd_subinit hook because things there assume that things done
>       here, like _hurd_pid, are already initialized.  */
> -  RUN_HOOK (_hurd_proc_subinit, ());
> +  RUN_RELHOOK (_hurd_proc_subinit, ());
>
>    /* XXX This code should probably be removed entirely at some point.  This
>       conditional should make it reasonably usable with old gdb's for a
> @@ -242,7 +242,7 @@ _hurd_setproc (process_t procserver)
>
>      /* Call these functions again so they can fetch the
>         new information from the new proc server.  */
> -    RUN_HOOK (_hurd_proc_subinit, ());
> +    RUN_RELHOOK (_hurd_proc_subinit, ());
>
>      if (_hurd_pgrp != oldpgrp)
>        {
> diff --git a/hurd/hurdmalloc.c b/hurd/hurdmalloc.c
> index 7046bcef33..ccb8fc8b54 100644
> --- a/hurd/hurdmalloc.c
> +++ b/hurd/hurdmalloc.c
> @@ -1,5 +1,6 @@
>  #include <stdlib.h>
>  #include <string.h>
> +#include "set-hooks.h"
>
>  #include "hurdmalloc.h"                /* XXX see that file */
>
> @@ -148,7 +149,7 @@ static struct free_list malloc_free_list[NBUCKETS];
>     It preserves the values of data variables like malloc_free_list, but
>     does not save the vm_allocate'd space allocated by this malloc.  */
>
> -static void
> +static void attribute_used_retain
>  malloc_init (void)
>  {
>    int i;
> @@ -160,11 +161,6 @@ malloc_init (void)
>        malloc_free_list[i].in_use = 0;
>  #endif
>      }
> -
> -  /* This not only suppresses a `defined but not used' warning,
> -     but it is ABSOLUTELY NECESSARY to avoid the hyperclever
> -     compiler from "optimizing out" the entire function!  */
> -  (void) &malloc_init;
>  }
>
>  static void
> @@ -445,4 +441,4 @@ _hurd_malloc_fork_child(void)
>  }
>
>
> -text_set_element (_hurd_preinit_hook, malloc_init);
> +SET_RELHOOK (_hurd_preinit_hook, malloc_init);
> diff --git a/hurd/hurdpid.c b/hurd/hurdpid.c
> index 9014de58e9..613ca357c6 100644
> --- a/hurd/hurdpid.c
> +++ b/hurd/hurdpid.c
> @@ -17,11 +17,12 @@
>
>  #include <hurd.h>
>  #include <lowlevellock.h>
> +#include "set-hooks.h"
>
>  pid_t _hurd_pid, _hurd_ppid, _hurd_pgrp;
>  int _hurd_orphaned;
>
> -static void
> +static void attribute_used_retain
>  init_pids (void)
>  {
>    __USEPORT (PROC,
> @@ -29,11 +30,9 @@ init_pids (void)
>                __proc_getpids (port, &_hurd_pid, &_hurd_ppid, 
> &_hurd_orphaned);
>                __proc_getpgrp (port, _hurd_pid, &_hurd_pgrp);
>              }));
> -
> -  (void) &init_pids;           /* Avoid "defined but not used" warning.  */
>  }
>
> -text_set_element (_hurd_proc_subinit, init_pids);
> +SET_RELHOOK (_hurd_proc_subinit, init_pids);
>
>  #include <hurd/msg_server.h>
>  #include "set-hooks.h"
> diff --git a/hurd/hurdrlimit.c b/hurd/hurdrlimit.c
> index 17535c2851..4d16bd4f3f 100644
> --- a/hurd/hurdrlimit.c
> +++ b/hurd/hurdrlimit.c
> @@ -19,6 +19,7 @@
>  #include <hurd.h>
>  #include <lock-intern.h>
>  #include <hurd/resource.h>
> +#include "set-hooks.h"
>
>  /* This must be given an initializer, or the a.out linking rules will
>     not include the entire file when this symbol is referenced. */
> @@ -29,7 +30,7 @@ struct rlimit _hurd_rlimits[RLIM_NLIMITS] = { { 0, }, };
>     mutex_init is still required below just in case of unexec.  */
>  struct mutex _hurd_rlimit_lock = { SPIN_LOCK_INITIALIZER, };
>
> -static void
> +static void attribute_used_retain
>  init_rlimit (void)
>  {
>    int i;
> @@ -52,7 +53,5 @@ init_rlimit (void)
>           }
>  #undef I
>      }
> -
> -  (void) &init_rlimit;
>  }
> -text_set_element (_hurd_preinit_hook, init_rlimit);
> +SET_RELHOOK (_hurd_preinit_hook, init_rlimit);
> diff --git a/hurd/hurdsock.c b/hurd/hurdsock.c
> index 04e86b4324..116326b2f2 100644
> --- a/hurd/hurdsock.c
> +++ b/hurd/hurdsock.c
> @@ -25,6 +25,7 @@
>  #include <_itoa.h>
>  #include <lock-intern.h>       /* For `struct mutex'.  */
>  #include "hurdmalloc.h"                /* XXX */
> +#include "set-hooks.h"
>
>  static struct mutex lock;
>
> @@ -109,7 +110,7 @@ retry:
>    return server;
>  }
>
> -static void
> +static void attribute_used_retain
>  init (void)
>  {
>    int i;
> @@ -118,7 +119,5 @@ init (void)
>
>    for (i = 0; i < max_domain; ++i)
>      servers[i] = MACH_PORT_NULL;
> -
> -  (void) &init;                        /* Avoid "defined but not used" 
> warning.  */
>  }
> -text_set_element (_hurd_preinit_hook, init);
> +SET_RELHOOK (_hurd_preinit_hook, init);
> diff --git a/include/set-hooks.h b/include/set-hooks.h
> index a60b5ae19f..1a2f6ad56b 100644
> --- a/include/set-hooks.h
> +++ b/include/set-hooks.h
> @@ -24,6 +24,8 @@
>  #include <sys/cdefs.h>
>  #include <libc-symbols.h>
>
> +#include "set-hooks-arch.h"
> +
>  #ifdef symbol_set_define
>  /* Define a hook variable called NAME.  Functions put on this hook take
>     arguments described by PROTO.  Use `text_set_element (NAME, FUNCTION)'
> @@ -55,6 +57,25 @@ do {                                                       
>                 \
>  DEFINE_HOOK (name, proto); \
>  extern void runner proto; void runner proto { RUN_HOOK (name, args); }
>
> +# ifdef SET_RELHOOK
> +/* This is similar to RUN_RELHOOK, but the hooks were registered with
> + * SET_RELHOOK so that a relative offset was computed by the linker
> + * rather than an absolute address by the dynamic linker. */
> +#  define RUN_RELHOOK(NAME, ARGS)                                    \
> +do {                                                                 \
> +  void *const *ptr;                                                  \
> +  for (ptr = (void *const *) symbol_set_first_element (NAME);        \
> +       ! symbol_set_end_p (NAME, ptr); ++ptr) {                              
> \
> +    __##NAME##_hook_function_t *f =                                  \
> +       (void*) ((uintptr_t) ptr + (ptrdiff_t) *ptr);                 \
> +    (*f) ARGS;                                                       \
> +  } \
> +} while (0)
> +# else
> +#  define SET_RELHOOK(NAME, HOOK) text_set_element (NAME, HOOK)
> +#  define RUN_RELHOOK(NAME, ARGS) RUN_HOOK(NAME, ARGS)
> +# endif
> +
>  #else
>
>  /* The system does not provide necessary support for this.  */
> @@ -66,6 +87,10 @@ extern void runner proto; void runner proto { RUN_HOOK 
> (name, args); }
>
>  # define DEFINE_HOOK_RUNNER(name, runner, proto, args)
>
> +# define SET_RELHOOK(NAME, HOOK)
> +
> +# define RUN_RELHOOK(NAME, ARGS)
> +
>  #endif
>
>  #endif /* set-hooks.h */
> diff --git a/sysdeps/generic/set-hooks-arch.h 
> b/sysdeps/generic/set-hooks-arch.h
> new file mode 100644
> index 0000000000..4b47fa2f2e
> --- /dev/null
> +++ b/sysdeps/generic/set-hooks-arch.h
> @@ -0,0 +1,31 @@
> +/* Machine-dependent macros for using symbol sets for running lists of
> +   functions. Generic/stub version.
> +   Copyright (C) 2021 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SET_HOOKS_ARCH_H
> +#define _SET_HOOKS_ARCH_H
> +
> +/* Define SET_RELHOOK to a variant of text_set_element that records a 
> relative
> +   offset rather than an absolute address. See sysdeps/i386/set-hooks-arch.h
> +   for an example.
> +
> +#define SET_RELHOOK(NAME, HOOK) ...
> +
> + */
> +
> +#endif /* set_hooks_arch.h */
> diff --git a/sysdeps/i386/set-hooks-arch.h b/sysdeps/i386/set-hooks-arch.h
> new file mode 100644
> index 0000000000..97513bf897
> --- /dev/null
> +++ b/sysdeps/i386/set-hooks-arch.h
> @@ -0,0 +1,28 @@
> +/* Machine-dependent macros for using symbol sets for running lists of
> +   functions. i386 version.
> +   Copyright (C) 2021 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SET_HOOKS_ARCH_H
> +#define _SET_HOOKS_ARCH_H
> +
> +#define SET_RELHOOK(NAME, HOOK) \
> +       asm(".section " #NAME",\"aR\"\n" \
> +           ".long "#HOOK" - .\n" \
> +           ".section .text");
> +
> +#endif /* set_hooks_arch.h */
> diff --git a/sysdeps/mach/hurd/brk.c b/sysdeps/mach/hurd/brk.c
> index a4294b9eae..088c99b2c8 100644
> --- a/sysdeps/mach/hurd/brk.c
> +++ b/sysdeps/mach/hurd/brk.c
> @@ -21,6 +21,8 @@
>  #include <lock-intern.h>       /* For `struct mutex'.  */
>  #include <vm_param.h>
>
> +#include "set-hooks.h"
> +
>
>  /* Initial maximum size of the data segment (this is arbitrary).  */
>  #define        DATA_SIZE       (128 * 1024 * 1024)
> @@ -130,7 +132,7 @@ _hurd_set_brk (vm_address_t addr)
>    return 0;
>  }
>
> -static void
> +static void attribute_used_retain
>  init_brk (void)
>  {
>    vm_address_t pagend;
> @@ -160,7 +162,5 @@ init_brk (void)
>         /* Couldn't allocate the memory.  The break will be very short.  */
>         _hurd_data_end = pagend;
>      }
> -
> -  (void) &init_brk;            /* Avoid ``defined but not used'' warning.  */
>  }
> -text_set_element (_hurd_preinit_hook, init_brk);
> +SET_RELHOOK (_hurd_preinit_hook, init_brk);
> diff --git a/sysdeps/mach/hurd/check_fds.c b/sysdeps/mach/hurd/check_fds.c
> index 155e9dd3e9..61e6055d35 100644
> --- a/sysdeps/mach/hurd/check_fds.c
> +++ b/sysdeps/mach/hurd/check_fds.c
> @@ -79,7 +79,7 @@ check_standard_fds (void)
>    check_one_fd (STDERR_FILENO, O_RDWR);
>  }
>
> -static void
> +static void attribute_used_retain
>  init_standard_fds (void)
>  {
>    /* Now that we have FDs, make sure that, if this is a SUID program,
> @@ -87,10 +87,8 @@ init_standard_fds (void)
>       ourselves.  If that's not possible we stop the program.  */
>    if (__builtin_expect (__libc_enable_secure, 0))
>      check_standard_fds ();
> -
> -  (void) &init_standard_fds;   /* Avoid "defined but not used" warning.  */
>  }
> -text_set_element (_hurd_fd_subinit, init_standard_fds);
> +SET_RELHOOK (_hurd_fd_subinit, init_standard_fds);
>
>
>  #ifndef SHARED
> diff --git a/sysdeps/mach/hurd/i386/init-first.c 
> b/sysdeps/mach/hurd/i386/init-first.c
> index 5e85aa2bc5..c6ae370daf 100644
> --- a/sysdeps/mach/hurd/i386/init-first.c
> +++ b/sysdeps/mach/hurd/i386/init-first.c
> @@ -242,7 +242,7 @@ first_init (void)
>    /* Initialize data structures so we can do RPCs.  */
>    __mach_init ();
>
> -  RUN_HOOK (_hurd_preinit_hook, ());
> +  RUN_RELHOOK (_hurd_preinit_hook, ());
>  }
>
>  #ifdef SHARED
> diff --git a/sysdeps/x86_64/set-hooks-arch.h b/sysdeps/x86_64/set-hooks-arch.h
> new file mode 100644
> index 0000000000..227d4f8e08
> --- /dev/null
> +++ b/sysdeps/x86_64/set-hooks-arch.h
> @@ -0,0 +1,28 @@
> +/* Machine-dependent macros for using symbol sets for running lists of
> +   functions. x86-64 version.
> +   Copyright (C) 2021 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SET_HOOKS_ARCH_H
> +#define _SET_HOOKS_ARCH_H
> +
> +#define SET_RELHOOK(NAME, HOOK) \
> +       asm(".section " #NAME",\"aR\"\n" \
> +           ".quad "#HOOK" - .\n" \
> +           ".section .text");
> +
> +#endif /* set_hooks_arch.h */

SET_RELHOOK is used only for hurd which doesn't support x86-64.
Why not remove sysdeps/x86_64/set-hooks-arch.h and move
sysdeps/i386/set-hooks-arch.h to sysdeps/mach/hurd/i386?


-- 
H.J.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]