Message ID | 20210803215914.4170913-2-hjl.tools@gmail.com |
---|---|
State | New |
Headers | show |
Series | [1/2] Initial support for GNU_PROPERTY_1_NEEDED | expand |
On Tue, Aug 3, 2021 at 2:59 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > When performing symbol lookup for references in executable without > indirect external access: > > 1. Disallow copy relocations in executable against protected data symbols > in a shared object with indirect external access. > 2. Disallow non-zero symbol values of undefined function symbols in > executable, which are used as the function pointer, against protected > function symbols in a shared object with indirect external access. PING: https://sourceware.org/pipermail/libc-alpha/2021-August/129798.html https://sourceware.org/pipermail/libc-alpha/2021-August/129799.html
On 03/08/2021 18:59, H.J. Lu via Libc-alpha wrote: > When performing symbol lookup for references in executable without > indirect external access: > > 1. Disallow copy relocations in executable against protected data symbols > in a shared object with indirect external access. > 2. Disallow non-zero symbol values of undefined function symbols in > executable, which are used as the function pointer, against protected > function symbols in a shared object with indirect external access. How hard would to add some testcases for both cases? To simplify we may want to build it iff binutins supports noindirect-extern-access. The rest LGTM, just a nit below due an ununsed variable. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > elf/dl-lookup.c | 5 ++++ > sysdeps/generic/dl-protected.h | 54 ++++++++++++++++++++++++++++++++++ > 2 files changed, 59 insertions(+) > create mode 100644 sysdeps/generic/dl-protected.h > > diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c > index eea217eb28..430359af39 100644 > --- a/elf/dl-lookup.c > +++ b/elf/dl-lookup.c > @@ -24,6 +24,7 @@ > #include <ldsodefs.h> > #include <dl-hash.h> > #include <dl-machine.h> > +#include <dl-protected.h> > #include <sysdep-cancel.h> > #include <libc-lock.h> > #include <tls.h> > @@ -527,6 +528,10 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash, > if (__glibc_unlikely (dl_symbol_visibility_binds_local_p (sym))) > goto skip; > > + if (ELFW(ST_VISIBILITY) (sym->st_other) == STV_PROTECTED) > + _dl_check_protected_symbol (undef_name, undef_map, ref, map, > + type_class); > + > switch (ELFW(ST_BIND) (sym->st_info)) > { > case STB_WEAK: Ok. > diff --git a/sysdeps/generic/dl-protected.h b/sysdeps/generic/dl-protected.h > new file mode 100644 > index 0000000000..244d020dc4 > --- /dev/null > +++ b/sysdeps/generic/dl-protected.h > @@ -0,0 +1,54 @@ > +/* Support for STV_PROTECTED visibility. Generic 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 _DL_PROTECTED_H > +#define _DL_PROTECTED_H > + > +static inline void __attribute__ ((always_inline)) > +_dl_check_protected_symbol (const char *undef_name, This argument seems unused. > + const struct link_map *undef_map, > + const ElfW(Sym) *ref, > + const struct link_map *map, > + int type_class) > +{ > + if (undef_map != NULL > + && undef_map->l_type == lt_executable > + && !(undef_map->l_1_needed > + & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS) > + && (map->l_1_needed > + & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS)) > + { > + if ((type_class & ELF_RTYPE_CLASS_COPY)) > + /* Disallow copy relocations in executable against protected > + data symbols in a shared object which needs indirect external > + access. */ > + _dl_signal_error (0, map->l_name, undef_name, > + N_("copy relocation against non-copyable protected symbol")); > + else if (ref->st_value != 0 > + && ref->st_shndx == SHN_UNDEF > + && (type_class & ELF_RTYPE_CLASS_PLT)) > + /* Disallow non-zero symbol values of undefined symbols in > + executable, which are used as the function pointer, against > + protected function symbols in a shared object with indirect > + external access. */ > + _dl_signal_error (0, map->l_name, undef_name, > + N_("non-canonical reference to canonical protected function")); > + } > +} > + > +#endif /* _DL_PROTECTED_H */ > Ok.
On Thu, Oct 7, 2021 at 9:58 AM Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > > > On 03/08/2021 18:59, H.J. Lu via Libc-alpha wrote: > > When performing symbol lookup for references in executable without > > indirect external access: > > > > 1. Disallow copy relocations in executable against protected data symbols > > in a shared object with indirect external access. > > 2. Disallow non-zero symbol values of undefined function symbols in > > executable, which are used as the function pointer, against protected > > function symbols in a shared object with indirect external access. > > How hard would to add some testcases for both cases? To simplify we may > want to build it iff binutins supports noindirect-extern-access. I will submit followup patches with testcases from users/hjl/indirect/master branch: https://gitlab.com/x86-glibc/glibc/-/tree/users/hjl/indirect/master including adding LD_DEBUG=protected to check copy relocations against protected data and non-canonical reference to protected function. > The rest LGTM, just a nit below due an ununsed variable. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > > --- > > elf/dl-lookup.c | 5 ++++ > > sysdeps/generic/dl-protected.h | 54 ++++++++++++++++++++++++++++++++++ > > 2 files changed, 59 insertions(+) > > create mode 100644 sysdeps/generic/dl-protected.h > > > > diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c > > index eea217eb28..430359af39 100644 > > --- a/elf/dl-lookup.c > > +++ b/elf/dl-lookup.c > > @@ -24,6 +24,7 @@ > > #include <ldsodefs.h> > > #include <dl-hash.h> > > #include <dl-machine.h> > > +#include <dl-protected.h> > > #include <sysdep-cancel.h> > > #include <libc-lock.h> > > #include <tls.h> > > @@ -527,6 +528,10 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash, > > if (__glibc_unlikely (dl_symbol_visibility_binds_local_p (sym))) > > goto skip; > > > > + if (ELFW(ST_VISIBILITY) (sym->st_other) == STV_PROTECTED) > > + _dl_check_protected_symbol (undef_name, undef_map, ref, map, > > + type_class); > > + > > switch (ELFW(ST_BIND) (sym->st_info)) > > { > > case STB_WEAK: > > Ok. > > > diff --git a/sysdeps/generic/dl-protected.h b/sysdeps/generic/dl-protected.h > > new file mode 100644 > > index 0000000000..244d020dc4 > > --- /dev/null > > +++ b/sysdeps/generic/dl-protected.h > > @@ -0,0 +1,54 @@ > > +/* Support for STV_PROTECTED visibility. Generic 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 _DL_PROTECTED_H > > +#define _DL_PROTECTED_H > > + > > +static inline void __attribute__ ((always_inline)) > > +_dl_check_protected_symbol (const char *undef_name, > > This argument seems unused. It is used in _dl_signal_error (0, map->l_name, undef_name, N_("non-canonical reference to canonical protected function")); > > + const struct link_map *undef_map, > > + const ElfW(Sym) *ref, > > + const struct link_map *map, > > + int type_class) > > +{ > > + if (undef_map != NULL > > + && undef_map->l_type == lt_executable > > + && !(undef_map->l_1_needed > > + & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS) > > + && (map->l_1_needed > > + & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS)) > > + { > > + if ((type_class & ELF_RTYPE_CLASS_COPY)) > > + /* Disallow copy relocations in executable against protected > > + data symbols in a shared object which needs indirect external > > + access. */ > > + _dl_signal_error (0, map->l_name, undef_name, > > + N_("copy relocation against non-copyable protected symbol")); > > + else if (ref->st_value != 0 > > + && ref->st_shndx == SHN_UNDEF > > + && (type_class & ELF_RTYPE_CLASS_PLT)) > > + /* Disallow non-zero symbol values of undefined symbols in > > + executable, which are used as the function pointer, against > > + protected function symbols in a shared object with indirect > > + external access. */ > > + _dl_signal_error (0, map->l_name, undef_name, > > + N_("non-canonical reference to canonical protected function")); > > + } > > +} > > + > > +#endif /* _DL_PROTECTED_H */ > > > > Ok. I will check it in ASIS. Thanks.
On 07/10/2021 14:10, H.J. Lu wrote: > On Thu, Oct 7, 2021 at 9:58 AM Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: >> >> >> >> On 03/08/2021 18:59, H.J. Lu via Libc-alpha wrote: >>> When performing symbol lookup for references in executable without >>> indirect external access: >>> >>> 1. Disallow copy relocations in executable against protected data symbols >>> in a shared object with indirect external access. >>> 2. Disallow non-zero symbol values of undefined function symbols in >>> executable, which are used as the function pointer, against protected >>> function symbols in a shared object with indirect external access. >> >> How hard would to add some testcases for both cases? To simplify we may >> want to build it iff binutins supports noindirect-extern-access. > > I will submit followup patches with testcases from > users/hjl/indirect/master branch: > > https://gitlab.com/x86-glibc/glibc/-/tree/users/hjl/indirect/master > > including adding LD_DEBUG=protected to check copy relocations against > protected data and non-canonical reference to protected function. > >> The rest LGTM, just a nit below due an ununsed variable. >> >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> >>> --- >>> elf/dl-lookup.c | 5 ++++ >>> sysdeps/generic/dl-protected.h | 54 ++++++++++++++++++++++++++++++++++ >>> 2 files changed, 59 insertions(+) >>> create mode 100644 sysdeps/generic/dl-protected.h >>> >>> diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c >>> index eea217eb28..430359af39 100644 >>> --- a/elf/dl-lookup.c >>> +++ b/elf/dl-lookup.c >>> @@ -24,6 +24,7 @@ >>> #include <ldsodefs.h> >>> #include <dl-hash.h> >>> #include <dl-machine.h> >>> +#include <dl-protected.h> >>> #include <sysdep-cancel.h> >>> #include <libc-lock.h> >>> #include <tls.h> >>> @@ -527,6 +528,10 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash, >>> if (__glibc_unlikely (dl_symbol_visibility_binds_local_p (sym))) >>> goto skip; >>> >>> + if (ELFW(ST_VISIBILITY) (sym->st_other) == STV_PROTECTED) >>> + _dl_check_protected_symbol (undef_name, undef_map, ref, map, >>> + type_class); >>> + >>> switch (ELFW(ST_BIND) (sym->st_info)) >>> { >>> case STB_WEAK: >> >> Ok. >> >>> diff --git a/sysdeps/generic/dl-protected.h b/sysdeps/generic/dl-protected.h >>> new file mode 100644 >>> index 0000000000..244d020dc4 >>> --- /dev/null >>> +++ b/sysdeps/generic/dl-protected.h >>> @@ -0,0 +1,54 @@ >>> +/* Support for STV_PROTECTED visibility. Generic 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 _DL_PROTECTED_H >>> +#define _DL_PROTECTED_H >>> + >>> +static inline void __attribute__ ((always_inline)) >>> +_dl_check_protected_symbol (const char *undef_name, >> >> This argument seems unused. > > It is used in > > _dl_signal_error (0, map->l_name, undef_name, > N_("non-canonical reference to canonical > protected function")); Indeed, you are right. > >>> + const struct link_map *undef_map, >>> + const ElfW(Sym) *ref, >>> + const struct link_map *map, >>> + int type_class) >>> +{ >>> + if (undef_map != NULL >>> + && undef_map->l_type == lt_executable >>> + && !(undef_map->l_1_needed >>> + & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS) >>> + && (map->l_1_needed >>> + & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS)) >>> + { >>> + if ((type_class & ELF_RTYPE_CLASS_COPY)) >>> + /* Disallow copy relocations in executable against protected >>> + data symbols in a shared object which needs indirect external >>> + access. */ >>> + _dl_signal_error (0, map->l_name, undef_name, >>> + N_("copy relocation against non-copyable protected symbol")); >>> + else if (ref->st_value != 0 >>> + && ref->st_shndx == SHN_UNDEF >>> + && (type_class & ELF_RTYPE_CLASS_PLT)) >>> + /* Disallow non-zero symbol values of undefined symbols in >>> + executable, which are used as the function pointer, against >>> + protected function symbols in a shared object with indirect >>> + external access. */ >>> + _dl_signal_error (0, map->l_name, undef_name, >>> + N_("non-canonical reference to canonical protected function")); >>> + } >>> +} >>> + >>> +#endif /* _DL_PROTECTED_H */ >>> >> >> Ok. > > I will check it in ASIS. > > Thanks. >
diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c index eea217eb28..430359af39 100644 --- a/elf/dl-lookup.c +++ b/elf/dl-lookup.c @@ -24,6 +24,7 @@ #include <ldsodefs.h> #include <dl-hash.h> #include <dl-machine.h> +#include <dl-protected.h> #include <sysdep-cancel.h> #include <libc-lock.h> #include <tls.h> @@ -527,6 +528,10 @@ do_lookup_x (const char *undef_name, uint_fast32_t new_hash, if (__glibc_unlikely (dl_symbol_visibility_binds_local_p (sym))) goto skip; + if (ELFW(ST_VISIBILITY) (sym->st_other) == STV_PROTECTED) + _dl_check_protected_symbol (undef_name, undef_map, ref, map, + type_class); + switch (ELFW(ST_BIND) (sym->st_info)) { case STB_WEAK: diff --git a/sysdeps/generic/dl-protected.h b/sysdeps/generic/dl-protected.h new file mode 100644 index 0000000000..244d020dc4 --- /dev/null +++ b/sysdeps/generic/dl-protected.h @@ -0,0 +1,54 @@ +/* Support for STV_PROTECTED visibility. Generic 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 _DL_PROTECTED_H +#define _DL_PROTECTED_H + +static inline void __attribute__ ((always_inline)) +_dl_check_protected_symbol (const char *undef_name, + const struct link_map *undef_map, + const ElfW(Sym) *ref, + const struct link_map *map, + int type_class) +{ + if (undef_map != NULL + && undef_map->l_type == lt_executable + && !(undef_map->l_1_needed + & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS) + && (map->l_1_needed + & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS)) + { + if ((type_class & ELF_RTYPE_CLASS_COPY)) + /* Disallow copy relocations in executable against protected + data symbols in a shared object which needs indirect external + access. */ + _dl_signal_error (0, map->l_name, undef_name, + N_("copy relocation against non-copyable protected symbol")); + else if (ref->st_value != 0 + && ref->st_shndx == SHN_UNDEF + && (type_class & ELF_RTYPE_CLASS_PLT)) + /* Disallow non-zero symbol values of undefined symbols in + executable, which are used as the function pointer, against + protected function symbols in a shared object with indirect + external access. */ + _dl_signal_error (0, map->l_name, undef_name, + N_("non-canonical reference to canonical protected function")); + } +} + +#endif /* _DL_PROTECTED_H */