[PATCH v4] elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA
Szabolcs Nagy
szabolcs.nagy@arm.com
Wed Jun 15 13:18:54 GMT 2022
The 06/08/2022 10:10, Fangrui Song wrote:
> If an executable has copy relocations for extern protected data, that
> can only work if the library containing the definition is built with
> assumptions (a) the compiler emits GOT-generating relocations (b) the
> linker produces R_*_GLOB_DAT instead of R_*_RELATIVE. Otherwise the
> library uses its own definition directly and the executable accesses a
> stale copy. Note: the GOT relocations defeat the purpose of protected
> visibility as an optimization, but allow rtld to make the executable and
> library use the same copy when copy relocations are present, but it
> turns out this never worked perfectly.
>
> ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange semantics when both
> a.so and b.so define protected var and the executable copy relocates
> var: b.so accesses its own copy even with GLOB_DAT. The behavior change
> is from commit 62da1e3b00b51383ffa7efc89d8addda0502e107 (x86) and then
> copied to nios2 (ae5eae7cfc9c4a8297ff82ec6b794faca1976ecc) and arc
> (0e7d930c4c11de896fe807f67fa1eb756c9c1e05).
>
> Without ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA, b.so accesses the copy
> relocated data like a.so.
>
> ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has another effect in the absence
> of copy relocations: when a protected data symbol is defined in multiple
> objects, the code tries to bind the relocation locally. Without
> ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA, STV_PROTECTED is handled in the
> same way as STV_DEFAULT: if ld produces GLOB_DAT (some ports of GNU ld),
> the relocation will bind to the first definition; otherwise (e.g.
> ld.lld) ld does the binding locally and ld.so doesn't help.
this text does not look right with the updated patch.
i think you should just remove it, otherwise the patch looks ok.
>
> It's extremely unlikely anyone relies on the
> ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA behavior, so let's remove it: this
> removes a check in the symbol lookup code.
>
> --
> Changes from v1:
> * Reword commit message as suggested by Szabolcs Nagy
>
> Changes from v2:
> * Explain interposition behavior
>
> Changes from v3:
> * Keep `if (__glibc_unlikely (protected != 0))` for now to make this
> more focus on data symbols. I will still try removing the code in the future.
> ---
> elf/dl-lookup.c | 60 +------------------------------------
> sysdeps/arc/dl-sysdep.h | 21 -------------
> sysdeps/generic/ldsodefs.h | 12 +-------
> sysdeps/i386/dl-machine.h | 3 +-
> sysdeps/nios2/dl-sysdep.h | 21 -------------
> sysdeps/x86/dl-lookupcfg.h | 4 ---
> sysdeps/x86_64/dl-machine.h | 8 ++---
> 7 files changed, 5 insertions(+), 124 deletions(-)
> delete mode 100644 sysdeps/arc/dl-sysdep.h
> delete mode 100644 sysdeps/nios2/dl-sysdep.h
>
> diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
> index a42f6d5390..8cb32321da 100644
> --- a/elf/dl-lookup.c
> +++ b/elf/dl-lookup.c
> @@ -456,59 +456,6 @@ do_lookup_x (const char *undef_name, unsigned int new_hash,
> if (sym != NULL)
> {
> found_it:
> - /* When UNDEF_MAP is NULL, which indicates we are called from
> - do_lookup_x on relocation against protected data, we skip
> - the data definion in the executable from copy reloc. */
> - if (ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA
> - && undef_map == NULL
> - && map->l_type == lt_executable
> - && type_class == ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA)
> - {
> - const ElfW(Sym) *s;
> - unsigned int i;
> -
> -#if ! ELF_MACHINE_NO_RELA
> - if (map->l_info[DT_RELA] != NULL
> - && map->l_info[DT_RELASZ] != NULL
> - && map->l_info[DT_RELASZ]->d_un.d_val != 0)
> - {
> - const ElfW(Rela) *rela
> - = (const ElfW(Rela) *) D_PTR (map, l_info[DT_RELA]);
> - unsigned int rela_count
> - = map->l_info[DT_RELASZ]->d_un.d_val / sizeof (*rela);
> -
> - for (i = 0; i < rela_count; i++, rela++)
> - if (elf_machine_type_class (ELFW(R_TYPE) (rela->r_info))
> - == ELF_RTYPE_CLASS_COPY)
> - {
> - s = &symtab[ELFW(R_SYM) (rela->r_info)];
> - if (!strcmp (strtab + s->st_name, undef_name))
> - goto skip;
> - }
> - }
> -#endif
> -#if ! ELF_MACHINE_NO_REL
> - if (map->l_info[DT_REL] != NULL
> - && map->l_info[DT_RELSZ] != NULL
> - && map->l_info[DT_RELSZ]->d_un.d_val != 0)
> - {
> - const ElfW(Rel) *rel
> - = (const ElfW(Rel) *) D_PTR (map, l_info[DT_REL]);
> - unsigned int rel_count
> - = map->l_info[DT_RELSZ]->d_un.d_val / sizeof (*rel);
> -
> - for (i = 0; i < rel_count; i++, rel++)
> - if (elf_machine_type_class (ELFW(R_TYPE) (rel->r_info))
> - == ELF_RTYPE_CLASS_COPY)
> - {
> - s = &symtab[ELFW(R_SYM) (rel->r_info)];
> - if (!strcmp (strtab + s->st_name, undef_name))
> - goto skip;
> - }
> - }
> -#endif
> - }
> -
> /* Hidden and internal symbols are local, ignore them. */
> if (__glibc_unlikely (dl_symbol_visibility_binds_local_p (sym)))
> goto skip;
> @@ -875,12 +822,7 @@ _dl_lookup_symbol_x (const char *undef_name, struct link_map *undef_map,
> for (scope = symbol_scope; *scope != NULL; i = 0, ++scope)
> if (do_lookup_x (undef_name, new_hash, &old_hash, *ref,
> &protected_value, *scope, i, version, flags,
> - skip_map,
> - (ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA
> - && ELFW(ST_TYPE) ((*ref)->st_info) == STT_OBJECT
> - && type_class == ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA)
> - ? ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA
> - : ELF_RTYPE_CLASS_PLT, NULL) != 0)
> + skip_map, ELF_RTYPE_CLASS_PLT, NULL) != 0)
> break;
>
> if (protected_value.s != NULL && protected_value.m != undef_map)
> diff --git a/sysdeps/arc/dl-sysdep.h b/sysdeps/arc/dl-sysdep.h
> deleted file mode 100644
> index cf4d160a73..0000000000
> --- a/sysdeps/arc/dl-sysdep.h
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -/* System-specific settings for dynamic linker code. ARC version.
> - Copyright (C) 2020-2022 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/>. */
> -
> -#include_next <dl-sysdep.h>
> -
> -#define DL_EXTERN_PROTECTED_DATA
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 446d12a68c..050a3032de 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -149,23 +149,13 @@ dl_symbol_visibility_binds_local_p (const ElfW(Sym) *sym)
> satisfied by any symbol in the executable. Some architectures do
> not support copy relocations. In this case we define the macro to
> zero so that the code for handling them gets automatically optimized
> - out. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA means address of protected
> - data defined in the shared library may be external, i.e., due to copy
> - relocation. */
> + out. */
> #define ELF_RTYPE_CLASS_PLT 1
> #ifndef DL_NO_COPY_RELOCS
> # define ELF_RTYPE_CLASS_COPY 2
> #else
> # define ELF_RTYPE_CLASS_COPY 0
> #endif
> -/* If DL_EXTERN_PROTECTED_DATA is defined, address of protected data
> - defined in the shared library may be external, i.e., due to copy
> - relocation. */
> -#ifdef DL_EXTERN_PROTECTED_DATA
> -# define ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA 4
> -#else
> -# define ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA 0
> -#endif
>
> /* ELF uses the PF_x macros to specify the segment permissions, mmap
> uses PROT_xxx. In most cases the three macros have the values 1, 2,
> diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h
> index 8779983c8c..a8ee594ff4 100644
> --- a/sysdeps/i386/dl-machine.h
> +++ b/sysdeps/i386/dl-machine.h
> @@ -194,8 +194,7 @@ _dl_start_user:\n\
> || (type) == R_386_TLS_DTPOFF32 || (type) == R_386_TLS_TPOFF32 \
> || (type) == R_386_TLS_TPOFF || (type) == R_386_TLS_DESC) \
> * ELF_RTYPE_CLASS_PLT) \
> - | (((type) == R_386_COPY) * ELF_RTYPE_CLASS_COPY) \
> - | (((type) == R_386_GLOB_DAT) * ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA))
> + | (((type) == R_386_COPY) * ELF_RTYPE_CLASS_COPY))
>
> /* A reloc type used for ld.so cmdline arg lookups to reject PLT entries. */
> #define ELF_MACHINE_JMP_SLOT R_386_JMP_SLOT
> diff --git a/sysdeps/nios2/dl-sysdep.h b/sysdeps/nios2/dl-sysdep.h
> deleted file mode 100644
> index 257b37c258..0000000000
> --- a/sysdeps/nios2/dl-sysdep.h
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -/* System-specific settings for dynamic linker code. Nios II version.
> - Copyright (C) 2009-2022 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/>. */
> -
> -#include_next <dl-sysdep.h>
> -
> -#define DL_EXTERN_PROTECTED_DATA
> diff --git a/sysdeps/x86/dl-lookupcfg.h b/sysdeps/x86/dl-lookupcfg.h
> index 18b3b49f6e..e136cc63af 100644
> --- a/sysdeps/x86/dl-lookupcfg.h
> +++ b/sysdeps/x86/dl-lookupcfg.h
> @@ -20,10 +20,6 @@
>
> #include_next <dl-lookupcfg.h>
>
> -/* Address of protected data defined in the shared library may be
> - external due to copy relocation. */
> -#define DL_EXTERN_PROTECTED_DATA
> -
> struct link_map;
>
> extern void _dl_unmap (struct link_map *map) attribute_hidden;
> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
> index b9122944b9..06c9a932b0 100644
> --- a/sysdeps/x86_64/dl-machine.h
> +++ b/sysdeps/x86_64/dl-machine.h
> @@ -172,10 +172,7 @@ _dl_start_user:\n\
> TLS variable, so undefined references should not be allowed to
> define the value.
> ELF_RTYPE_CLASS_COPY iff TYPE should not be allowed to resolve to one
> - of the main executable's symbols, as for a COPY reloc.
> - ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA iff TYPE describes relocation may
> - against protected data whose address be external due to copy relocation.
> - */
> + of the main executable's symbols, as for a COPY reloc. */
> #define elf_machine_type_class(type) \
> ((((type) == R_X86_64_JUMP_SLOT \
> || (type) == R_X86_64_DTPMOD64 \
> @@ -183,8 +180,7 @@ _dl_start_user:\n\
> || (type) == R_X86_64_TPOFF64 \
> || (type) == R_X86_64_TLSDESC) \
> * ELF_RTYPE_CLASS_PLT) \
> - | (((type) == R_X86_64_COPY) * ELF_RTYPE_CLASS_COPY) \
> - | (((type) == R_X86_64_GLOB_DAT) * ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA))
> + | (((type) == R_X86_64_COPY) * ELF_RTYPE_CLASS_COPY))
>
> /* A reloc type used for ld.so cmdline arg lookups to reject PLT entries. */
> #define ELF_MACHINE_JMP_SLOT R_X86_64_JUMP_SLOT
> --
> 2.36.1.255.ge46751e96f-goog
>
More information about the Libc-alpha
mailing list