[2/3] Revert "[AArch64][BZ #17711] Fix extern protected data handling"
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| redhat-pt-bot/TryBot-still_applies |
warning
|
Patch no longer applies to master
|
Commit Message
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(-)
Comments
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)
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.
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")
@@ -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)
@@ -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