[3/4] Add run-time chesk for single global definition

Message ID 20210620233620.391576-4-hjl.tools@gmail.com
State Superseded
Headers
Series Implement single global definition marker |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

H.J. Lu June 20, 2021, 11:36 p.m. UTC
  When performing symbol lookup for references in an object without single
global definition:

1. Disallow copy relocations against protected data symbols in an object
with single global definition.
2. Disallow non-zero symbol values of undefined function symbols, which
are used as the function pointer, against protected function symbols in
an object with single global definition.
---
 elf/dl-lookup.c                |  5 ++++
 sysdeps/generic/dl-protected.h | 51 ++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 sysdeps/generic/dl-protected.h
  

Comments

Florian Weimer June 21, 2021, 7:16 a.m. UTC | #1
* H. J. Lu via Libc-alpha:

> +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_1_needed
> +	   & GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION)
> +      && (map->l_1_needed
> +	  & GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION))
> +    {
> +      if ((type_class & ELF_RTYPE_CLASS_COPY))
> +	/* Disallow copy relocations against protected data symbols in
> +	   an object with single global definition.  */
> +	_dl_fatal_printf ("copy relocation against non-copyable protected symbol=%s in file=%s\n",
> +			  undef_name, DSO_FILENAME (map->l_name));
> +      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, which
> +	   are used as the function pointer, against protected function
> +	   symbols in an object with single global definition.  */
> +	_dl_fatal_printf ("non-canonical reference to canonical protected function symbol=%s in file=%s\n",
> +			  undef_name, DSO_FILENAME (map->l_name));
> +    }
> +}

Why are those fatal errors?

I have trouble understanding the second comment (for the
ELF_RTYPE_CLASS_PLT).

Thanks,
Florian
  
H.J. Lu June 21, 2021, 1:20 p.m. UTC | #2
On Mon, Jun 21, 2021 at 12:16 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > +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_1_needed
> > +        & GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION)
> > +      && (map->l_1_needed
> > +       & GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION))
> > +    {
> > +      if ((type_class & ELF_RTYPE_CLASS_COPY))
> > +     /* Disallow copy relocations against protected data symbols in
> > +        an object with single global definition.  */
> > +     _dl_fatal_printf ("copy relocation against non-copyable protected symbol=%s in file=%s\n",
> > +                       undef_name, DSO_FILENAME (map->l_name));
> > +      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, which
> > +        are used as the function pointer, against protected function
> > +        symbols in an object with single global definition.  */
> > +     _dl_fatal_printf ("non-canonical reference to canonical protected function symbol=%s in file=%s\n",
> > +                       undef_name, DSO_FILENAME (map->l_name));
> > +    }
> > +}
>
> Why are those fatal errors?

2 copies of the data symbol can be out of sync between executable and
shared library.  We can make them as warnings with tunable to control
it.

> I have trouble understanding the second comment (for the
> ELF_RTYPE_CLASS_PLT).

If st_value is the undefined symbol in executable is not zero, it
is the PLT address in executable and ld.so will use it for function
pointer which is different from the function address in shared
library.
  
Florian Weimer June 22, 2021, 7:12 a.m. UTC | #3
* H. J. Lu:

> On Mon, Jun 21, 2021 at 12:16 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu via Libc-alpha:
>>
>> > +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_1_needed
>> > +        & GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION)
>> > +      && (map->l_1_needed
>> > +       & GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION))
>> > +    {
>> > +      if ((type_class & ELF_RTYPE_CLASS_COPY))
>> > +     /* Disallow copy relocations against protected data symbols in
>> > +        an object with single global definition.  */
>> > +     _dl_fatal_printf ("copy relocation against non-copyable protected symbol=%s in file=%s\n",
>> > +                       undef_name, DSO_FILENAME (map->l_name));
>> > +      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, which
>> > +        are used as the function pointer, against protected function
>> > +        symbols in an object with single global definition.  */
>> > +     _dl_fatal_printf ("non-canonical reference to canonical protected function symbol=%s in file=%s\n",
>> > +                       undef_name, DSO_FILENAME (map->l_name));
>> > +    }
>> > +}
>>
>> Why are those fatal errors?
>
> 2 copies of the data symbol can be out of sync between executable and
> shared library.  We can make them as warnings with tunable to control
> it.

I meant: Why can't you turn them into regular dlopen errors that are
reported to the caller?  (Use _dl_signal_error or any of the related
functions.)

>> I have trouble understanding the second comment (for the
>> ELF_RTYPE_CLASS_PLT).
>
> If st_value is the undefined symbol in executable is not zero, it
> is the PLT address in executable and ld.so will use it for function
> pointer which is different from the function address in shared
> library.

Ahh, I wasn't aware of that ELF detail.

Thanks,
Florian
  

Patch

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..a530eb3e51
--- /dev/null
+++ b/sysdeps/generic/dl-protected.h
@@ -0,0 +1,51 @@ 
+/* 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_1_needed
+	   & GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION)
+      && (map->l_1_needed
+	  & GNU_PROPERTY_1_NEEDED_SINGLE_GLOBAL_DEFINITION))
+    {
+      if ((type_class & ELF_RTYPE_CLASS_COPY))
+	/* Disallow copy relocations against protected data symbols in
+	   an object with single global definition.  */
+	_dl_fatal_printf ("copy relocation against non-copyable protected symbol=%s in file=%s\n",
+			  undef_name, DSO_FILENAME (map->l_name));
+      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, which
+	   are used as the function pointer, against protected function
+	   symbols in an object with single global definition.  */
+	_dl_fatal_printf ("non-canonical reference to canonical protected function symbol=%s in file=%s\n",
+			  undef_name, DSO_FILENAME (map->l_name));
+    }
+}
+
+#endif /* _DL_PROTECTED_H */