* [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling
@ 2022-05-01 6:06 Fangrui Song
2022-05-01 6:06 ` [PATCH 1/3] elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for non-DL_EXTERN_PROTECTED_DATA ports Fangrui Song
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Fangrui Song @ 2022-05-01 6:06 UTC (permalink / raw)
To: libc-alpha, Adhemerval Zanella, Szabolcs Nagy
Say both a.so and b.so define protected var and the executable copy
relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
semantics: a.so accesses the copy in the executable while b.so accesses
its own. This behavior requires that (a) the compiler emits
GOT-generating relocations (b) the linker produces GLOB_DAT instead of
RELATIVE.
Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
will bind to the executable (normal behavior).
For aarch64/arm it makes sense to restore the original behavior and don't
pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
unlikely used by anyone.
* Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
no GOT-generating relocation in the first place.
* gold and lld reject copy relocation on a STV_PROTECTED symbol.
* Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
GOT-generating relocation when accessing an default visibility
external symbol which avoids copy relocation.
Fangrui Song (3):
elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for
non-DL_EXTERN_PROTECTED_DATA ports
Revert "[AArch64][BZ #17711] Fix extern protected data handling"
Revert "[ARM][BZ #17711] Fix extern protected data handling"
elf/dl-lookup.c | 46 ++++++++++++------------------------
sysdeps/aarch64/dl-machine.h | 13 +++++-----
sysdeps/aarch64/dl-sysdep.h | 2 --
sysdeps/arm/dl-machine.h | 10 +++-----
sysdeps/arm/dl-sysdep.h | 2 --
5 files changed, 24 insertions(+), 49 deletions(-)
--
2.36.0.464.gb9c8b46e94-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for non-DL_EXTERN_PROTECTED_DATA ports
2022-05-01 6:06 [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling Fangrui Song
@ 2022-05-01 6:06 ` Fangrui Song
2022-05-01 6:06 ` [PATCH 2/3] Revert "[AArch64][BZ #17711] Fix extern protected data handling" Fangrui Song
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Fangrui Song @ 2022-05-01 6:06 UTC (permalink / raw)
To: libc-alpha, Adhemerval Zanella, Szabolcs Nagy
A port not defining DL_EXTERN_PROTECTED_DATA should not incur the
overhead. In addition, taking the address of a protected function never
works (either breaks pointer equality or rejected by ld). Just remove
the code related to functions.
---
elf/dl-lookup.c | 46 +++++++++++++++-------------------------------
1 file changed, 15 insertions(+), 31 deletions(-)
diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
index 989b073e4f..1bb80688ad 100644
--- a/elf/dl-lookup.c
+++ b/elf/dl-lookup.c
@@ -863,40 +863,24 @@ _dl_lookup_symbol_x (const char *undef_name, struct link_map *undef_map,
return 0;
}
- int protected = (*ref
- && ELFW(ST_VISIBILITY) ((*ref)->st_other) == STV_PROTECTED);
- if (__glibc_unlikely (protected != 0))
+ if (ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA
+ && __glibc_unlikely (
+ *ref && ELFW (ST_VISIBILITY) ((*ref)->st_other) == STV_PROTECTED
+ && ELFW (ST_TYPE) ((*ref)->st_info) == STT_OBJECT))
{
- /* It is very tricky. We need to figure out what value to
- return for the protected symbol. */
- if (type_class == ELF_RTYPE_CLASS_PLT)
- {
- if (current_value.s != NULL && current_value.m != undef_map)
- {
- current_value.s = *ref;
- current_value.m = undef_map;
- }
- }
- else
- {
- struct sym_val protected_value = { NULL, NULL };
+ struct sym_val protected_value = { NULL, NULL };
- 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)
- break;
+ 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, NULL)
+ != 0)
+ break;
- if (protected_value.s != NULL && protected_value.m != undef_map)
- {
- current_value.s = *ref;
- current_value.m = undef_map;
- }
+ if (protected_value.s != NULL && protected_value.m != undef_map)
+ {
+ current_value.s = *ref;
+ current_value.m = undef_map;
}
}
--
2.36.0.464.gb9c8b46e94-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/3] Revert "[AArch64][BZ #17711] Fix extern protected data handling"
2022-05-01 6:06 [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling Fangrui Song
2022-05-01 6:06 ` [PATCH 1/3] elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for non-DL_EXTERN_PROTECTED_DATA ports Fangrui Song
@ 2022-05-01 6:06 ` Fangrui Song
2022-05-23 20:10 ` Szabolcs Nagy
2022-05-01 6:06 ` [PATCH 3/3] Revert "[ARM][BZ " Fangrui Song
2022-05-09 21:26 ` [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling H.J. Lu
3 siblings, 1 reply; 15+ messages in thread
From: Fangrui Song @ 2022-05-01 6:06 UTC (permalink / raw)
To: libc-alpha, Adhemerval Zanella, Szabolcs Nagy
This reverts commit 0910702c4d2cf9e8302b35c9519548726e1ac489.
Say both a.so and b.so define protected var and the executable copy
relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
semantics: a.so accesses the copy in the executable while b.so accesses
its own. This behavior requires that (a) the compiler emits
GOT-generating relocations (b) the linker produces GLOB_DAT instead of
RELATIVE.
Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
will bind to the executable (normal behavior).
For aarch64 it makes sense to restore the original behavior and don't
pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
unlikely used by anyone.
* Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
no GOT-generating relocation in the first place.
* gold and lld reject copy relocation on a STV_PROTECTED symbol.
* Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
GOT-generating relocation when accessing an default visibility
external symbol which avoids copy relocation.
---
sysdeps/aarch64/dl-machine.h | 13 ++++++-------
sysdeps/aarch64/dl-sysdep.h | 2 --
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/sysdeps/aarch64/dl-machine.h b/sysdeps/aarch64/dl-machine.h
index b40050a981..530952a736 100644
--- a/sysdeps/aarch64/dl-machine.h
+++ b/sysdeps/aarch64/dl-machine.h
@@ -182,13 +182,12 @@ _dl_start_user: \n\
");
#define elf_machine_type_class(type) \
- ((((type) == AARCH64_R(JUMP_SLOT) \
- || (type) == AARCH64_R(TLS_DTPMOD) \
- || (type) == AARCH64_R(TLS_DTPREL) \
- || (type) == AARCH64_R(TLS_TPREL) \
- || (type) == AARCH64_R(TLSDESC)) * ELF_RTYPE_CLASS_PLT) \
- | (((type) == AARCH64_R(COPY)) * ELF_RTYPE_CLASS_COPY) \
- | (((type) == AARCH64_R(GLOB_DAT)) * ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA))
+ ((((type) == R_AARCH64_JUMP_SLOT || \
+ (type) == R_AARCH64_TLS_DTPMOD || \
+ (type) == R_AARCH64_TLS_DTPREL || \
+ (type) == R_AARCH64_TLS_TPREL || \
+ (type) == R_AARCH64_TLSDESC) * ELF_RTYPE_CLASS_PLT) \
+ | (((type) == R_AARCH64_COPY) * ELF_RTYPE_CLASS_COPY))
#define ELF_MACHINE_JMP_SLOT AARCH64_R(JUMP_SLOT)
diff --git a/sysdeps/aarch64/dl-sysdep.h b/sysdeps/aarch64/dl-sysdep.h
index 667786671c..ac69f414f3 100644
--- a/sysdeps/aarch64/dl-sysdep.h
+++ b/sysdeps/aarch64/dl-sysdep.h
@@ -21,5 +21,3 @@
/* _dl_argv cannot be attribute_relro, because _dl_start_user
might write into it after _dl_start returns. */
#define DL_ARGV_NOT_RELRO 1
-
-#define DL_EXTERN_PROTECTED_DATA
--
2.36.0.464.gb9c8b46e94-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] Revert "[ARM][BZ #17711] Fix extern protected data handling"
2022-05-01 6:06 [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling Fangrui Song
2022-05-01 6:06 ` [PATCH 1/3] elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for non-DL_EXTERN_PROTECTED_DATA ports Fangrui Song
2022-05-01 6:06 ` [PATCH 2/3] Revert "[AArch64][BZ #17711] Fix extern protected data handling" Fangrui Song
@ 2022-05-01 6:06 ` Fangrui Song
2022-05-23 20:11 ` Szabolcs Nagy
2022-05-09 21:26 ` [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling H.J. Lu
3 siblings, 1 reply; 15+ messages in thread
From: Fangrui Song @ 2022-05-01 6:06 UTC (permalink / raw)
To: libc-alpha, Adhemerval Zanella, Szabolcs Nagy
This reverts commit 3bcea719ddd6ce399d7bccb492c40af77d216e42.
---
sysdeps/arm/dl-machine.h | 10 +++-------
sysdeps/arm/dl-sysdep.h | 2 --
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/sysdeps/arm/dl-machine.h b/sysdeps/arm/dl-machine.h
index a7898bf420..2a7f795e46 100644
--- a/sysdeps/arm/dl-machine.h
+++ b/sysdeps/arm/dl-machine.h
@@ -212,22 +212,18 @@ _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 against
- protected data whose address may be external due to copy relocation. */
+ of the main executable's symbols, as for a COPY reloc. */
#ifndef RTLD_BOOTSTRAP
# define elf_machine_type_class(type) \
((((type) == R_ARM_JUMP_SLOT || (type) == R_ARM_TLS_DTPMOD32 \
|| (type) == R_ARM_TLS_DTPOFF32 || (type) == R_ARM_TLS_TPOFF32 \
|| (type) == R_ARM_TLS_DESC) \
* ELF_RTYPE_CLASS_PLT) \
- | (((type) == R_ARM_COPY) * ELF_RTYPE_CLASS_COPY) \
- | (((type) == R_ARM_GLOB_DAT) * ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA))
+ | (((type) == R_ARM_COPY) * ELF_RTYPE_CLASS_COPY))
#else
#define elf_machine_type_class(type) \
((((type) == R_ARM_JUMP_SLOT) * ELF_RTYPE_CLASS_PLT) \
- | (((type) == R_ARM_COPY) * ELF_RTYPE_CLASS_COPY) \
- | (((type) == R_ARM_GLOB_DAT) * ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA))
+ | (((type) == R_ARM_COPY) * ELF_RTYPE_CLASS_COPY))
#endif
/* A reloc type used for ld.so cmdline arg lookups to reject PLT entries. */
diff --git a/sysdeps/arm/dl-sysdep.h b/sysdeps/arm/dl-sysdep.h
index ce7a84a7de..3099ee419f 100644
--- a/sysdeps/arm/dl-sysdep.h
+++ b/sysdeps/arm/dl-sysdep.h
@@ -21,5 +21,3 @@
/* _dl_argv cannot be attribute_relro, because _dl_start_user
might write into it after _dl_start returns. */
#define DL_ARGV_NOT_RELRO 1
-
-#define DL_EXTERN_PROTECTED_DATA
--
2.36.0.464.gb9c8b46e94-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling
2022-05-01 6:06 [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling Fangrui Song
` (2 preceding siblings ...)
2022-05-01 6:06 ` [PATCH 3/3] Revert "[ARM][BZ " Fangrui Song
@ 2022-05-09 21:26 ` H.J. Lu
2022-05-10 7:42 ` Fangrui Song
3 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2022-05-09 21:26 UTC (permalink / raw)
To: Fangrui Song; +Cc: libc-alpha, Adhemerval Zanella, Szabolcs Nagy
On Sat, Apr 30, 2022 at 11:06 PM Fangrui Song via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Say both a.so and b.so define protected var and the executable copy
> relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
> semantics: a.so accesses the copy in the executable while b.so accesses
> its own. This behavior requires that (a) the compiler emits
> GOT-generating relocations (b) the linker produces GLOB_DAT instead of
> RELATIVE.
>
> Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
> will bind to the executable (normal behavior).
>
> For aarch64/arm it makes sense to restore the original behavior and don't
> pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
> unlikely used by anyone.
>
> * Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
> no GOT-generating relocation in the first place.
> * gold and lld reject copy relocation on a STV_PROTECTED symbol.
> * Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
> GOT-generating relocation when accessing an default visibility
> external symbol which avoids copy relocation.
>
> Fangrui Song (3):
> elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for
> non-DL_EXTERN_PROTECTED_DATA ports
> Revert "[AArch64][BZ #17711] Fix extern protected data handling"
> Revert "[ARM][BZ #17711] Fix extern protected data handling"
>
> elf/dl-lookup.c | 46 ++++++++++++------------------------
> sysdeps/aarch64/dl-machine.h | 13 +++++-----
> sysdeps/aarch64/dl-sysdep.h | 2 --
> sysdeps/arm/dl-machine.h | 10 +++-----
> sysdeps/arm/dl-sysdep.h | 2 --
> 5 files changed, 24 insertions(+), 49 deletions(-)
>
> --
> 2.36.0.464.gb9c8b46e94-goog
>
Given that protected symbols never work properly with copy relocation,
can we change protected symbol handling to simply warn copy relocation
against protected data symbol and non-zero symbol values of undefined
symbols against protected function symbols?
--
H.J.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling
2022-05-09 21:26 ` [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling H.J. Lu
@ 2022-05-10 7:42 ` Fangrui Song
2022-05-10 15:02 ` H.J. Lu
0 siblings, 1 reply; 15+ messages in thread
From: Fangrui Song @ 2022-05-10 7:42 UTC (permalink / raw)
To: H.J. Lu; +Cc: libc-alpha, Adhemerval Zanella, Szabolcs Nagy
On 2022-05-09, H.J. Lu wrote:
>On Sat, Apr 30, 2022 at 11:06 PM Fangrui Song via Libc-alpha
><libc-alpha@sourceware.org> wrote:
>>
>> Say both a.so and b.so define protected var and the executable copy
>> relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
>> semantics: a.so accesses the copy in the executable while b.so accesses
>> its own. This behavior requires that (a) the compiler emits
>> GOT-generating relocations (b) the linker produces GLOB_DAT instead of
>> RELATIVE.
>>
>> Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
>> will bind to the executable (normal behavior).
>>
>> For aarch64/arm it makes sense to restore the original behavior and don't
>> pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
>> unlikely used by anyone.
>>
>> * Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
>> no GOT-generating relocation in the first place.
>> * gold and lld reject copy relocation on a STV_PROTECTED symbol.
>> * Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
>> GOT-generating relocation when accessing an default visibility
>> external symbol which avoids copy relocation.
>>
>> Fangrui Song (3):
>> elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for
>> non-DL_EXTERN_PROTECTED_DATA ports
>> Revert "[AArch64][BZ #17711] Fix extern protected data handling"
>> Revert "[ARM][BZ #17711] Fix extern protected data handling"
>>
>> elf/dl-lookup.c | 46 ++++++++++++------------------------
>> sysdeps/aarch64/dl-machine.h | 13 +++++-----
>> sysdeps/aarch64/dl-sysdep.h | 2 --
>> sysdeps/arm/dl-machine.h | 10 +++-----
>> sysdeps/arm/dl-sysdep.h | 2 --
>> 5 files changed, 24 insertions(+), 49 deletions(-)
>>
>> --
>> 2.36.0.464.gb9c8b46e94-goog
>>
>
>Given that protected symbols never work properly with copy relocation,
>can we change protected symbol handling to simply warn copy relocation
>against protected data symbol and non-zero symbol values of undefined
>symbols against protected function symbols?
"protected symbols never work properly with copy relocation" generally
applies to all non-x86 architectures (arm/aarch64 has some guarantee,
but not reliable). My goal is indeed to remove relevant code for all
non-x86 architectures. They should not even bother testing
STV_PROTECTED.
On x86 there is some guarantee. The gcc>=5 -fpie
HAVE_LD_PIE_COPYRELOC behavior makes the problem trigger in some cases.
Having a warning in x86 specific files (e.g.
sysdeps/x86_64/dl-machine.h R_X86_64_COPY) seems appropriate.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling
2022-05-10 7:42 ` Fangrui Song
@ 2022-05-10 15:02 ` H.J. Lu
2022-05-12 4:56 ` Fangrui Song
0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2022-05-10 15:02 UTC (permalink / raw)
To: Fangrui Song; +Cc: GNU C Library, Adhemerval Zanella, Szabolcs Nagy
On Tue, May 10, 2022 at 12:42 AM Fangrui Song <maskray@google.com> wrote:
>
> On 2022-05-09, H.J. Lu wrote:
> >On Sat, Apr 30, 2022 at 11:06 PM Fangrui Song via Libc-alpha
> ><libc-alpha@sourceware.org> wrote:
> >>
> >> Say both a.so and b.so define protected var and the executable copy
> >> relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
> >> semantics: a.so accesses the copy in the executable while b.so accesses
> >> its own. This behavior requires that (a) the compiler emits
> >> GOT-generating relocations (b) the linker produces GLOB_DAT instead of
> >> RELATIVE.
> >>
> >> Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
> >> will bind to the executable (normal behavior).
> >>
> >> For aarch64/arm it makes sense to restore the original behavior and don't
> >> pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
> >> unlikely used by anyone.
> >>
> >> * Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
> >> no GOT-generating relocation in the first place.
> >> * gold and lld reject copy relocation on a STV_PROTECTED symbol.
> >> * Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
> >> GOT-generating relocation when accessing an default visibility
> >> external symbol which avoids copy relocation.
> >>
> >> Fangrui Song (3):
> >> elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for
> >> non-DL_EXTERN_PROTECTED_DATA ports
> >> Revert "[AArch64][BZ #17711] Fix extern protected data handling"
> >> Revert "[ARM][BZ #17711] Fix extern protected data handling"
> >>
> >> elf/dl-lookup.c | 46 ++++++++++++------------------------
> >> sysdeps/aarch64/dl-machine.h | 13 +++++-----
> >> sysdeps/aarch64/dl-sysdep.h | 2 --
> >> sysdeps/arm/dl-machine.h | 10 +++-----
> >> sysdeps/arm/dl-sysdep.h | 2 --
> >> 5 files changed, 24 insertions(+), 49 deletions(-)
> >>
> >> --
> >> 2.36.0.464.gb9c8b46e94-goog
> >>
> >
> >Given that protected symbols never work properly with copy relocation,
> >can we change protected symbol handling to simply warn copy relocation
> >against protected data symbol and non-zero symbol values of undefined
> >symbols against protected function symbols?
>
> "protected symbols never work properly with copy relocation" generally
> applies to all non-x86 architectures (arm/aarch64 has some guarantee,
> but not reliable). My goal is indeed to remove relevant code for all
> non-x86 architectures. They should not even bother testing
> STV_PROTECTED.
Even on x86, protected symbols don't work as intended. They don't
provide any performance benefits. I think we should move away from
copy relocation on protected symbols. We can either issue a warning
or an error.
> On x86 there is some guarantee. The gcc>=5 -fpie
> HAVE_LD_PIE_COPYRELOC behavior makes the problem trigger in some cases.
> Having a warning in x86 specific files (e.g.
> sysdeps/x86_64/dl-machine.h R_X86_64_COPY) seems appropriate.
--
H.J.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling
2022-05-10 15:02 ` H.J. Lu
@ 2022-05-12 4:56 ` Fangrui Song
2022-05-13 22:18 ` H.J. Lu
0 siblings, 1 reply; 15+ messages in thread
From: Fangrui Song @ 2022-05-12 4:56 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library, Adhemerval Zanella, Szabolcs Nagy
On 2022-05-10, H.J. Lu wrote:
>On Tue, May 10, 2022 at 12:42 AM Fangrui Song <maskray@google.com> wrote:
>>
>> On 2022-05-09, H.J. Lu wrote:
>> >On Sat, Apr 30, 2022 at 11:06 PM Fangrui Song via Libc-alpha
>> ><libc-alpha@sourceware.org> wrote:
>> >>
>> >> Say both a.so and b.so define protected var and the executable copy
>> >> relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
>> >> semantics: a.so accesses the copy in the executable while b.so accesses
>> >> its own. This behavior requires that (a) the compiler emits
>> >> GOT-generating relocations (b) the linker produces GLOB_DAT instead of
>> >> RELATIVE.
>> >>
>> >> Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
>> >> will bind to the executable (normal behavior).
>> >>
>> >> For aarch64/arm it makes sense to restore the original behavior and don't
>> >> pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
>> >> unlikely used by anyone.
>> >>
>> >> * Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
>> >> no GOT-generating relocation in the first place.
>> >> * gold and lld reject copy relocation on a STV_PROTECTED symbol.
>> >> * Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
>> >> GOT-generating relocation when accessing an default visibility
>> >> external symbol which avoids copy relocation.
>> >>
>> >> Fangrui Song (3):
>> >> elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for
>> >> non-DL_EXTERN_PROTECTED_DATA ports
>> >> Revert "[AArch64][BZ #17711] Fix extern protected data handling"
>> >> Revert "[ARM][BZ #17711] Fix extern protected data handling"
>> >>
>> >> elf/dl-lookup.c | 46 ++++++++++++------------------------
>> >> sysdeps/aarch64/dl-machine.h | 13 +++++-----
>> >> sysdeps/aarch64/dl-sysdep.h | 2 --
>> >> sysdeps/arm/dl-machine.h | 10 +++-----
>> >> sysdeps/arm/dl-sysdep.h | 2 --
>> >> 5 files changed, 24 insertions(+), 49 deletions(-)
>> >>
>> >> --
>> >> 2.36.0.464.gb9c8b46e94-goog
>> >>
>> >
>> >Given that protected symbols never work properly with copy relocation,
>> >can we change protected symbol handling to simply warn copy relocation
>> >against protected data symbol and non-zero symbol values of undefined
>> >symbols against protected function symbols?
>>
>> "protected symbols never work properly with copy relocation" generally
>> applies to all non-x86 architectures (arm/aarch64 has some guarantee,
>> but not reliable). My goal is indeed to remove relevant code for all
>> non-x86 architectures. They should not even bother testing
>> STV_PROTECTED.
>
>Even on x86, protected symbols don't work as intended. They don't
>provide any performance benefits. I think we should move away from
>copy relocation on protected symbols. We can either issue a warning
>or an error.
>
>> On x86 there is some guarantee. The gcc>=5 -fpie
>> HAVE_LD_PIE_COPYRELOC behavior makes the problem trigger in some cases.
>> Having a warning in x86 specific files (e.g.
>> sysdeps/x86_64/dl-machine.h R_X86_64_COPY) seems appropriate.
How about adding a warning for R_X86_64_COPY like the following, then removing
all code paths of (ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA!=0)?
From 3f905ff283178396840861a965acf35bd21cf5f0 Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@google.com>
Date: Wed, 11 May 2022 21:54:02 -0700
Subject: [PATCH] x86: Warn for copy relocations on a protected symbol
---
sysdeps/x86_64/dl-machine.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
index c70af7ab1e..48f08ecc0b 100644
--- a/sysdeps/x86_64/dl-machine.h
+++ b/sysdeps/x86_64/dl-machine.h
@@ -474,6 +474,12 @@ and creates an unsatisfiable circular dependency.\n",
break;
memcpy (reloc_addr_arg, (void *) value,
MIN (sym->st_size, refsym->st_size));
+ if (ELFW(ST_VISIBILITY) (sym->st_other) == STV_PROTECTED)
+ {
+ fmt = "\
+%s: Protected symbol `%s' is incompatible with copy relocations\n";
+ goto print_err;
+ }
if (__glibc_unlikely (sym->st_size > refsym->st_size)
|| (__glibc_unlikely (sym->st_size < refsym->st_size)
&& GLRO(dl_verbose)))
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling
2022-05-12 4:56 ` Fangrui Song
@ 2022-05-13 22:18 ` H.J. Lu
2022-05-13 23:09 ` Fangrui Song
0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2022-05-13 22:18 UTC (permalink / raw)
To: Fangrui Song; +Cc: GNU C Library, Adhemerval Zanella, Szabolcs Nagy
On Wed, May 11, 2022 at 9:56 PM Fangrui Song <maskray@google.com> wrote:
>
> On 2022-05-10, H.J. Lu wrote:
> >On Tue, May 10, 2022 at 12:42 AM Fangrui Song <maskray@google.com> wrote:
> >>
> >> On 2022-05-09, H.J. Lu wrote:
> >> >On Sat, Apr 30, 2022 at 11:06 PM Fangrui Song via Libc-alpha
> >> ><libc-alpha@sourceware.org> wrote:
> >> >>
> >> >> Say both a.so and b.so define protected var and the executable copy
> >> >> relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
> >> >> semantics: a.so accesses the copy in the executable while b.so accesses
> >> >> its own. This behavior requires that (a) the compiler emits
> >> >> GOT-generating relocations (b) the linker produces GLOB_DAT instead of
> >> >> RELATIVE.
> >> >>
> >> >> Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
> >> >> will bind to the executable (normal behavior).
> >> >>
> >> >> For aarch64/arm it makes sense to restore the original behavior and don't
> >> >> pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
> >> >> unlikely used by anyone.
> >> >>
> >> >> * Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
> >> >> no GOT-generating relocation in the first place.
> >> >> * gold and lld reject copy relocation on a STV_PROTECTED symbol.
> >> >> * Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
> >> >> GOT-generating relocation when accessing an default visibility
> >> >> external symbol which avoids copy relocation.
> >> >>
> >> >> Fangrui Song (3):
> >> >> elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for
> >> >> non-DL_EXTERN_PROTECTED_DATA ports
> >> >> Revert "[AArch64][BZ #17711] Fix extern protected data handling"
> >> >> Revert "[ARM][BZ #17711] Fix extern protected data handling"
> >> >>
> >> >> elf/dl-lookup.c | 46 ++++++++++++------------------------
> >> >> sysdeps/aarch64/dl-machine.h | 13 +++++-----
> >> >> sysdeps/aarch64/dl-sysdep.h | 2 --
> >> >> sysdeps/arm/dl-machine.h | 10 +++-----
> >> >> sysdeps/arm/dl-sysdep.h | 2 --
> >> >> 5 files changed, 24 insertions(+), 49 deletions(-)
> >> >>
> >> >> --
> >> >> 2.36.0.464.gb9c8b46e94-goog
> >> >>
> >> >
> >> >Given that protected symbols never work properly with copy relocation,
> >> >can we change protected symbol handling to simply warn copy relocation
> >> >against protected data symbol and non-zero symbol values of undefined
> >> >symbols against protected function symbols?
> >>
> >> "protected symbols never work properly with copy relocation" generally
> >> applies to all non-x86 architectures (arm/aarch64 has some guarantee,
> >> but not reliable). My goal is indeed to remove relevant code for all
> >> non-x86 architectures. They should not even bother testing
> >> STV_PROTECTED.
> >
> >Even on x86, protected symbols don't work as intended. They don't
> >provide any performance benefits. I think we should move away from
> >copy relocation on protected symbols. We can either issue a warning
> >or an error.
> >
> >> On x86 there is some guarantee. The gcc>=5 -fpie
> >> HAVE_LD_PIE_COPYRELOC behavior makes the problem trigger in some cases.
> >> Having a warning in x86 specific files (e.g.
> >> sysdeps/x86_64/dl-machine.h R_X86_64_COPY) seems appropriate.
>
> How about adding a warning for R_X86_64_COPY like the following, then removing
> all code paths of (ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA!=0)?
>
> From 3f905ff283178396840861a965acf35bd21cf5f0 Mon Sep 17 00:00:00 2001
> From: Fangrui Song <maskray@google.com>
> Date: Wed, 11 May 2022 21:54:02 -0700
> Subject: [PATCH] x86: Warn for copy relocations on a protected symbol
>
> ---
> sysdeps/x86_64/dl-machine.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
> index c70af7ab1e..48f08ecc0b 100644
> --- a/sysdeps/x86_64/dl-machine.h
> +++ b/sysdeps/x86_64/dl-machine.h
> @@ -474,6 +474,12 @@ and creates an unsatisfiable circular dependency.\n",
> break;
> memcpy (reloc_addr_arg, (void *) value,
> MIN (sym->st_size, refsym->st_size));
> + if (ELFW(ST_VISIBILITY) (sym->st_other) == STV_PROTECTED)
> + {
> + fmt = "\
> +%s: Protected symbol `%s' is incompatible with copy relocations\n";
> + goto print_err;
> + }
> if (__glibc_unlikely (sym->st_size > refsym->st_size)
> || (__glibc_unlikely (sym->st_size < refsym->st_size)
> && GLRO(dl_verbose)))
Should it be handled in sysdeps/generic/dl-protected.h?
BTW, I'd like to see a migration plan to remove copy relocation and
use protected
visibility in shared libraries. Qt uses protected/default
visibilities to mark exported
and imported symbols with GCC 12:
https://github.com/qt/qtbase/commit/19b7f854a274812d9c95fc7aaf134a12530c105f
Should glibc provide some EXPORT and IMPORT macros to help this?
--
H.J.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling
2022-05-13 22:18 ` H.J. Lu
@ 2022-05-13 23:09 ` Fangrui Song
0 siblings, 0 replies; 15+ messages in thread
From: Fangrui Song @ 2022-05-13 23:09 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library, Adhemerval Zanella, Szabolcs Nagy
On 2022-05-13, H.J. Lu wrote:
>On Wed, May 11, 2022 at 9:56 PM Fangrui Song <maskray@google.com> wrote:
>>
>> On 2022-05-10, H.J. Lu wrote:
>> >On Tue, May 10, 2022 at 12:42 AM Fangrui Song <maskray@google.com> wrote:
>> >>
>> >> On 2022-05-09, H.J. Lu wrote:
>> >> >On Sat, Apr 30, 2022 at 11:06 PM Fangrui Song via Libc-alpha
>> >> ><libc-alpha@sourceware.org> wrote:
>> >> >>
>> >> >> Say both a.so and b.so define protected var and the executable copy
>> >> >> relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
>> >> >> semantics: a.so accesses the copy in the executable while b.so accesses
>> >> >> its own. This behavior requires that (a) the compiler emits
>> >> >> GOT-generating relocations (b) the linker produces GLOB_DAT instead of
>> >> >> RELATIVE.
>> >> >>
>> >> >> Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
>> >> >> will bind to the executable (normal behavior).
>> >> >>
>> >> >> For aarch64/arm it makes sense to restore the original behavior and don't
>> >> >> pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
>> >> >> unlikely used by anyone.
>> >> >>
>> >> >> * Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
>> >> >> no GOT-generating relocation in the first place.
>> >> >> * gold and lld reject copy relocation on a STV_PROTECTED symbol.
>> >> >> * Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
>> >> >> GOT-generating relocation when accessing an default visibility
>> >> >> external symbol which avoids copy relocation.
>> >> >>
>> >> >> Fangrui Song (3):
>> >> >> elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for
>> >> >> non-DL_EXTERN_PROTECTED_DATA ports
>> >> >> Revert "[AArch64][BZ #17711] Fix extern protected data handling"
>> >> >> Revert "[ARM][BZ #17711] Fix extern protected data handling"
>> >> >>
>> >> >> elf/dl-lookup.c | 46 ++++++++++++------------------------
>> >> >> sysdeps/aarch64/dl-machine.h | 13 +++++-----
>> >> >> sysdeps/aarch64/dl-sysdep.h | 2 --
>> >> >> sysdeps/arm/dl-machine.h | 10 +++-----
>> >> >> sysdeps/arm/dl-sysdep.h | 2 --
>> >> >> 5 files changed, 24 insertions(+), 49 deletions(-)
>> >> >>
>> >> >> --
>> >> >> 2.36.0.464.gb9c8b46e94-goog
>> >> >>
>> >> >
>> >> >Given that protected symbols never work properly with copy relocation,
>> >> >can we change protected symbol handling to simply warn copy relocation
>> >> >against protected data symbol and non-zero symbol values of undefined
>> >> >symbols against protected function symbols?
>> >>
>> >> "protected symbols never work properly with copy relocation" generally
>> >> applies to all non-x86 architectures (arm/aarch64 has some guarantee,
>> >> but not reliable). My goal is indeed to remove relevant code for all
>> >> non-x86 architectures. They should not even bother testing
>> >> STV_PROTECTED.
>> >
>> >Even on x86, protected symbols don't work as intended. They don't
>> >provide any performance benefits. I think we should move away from
>> >copy relocation on protected symbols. We can either issue a warning
>> >or an error.
>> >
>> >> On x86 there is some guarantee. The gcc>=5 -fpie
>> >> HAVE_LD_PIE_COPYRELOC behavior makes the problem trigger in some cases.
>> >> Having a warning in x86 specific files (e.g.
>> >> sysdeps/x86_64/dl-machine.h R_X86_64_COPY) seems appropriate.
>>
>> How about adding a warning for R_X86_64_COPY like the following, then removing
>> all code paths of (ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA!=0)?
>>
>> From 3f905ff283178396840861a965acf35bd21cf5f0 Mon Sep 17 00:00:00 2001
>> From: Fangrui Song <maskray@google.com>
>> Date: Wed, 11 May 2022 21:54:02 -0700
>> Subject: [PATCH] x86: Warn for copy relocations on a protected symbol
>>
>> ---
>> sysdeps/x86_64/dl-machine.h | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
>> index c70af7ab1e..48f08ecc0b 100644
>> --- a/sysdeps/x86_64/dl-machine.h
>> +++ b/sysdeps/x86_64/dl-machine.h
>> @@ -474,6 +474,12 @@ and creates an unsatisfiable circular dependency.\n",
>> break;
>> memcpy (reloc_addr_arg, (void *) value,
>> MIN (sym->st_size, refsym->st_size));
>> + if (ELFW(ST_VISIBILITY) (sym->st_other) == STV_PROTECTED)
>> + {
>> + fmt = "\
>> +%s: Protected symbol `%s' is incompatible with copy relocations\n";
>> + goto print_err;
>> + }
>> if (__glibc_unlikely (sym->st_size > refsym->st_size)
>> || (__glibc_unlikely (sym->st_size < refsym->st_size)
>> && GLRO(dl_verbose)))
>
>Should it be handled in sysdeps/generic/dl-protected.h?
Yes, look like the suitable place. I think we should report a warning
regardless of GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS, at least for
all non-x86 architectures.
For x86, gating the logic under GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS may be fine
but I think it may be a bit overengineering as well.
>BTW, I'd like to see a migration plan to remove copy relocation and
>use protected
>visibility in shared libraries.
As mentioned, the elf/dl-lookup.c code is to handle the case when a
protected variable is defined in multiple shared objects and the
executable copy relocates it. This is a pathologic case and does not
have clear semantics. So IMO we should just remove the
ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code that let the second shared
object access its own copy.
I sent the patch series keeping ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA,
because I did not know whether you want to keep it for x86 :)
Note: the single protected data copy relocated by the executable does
not need any ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code to work. It
simply depends on whether the compiler and the linker cooperate and
produce a GLOB_DAT dynamic relocation.
>Qt uses protected/default
>visibilities to mark exported
>and imported symbols with GCC 12:
>
>https://github.com/qt/qtbase/commit/19b7f854a274812d9c95fc7aaf134a12530c105f
The Qt example belongs to the majority of cases where there is one
single definition for a protected variable. Removing the dl-lookup.c
STV_PROTECTED code won't affect it. If Qt ever defines the protected
variable in two shared objects, we can see that it is in a very broken
state.
And the original issue was really related to GCC 5's x86-64 decision to use
HAVE_LD_PIE_COPYRELOC. Other architectures don't have the problem.
In recent years most (if not all) distributions have switched to default
PIE. On all non-x86 architecture supporting shared objects, -fPIE
codegen uses an indirection for an external data symbol.
On the GCC side, we should just fix x86-64 -fpie by removing
HAVE_LD_PIE_COPYRELOC
(https://gcc.gnu.org/pipermail/gcc-patches/2021-November/582987.html).
>Should glibc provide some EXPORT and IMPORT macros to help this?
The -fno-pic codegen incompatibility with -Bsymbolic/protected/some
--dynamic-list has been there since like forever for architectures with
copy relocations. It doesn't help for glibc to tell GCC what to do.
Programs specifying -fno-pic likely rely on the existing behavior.
Changing GCC -fno-pic to default indirect access may be fine. That will
just let them opt out as they currently do for some security hardening
options pushed by Debian/Gentoo/RedHat/etc (e.g. -fno-stack-protector)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] Revert "[AArch64][BZ #17711] Fix extern protected data handling"
2022-05-01 6:06 ` [PATCH 2/3] Revert "[AArch64][BZ #17711] Fix extern protected data handling" Fangrui Song
@ 2022-05-23 20:10 ` Szabolcs Nagy
2022-05-23 20:17 ` Fangrui Song
0 siblings, 1 reply; 15+ messages in thread
From: Szabolcs Nagy @ 2022-05-23 20:10 UTC (permalink / raw)
To: Fangrui Song; +Cc: libc-alpha, Adhemerval Zanella
The 04/30/2022 23:06, Fangrui Song wrote:
> This reverts commit 0910702c4d2cf9e8302b35c9519548726e1ac489.
>
> Say both a.so and b.so define protected var and the executable copy
> relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
> semantics: a.so accesses the copy in the executable while b.so accesses
> its own. This behavior requires that (a) the compiler emits
> GOT-generating relocations (b) the linker produces GLOB_DAT instead of
> RELATIVE.
>
> Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
> will bind to the executable (normal behavior).
>
> For aarch64 it makes sense to restore the original behavior and don't
> pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
> unlikely used by anyone.
>
> * Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
> no GOT-generating relocation in the first place.
> * gold and lld reject copy relocation on a STV_PROTECTED symbol.
> * Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
> GOT-generating relocation when accessing an default visibility
> external symbol which avoids copy relocation.
this looks fine. i guess bfd ld should warn/reject copy relocs too
since it wont work well with protected visibility.
Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
> --- a/sysdeps/aarch64/dl-sysdep.h
> +++ b/sysdeps/aarch64/dl-sysdep.h
> @@ -21,5 +21,3 @@
> /* _dl_argv cannot be attribute_relro, because _dl_start_user
> might write into it after _dl_start returns. */
> #define DL_ARGV_NOT_RELRO 1
> -
> -#define DL_EXTERN_PROTECTED_DATA
i think this file can be removed after rebase
(DL_ARGV_NOT_RELRO got removed)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] Revert "[ARM][BZ #17711] Fix extern protected data handling"
2022-05-01 6:06 ` [PATCH 3/3] Revert "[ARM][BZ " Fangrui Song
@ 2022-05-23 20:11 ` Szabolcs Nagy
2022-05-23 20:45 ` Fangrui Song
0 siblings, 1 reply; 15+ messages in thread
From: Szabolcs Nagy @ 2022-05-23 20:11 UTC (permalink / raw)
To: Fangrui Song; +Cc: libc-alpha, Adhemerval Zanella
The 04/30/2022 23:06, Fangrui Song wrote:
> This reverts commit 3bcea719ddd6ce399d7bccb492c40af77d216e42.
i'd reference the previous commit here for details.
Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] Revert "[AArch64][BZ #17711] Fix extern protected data handling"
2022-05-23 20:10 ` Szabolcs Nagy
@ 2022-05-23 20:17 ` Fangrui Song
2022-05-24 5:13 ` Fangrui Song
0 siblings, 1 reply; 15+ messages in thread
From: Fangrui Song @ 2022-05-23 20:17 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, Adhemerval Zanella
On 2022-05-23, Szabolcs Nagy wrote:
>The 04/30/2022 23:06, Fangrui Song wrote:
>> This reverts commit 0910702c4d2cf9e8302b35c9519548726e1ac489.
>>
>> Say both a.so and b.so define protected var and the executable copy
>> relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
>> semantics: a.so accesses the copy in the executable while b.so accesses
>> its own. This behavior requires that (a) the compiler emits
>> GOT-generating relocations (b) the linker produces GLOB_DAT instead of
>> RELATIVE.
>>
>> Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
>> will bind to the executable (normal behavior).
>>
>> For aarch64 it makes sense to restore the original behavior and don't
>> pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
>> unlikely used by anyone.
>>
>> * Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
>> no GOT-generating relocation in the first place.
>> * gold and lld reject copy relocation on a STV_PROTECTED symbol.
>> * Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
>> GOT-generating relocation when accessing an default visibility
>> external symbol which avoids copy relocation.
>
>this looks fine. i guess bfd ld should warn/reject copy relocs too
>since it wont work well with protected visibility.
>
>Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
Thanks for review. I'll check GNU ld's aarch64 port if I get time.
>> --- a/sysdeps/aarch64/dl-sysdep.h
>> +++ b/sysdeps/aarch64/dl-sysdep.h
>> @@ -21,5 +21,3 @@
>> /* _dl_argv cannot be attribute_relro, because _dl_start_user
>> might write into it after _dl_start returns. */
>> #define DL_ARGV_NOT_RELRO 1
>> -
>> -#define DL_EXTERN_PROTECTED_DATA
>
>i think this file can be removed after rebase
>(DL_ARGV_NOT_RELRO got removed)
Ack. Will remove this file.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] Revert "[ARM][BZ #17711] Fix extern protected data handling"
2022-05-23 20:11 ` Szabolcs Nagy
@ 2022-05-23 20:45 ` Fangrui Song
0 siblings, 0 replies; 15+ messages in thread
From: Fangrui Song @ 2022-05-23 20:45 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, Adhemerval Zanella
On 2022-05-23, Szabolcs Nagy wrote:
>The 04/30/2022 23:06, Fangrui Song wrote:
>> This reverts commit 3bcea719ddd6ce399d7bccb492c40af77d216e42.
>
>i'd reference the previous commit here for details.
>
>Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
>
Thanks:)
Added "Similar to commit e555954e026df1b85b8ef6c101d05f97b1520d7e for aarch64." to the commit message and pushed.
When I sent the patch series I did not have a commit id for this arm patch :)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] Revert "[AArch64][BZ #17711] Fix extern protected data handling"
2022-05-23 20:17 ` Fangrui Song
@ 2022-05-24 5:13 ` Fangrui Song
0 siblings, 0 replies; 15+ messages in thread
From: Fangrui Song @ 2022-05-24 5:13 UTC (permalink / raw)
To: Szabolcs Nagy; +Cc: libc-alpha, Adhemerval Zanella
On 2022-05-23, Fangrui Song wrote:
>On 2022-05-23, Szabolcs Nagy wrote:
>>The 04/30/2022 23:06, Fangrui Song wrote:
>>>This reverts commit 0910702c4d2cf9e8302b35c9519548726e1ac489.
>>>
>>>Say both a.so and b.so define protected var and the executable copy
>>>relocates var. ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange
>>>semantics: a.so accesses the copy in the executable while b.so accesses
>>>its own. This behavior requires that (a) the compiler emits
>>>GOT-generating relocations (b) the linker produces GLOB_DAT instead of
>>>RELATIVE.
>>>
>>>Without the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA code, b.so's GLOB_DAT
>>>will bind to the executable (normal behavior).
>>>
>>>For aarch64 it makes sense to restore the original behavior and don't
>>>pay the ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA cost. The behavior is very
>>>unlikely used by anyone.
>>>
>>>* Clang code generation treats STV_PROTECTED the same way as STV_HIDDEN:
>>> no GOT-generating relocation in the first place.
>>>* gold and lld reject copy relocation on a STV_PROTECTED symbol.
>>>* Nowadays -fpie/-fpic modes are popular. GCC/Clang's codegen uses
>>> GOT-generating relocation when accessing an default visibility
>>> external symbol which avoids copy relocation.
>>
>>this looks fine. i guess bfd ld should warn/reject copy relocs too
>>since it wont work well with protected visibility.
>>
>>Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
>
>Thanks for review. I'll check GNU ld's aarch64 port if I get time.
I sent a GNU ld patch for review:
https://sourceware.org/pipermail/binutils/2022-May/120970.html
("[PATCH] aarch64: Disallow copy relocations on protected data")
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-05-24 5:14 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-01 6:06 [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling Fangrui Song
2022-05-01 6:06 ` [PATCH 1/3] elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA check for non-DL_EXTERN_PROTECTED_DATA ports Fangrui Song
2022-05-01 6:06 ` [PATCH 2/3] Revert "[AArch64][BZ #17711] Fix extern protected data handling" Fangrui Song
2022-05-23 20:10 ` Szabolcs Nagy
2022-05-23 20:17 ` Fangrui Song
2022-05-24 5:13 ` Fangrui Song
2022-05-01 6:06 ` [PATCH 3/3] Revert "[ARM][BZ " Fangrui Song
2022-05-23 20:11 ` Szabolcs Nagy
2022-05-23 20:45 ` Fangrui Song
2022-05-09 21:26 ` [PATCH 0/3] Simplify ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA and revert aarch64/arm's extern protected data handling H.J. Lu
2022-05-10 7:42 ` Fangrui Song
2022-05-10 15:02 ` H.J. Lu
2022-05-12 4:56 ` Fangrui Song
2022-05-13 22:18 ` H.J. Lu
2022-05-13 23:09 ` Fangrui Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).