[v4] elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA

  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.

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
  

On 2022-06-08, 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.
>
>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

Does this look good now?

We now always report the diagnostic for a copy relocation/canonical PLT entry on a protected symbol.
https://sourceware.org/pipermail/libc-alpha/2022-June/139733.html
(https://sourceware.org/git/?p=glibc.git;a=commit;h=7374c02b683b7110b853a32496a619410364d70b)
  

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
>
  

On 2022-06-15, Szabolcs Nagy wrote:
>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.

Thanks. I'll remove this stale paragraph and push it, given H.J's favor
previously.

>>
>> 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
>>