* [PATCH] aarch64: Disallow copy relocations on protected data
@ 2022-05-24 5:10 Fangrui Song
2022-05-27 22:43 ` Fangrui Song
[not found] ` <CAN30aBHDJM4KJAJJ_=4=vKgJL-yow2roUEUa7LFaAFaivkVswg@mail.gmail.com>
0 siblings, 2 replies; 8+ messages in thread
From: Fangrui Song @ 2022-05-24 5:10 UTC (permalink / raw)
To: binutils, Adhemerval Zanella, Szabolcs Nagy
The behavior matches ld.lld which has been used on many aarch64 OSes.
Rationale: copy relocations indicate that the executable and the shared object
would access different copies, if the shared object does not reference the
variable with GLOB_DAT.
Note: x86 requires GNU_PROPERTY_NO_COPY_ON_PROTECTED to have the error.
This is to largely due to GCC 5's
"x86-64: Optimize access to globals in PIE with copy reloc" which started to use
direct access relocations for external data symbols in -fpie mode.
GCC's aarch64 port does not have the change. Nowadays with most builds switching
to -fpie/-fpic, aarch64 mostly doesn't need to worry about copy relocations. So
for aarch64 we simply don't check GNU_PROPERTY_NO_COPY_ON_PROTECTED.
---
bfd/elfnn-aarch64.c | 28 ++++++++++++++++++-
ld/testsuite/ld-aarch64/aarch64-elf.exp | 9 ++++++
.../ld-aarch64/copy-reloc-protected.d | 2 ++
ld/testsuite/ld-aarch64/protected.s | 8 ++++++
4 files changed, 46 insertions(+), 1 deletion(-)
create mode 100644 ld/testsuite/ld-aarch64/copy-reloc-protected.d
create mode 100644 ld/testsuite/ld-aarch64/protected.s
diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index 4926bab9cf2..8896b0f78dd 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -2579,6 +2579,9 @@ struct elf_aarch64_link_hash_entry
this symbol. */
unsigned int got_type;
+ /* TRUE if symbol is defined as a protected symbol. */
+ unsigned int def_protected : 1;
+
/* A pointer to the most recently used stub hash entry against this
symbol. */
struct elf_aarch64_stub_hash_entry *stub_cache;
@@ -2855,9 +2858,16 @@ elfNN_aarch64_copy_indirect_symbol (struct bfd_link_info *info,
static void
elfNN_aarch64_merge_symbol_attribute (struct elf_link_hash_entry *h,
unsigned int st_other,
- bool definition ATTRIBUTE_UNUSED,
+ bool definition,
bool dynamic ATTRIBUTE_UNUSED)
{
+ if (definition)
+ {
+ struct elf_aarch64_link_hash_entry *eh
+ = (struct elf_aarch64_link_hash_entry *) h;
+ eh->def_protected = ELF_ST_VISIBILITY (st_other) == STV_PROTECTED;
+ }
+
unsigned int isym_sto = st_other & ~ELF_ST_VISIBILITY (-1);
unsigned int h_sto = h->other & ~ELF_ST_VISIBILITY (-1);
@@ -8701,6 +8711,22 @@ elfNN_aarch64_allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
if (h->dyn_relocs == NULL)
return true;
+ for (p = h->dyn_relocs; p != NULL; p = p->next)
+ if (eh->def_protected)
+ {
+ /* Disallow copy relocations against protected symbol. */
+ asection *s = p->sec->output_section;
+ if (s != NULL && (s->flags & SEC_READONLY) != 0)
+ {
+ info->callbacks->einfo
+ /* xgettext:c-format */
+ (_ ("%F%P: %pB: copy relocation against non-copyable "
+ "protected symbol `%s'\n"),
+ p->sec->owner, h->root.root.string);
+ return false;
+ }
+ }
+
/* In the shared -Bsymbolic case, discard space allocated for
dynamic pc-relative relocs against symbols which turn out to be
defined in regular objects. For the normal shared case, discard
diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
index 64476f111e0..31162277bd9 100644
--- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
+++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
@@ -407,6 +407,8 @@ set aarch64elflinktests {
{copy-reloc-exe-2.s} {{objdump -R copy-reloc-2.d}} "copy-reloc-2"}
{"ld-aarch64/exe with copy relocation elimination" "-e0 tmpdir/copy-reloc-so.so" "" ""
{copy-reloc-exe-eliminate.s} {{objdump -R copy-reloc-eliminate.d}} "copy-reloc-elimination"}
+ {"Build .so with protected data" "-shared" "" "" {protected.s}
+ {} "protected.so"}
{"ld-aarch64/so with global func" "-shared" "" "" {func-in-so.s}
{} "func-in-so.so"}
{"ld-aarch64/func sym hash opt for exe"
@@ -416,8 +418,15 @@ set aarch64elflinktests {
{} "libbti-plt-so.so"}
}
+set aarch64elfcclinktests [list \
+ [list "copy relocation on protected data" \
+ "-no-pie tmpdir/copy-reloc-exe.o tmpdir/protected.so" "" \
+ {} {{error_output copy-reloc-protected.d}} "copy-reloc-protected"]
+]
+
if [check_shared_lib_support] {
run_ld_link_tests $aarch64elflinktests
+ run_cc_link_tests $aarch64elfcclinktests
}
run_dump_test "bti-plt-3"
diff --git a/ld/testsuite/ld-aarch64/copy-reloc-protected.d b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
new file mode 100644
index 00000000000..99a356a3df6
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
@@ -0,0 +1,2 @@
+.*: tmpdir/copy-reloc-exe.o: copy relocation against non-copyable protected symbol `global_a'
+#...
diff --git a/ld/testsuite/ld-aarch64/protected.s b/ld/testsuite/ld-aarch64/protected.s
new file mode 100644
index 00000000000..eb3fb402dc4
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/protected.s
@@ -0,0 +1,8 @@
+.global global_a
+.protected global_a
+.type global_a, %object
+.size global_a, 4
+
+.data
+global_a:
+.word 0xcafedead
--
2.36
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] aarch64: Disallow copy relocations on protected data
2022-05-24 5:10 [PATCH] aarch64: Disallow copy relocations on protected data Fangrui Song
@ 2022-05-27 22:43 ` Fangrui Song
[not found] ` <CAN30aBHDJM4KJAJJ_=4=vKgJL-yow2roUEUa7LFaAFaivkVswg@mail.gmail.com>
1 sibling, 0 replies; 8+ messages in thread
From: Fangrui Song @ 2022-05-27 22:43 UTC (permalink / raw)
To: binutils, Adhemerval Zanella, Szabolcs Nagy, Nick Clifton
On Mon, May 23, 2022 at 10:10 PM Fangrui Song <i@maskray.me> wrote:
>
> The behavior matches ld.lld which has been used on many aarch64 OSes.
>
> Rationale: copy relocations indicate that the executable and the shared object
> would access different copies, if the shared object does not reference the
> variable with GLOB_DAT.
>
> Note: x86 requires GNU_PROPERTY_NO_COPY_ON_PROTECTED to have the error.
> This is to largely due to GCC 5's
> "x86-64: Optimize access to globals in PIE with copy reloc" which started to use
> direct access relocations for external data symbols in -fpie mode.
>
> GCC's aarch64 port does not have the change. Nowadays with most builds switching
> to -fpie/-fpic, aarch64 mostly doesn't need to worry about copy relocations. So
> for aarch64 we simply don't check GNU_PROPERTY_NO_COPY_ON_PROTECTED.
The diagnostic is requested on
https://sourceware.org/pipermail/libc-alpha/2022-May/139014.html
> ---
> bfd/elfnn-aarch64.c | 28 ++++++++++++++++++-
> ld/testsuite/ld-aarch64/aarch64-elf.exp | 9 ++++++
> .../ld-aarch64/copy-reloc-protected.d | 2 ++
> ld/testsuite/ld-aarch64/protected.s | 8 ++++++
> 4 files changed, 46 insertions(+), 1 deletion(-)
> create mode 100644 ld/testsuite/ld-aarch64/copy-reloc-protected.d
> create mode 100644 ld/testsuite/ld-aarch64/protected.s
>
> diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
> index 4926bab9cf2..8896b0f78dd 100644
> --- a/bfd/elfnn-aarch64.c
> +++ b/bfd/elfnn-aarch64.c
> @@ -2579,6 +2579,9 @@ struct elf_aarch64_link_hash_entry
> this symbol. */
> unsigned int got_type;
>
> + /* TRUE if symbol is defined as a protected symbol. */
> + unsigned int def_protected : 1;
> +
> /* A pointer to the most recently used stub hash entry against this
> symbol. */
> struct elf_aarch64_stub_hash_entry *stub_cache;
> @@ -2855,9 +2858,16 @@ elfNN_aarch64_copy_indirect_symbol (struct bfd_link_info *info,
> static void
> elfNN_aarch64_merge_symbol_attribute (struct elf_link_hash_entry *h,
> unsigned int st_other,
> - bool definition ATTRIBUTE_UNUSED,
> + bool definition,
> bool dynamic ATTRIBUTE_UNUSED)
> {
> + if (definition)
> + {
> + struct elf_aarch64_link_hash_entry *eh
> + = (struct elf_aarch64_link_hash_entry *) h;
> + eh->def_protected = ELF_ST_VISIBILITY (st_other) == STV_PROTECTED;
> + }
> +
> unsigned int isym_sto = st_other & ~ELF_ST_VISIBILITY (-1);
> unsigned int h_sto = h->other & ~ELF_ST_VISIBILITY (-1);
>
> @@ -8701,6 +8711,22 @@ elfNN_aarch64_allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
> if (h->dyn_relocs == NULL)
> return true;
>
> + for (p = h->dyn_relocs; p != NULL; p = p->next)
> + if (eh->def_protected)
> + {
> + /* Disallow copy relocations against protected symbol. */
> + asection *s = p->sec->output_section;
> + if (s != NULL && (s->flags & SEC_READONLY) != 0)
> + {
> + info->callbacks->einfo
> + /* xgettext:c-format */
> + (_ ("%F%P: %pB: copy relocation against non-copyable "
> + "protected symbol `%s'\n"),
> + p->sec->owner, h->root.root.string);
> + return false;
> + }
> + }
> +
> /* In the shared -Bsymbolic case, discard space allocated for
> dynamic pc-relative relocs against symbols which turn out to be
> defined in regular objects. For the normal shared case, discard
> diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
> index 64476f111e0..31162277bd9 100644
> --- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
> +++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
> @@ -407,6 +407,8 @@ set aarch64elflinktests {
> {copy-reloc-exe-2.s} {{objdump -R copy-reloc-2.d}} "copy-reloc-2"}
> {"ld-aarch64/exe with copy relocation elimination" "-e0 tmpdir/copy-reloc-so.so" "" ""
> {copy-reloc-exe-eliminate.s} {{objdump -R copy-reloc-eliminate.d}} "copy-reloc-elimination"}
> + {"Build .so with protected data" "-shared" "" "" {protected.s}
> + {} "protected.so"}
> {"ld-aarch64/so with global func" "-shared" "" "" {func-in-so.s}
> {} "func-in-so.so"}
> {"ld-aarch64/func sym hash opt for exe"
> @@ -416,8 +418,15 @@ set aarch64elflinktests {
> {} "libbti-plt-so.so"}
> }
>
> +set aarch64elfcclinktests [list \
> + [list "copy relocation on protected data" \
> + "-no-pie tmpdir/copy-reloc-exe.o tmpdir/protected.so" "" \
> + {} {{error_output copy-reloc-protected.d}} "copy-reloc-protected"]
> +]
> +
> if [check_shared_lib_support] {
> run_ld_link_tests $aarch64elflinktests
> + run_cc_link_tests $aarch64elfcclinktests
> }
>
> run_dump_test "bti-plt-3"
> diff --git a/ld/testsuite/ld-aarch64/copy-reloc-protected.d b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
> new file mode 100644
> index 00000000000..99a356a3df6
> --- /dev/null
> +++ b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
> @@ -0,0 +1,2 @@
> +.*: tmpdir/copy-reloc-exe.o: copy relocation against non-copyable protected symbol `global_a'
> +#...
> diff --git a/ld/testsuite/ld-aarch64/protected.s b/ld/testsuite/ld-aarch64/protected.s
> new file mode 100644
> index 00000000000..eb3fb402dc4
> --- /dev/null
> +++ b/ld/testsuite/ld-aarch64/protected.s
> @@ -0,0 +1,8 @@
> +.global global_a
> +.protected global_a
> +.type global_a, %object
> +.size global_a, 4
> +
> +.data
> +global_a:
> +.word 0xcafedead
> --
> 2.36
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* PING: [PATCH] aarch64: Disallow copy relocations on protected data
[not found] ` <CAN30aBHDJM4KJAJJ_=4=vKgJL-yow2roUEUa7LFaAFaivkVswg@mail.gmail.com>
@ 2022-06-01 5:53 ` Fangrui Song
[not found] ` <CAN30aBH68jT8ArEPEx_X-zVzuOmN3u_KV5HKc-Vgc2tVohJADw@mail.gmail.com>
1 sibling, 0 replies; 8+ messages in thread
From: Fangrui Song @ 2022-06-01 5:53 UTC (permalink / raw)
To: binutils, Adhemerval Zanella, Szabolcs Nagy, Nick Clifton
On Fri, May 27, 2022 at 3:43 PM Fangrui Song <i@maskray.me> wrote:
>
> On Mon, May 23, 2022 at 10:10 PM Fangrui Song <i@maskray.me> wrote:
> >
> > The behavior matches ld.lld which has been used on many aarch64 OSes.
> >
> > Rationale: copy relocations indicate that the executable and the shared object
> > would access different copies, if the shared object does not reference the
> > variable with GLOB_DAT.
> >
> > Note: x86 requires GNU_PROPERTY_NO_COPY_ON_PROTECTED to have the error.
> > This is to largely due to GCC 5's
> > "x86-64: Optimize access to globals in PIE with copy reloc" which started to use
> > direct access relocations for external data symbols in -fpie mode.
> >
> > GCC's aarch64 port does not have the change. Nowadays with most builds switching
> > to -fpie/-fpic, aarch64 mostly doesn't need to worry about copy relocations. So
> > for aarch64 we simply don't check GNU_PROPERTY_NO_COPY_ON_PROTECTED.
>
> The diagnostic is requested on
> https://sourceware.org/pipermail/libc-alpha/2022-May/139014.html
>
> > ---
> > bfd/elfnn-aarch64.c | 28 ++++++++++++++++++-
> > ld/testsuite/ld-aarch64/aarch64-elf.exp | 9 ++++++
> > .../ld-aarch64/copy-reloc-protected.d | 2 ++
> > ld/testsuite/ld-aarch64/protected.s | 8 ++++++
> > 4 files changed, 46 insertions(+), 1 deletion(-)
> > create mode 100644 ld/testsuite/ld-aarch64/copy-reloc-protected.d
> > create mode 100644 ld/testsuite/ld-aarch64/protected.s
> >
> > diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
> > index 4926bab9cf2..8896b0f78dd 100644
> > --- a/bfd/elfnn-aarch64.c
> > +++ b/bfd/elfnn-aarch64.c
> > @@ -2579,6 +2579,9 @@ struct elf_aarch64_link_hash_entry
> > this symbol. */
> > unsigned int got_type;
> >
> > + /* TRUE if symbol is defined as a protected symbol. */
> > + unsigned int def_protected : 1;
> > +
> > /* A pointer to the most recently used stub hash entry against this
> > symbol. */
> > struct elf_aarch64_stub_hash_entry *stub_cache;
> > @@ -2855,9 +2858,16 @@ elfNN_aarch64_copy_indirect_symbol (struct bfd_link_info *info,
> > static void
> > elfNN_aarch64_merge_symbol_attribute (struct elf_link_hash_entry *h,
> > unsigned int st_other,
> > - bool definition ATTRIBUTE_UNUSED,
> > + bool definition,
> > bool dynamic ATTRIBUTE_UNUSED)
> > {
> > + if (definition)
> > + {
> > + struct elf_aarch64_link_hash_entry *eh
> > + = (struct elf_aarch64_link_hash_entry *) h;
> > + eh->def_protected = ELF_ST_VISIBILITY (st_other) == STV_PROTECTED;
> > + }
> > +
> > unsigned int isym_sto = st_other & ~ELF_ST_VISIBILITY (-1);
> > unsigned int h_sto = h->other & ~ELF_ST_VISIBILITY (-1);
> >
> > @@ -8701,6 +8711,22 @@ elfNN_aarch64_allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
> > if (h->dyn_relocs == NULL)
> > return true;
> >
> > + for (p = h->dyn_relocs; p != NULL; p = p->next)
> > + if (eh->def_protected)
> > + {
> > + /* Disallow copy relocations against protected symbol. */
> > + asection *s = p->sec->output_section;
> > + if (s != NULL && (s->flags & SEC_READONLY) != 0)
> > + {
> > + info->callbacks->einfo
> > + /* xgettext:c-format */
> > + (_ ("%F%P: %pB: copy relocation against non-copyable "
> > + "protected symbol `%s'\n"),
> > + p->sec->owner, h->root.root.string);
> > + return false;
> > + }
> > + }
> > +
> > /* In the shared -Bsymbolic case, discard space allocated for
> > dynamic pc-relative relocs against symbols which turn out to be
> > defined in regular objects. For the normal shared case, discard
> > diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
> > index 64476f111e0..31162277bd9 100644
> > --- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
> > +++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
> > @@ -407,6 +407,8 @@ set aarch64elflinktests {
> > {copy-reloc-exe-2.s} {{objdump -R copy-reloc-2.d}} "copy-reloc-2"}
> > {"ld-aarch64/exe with copy relocation elimination" "-e0 tmpdir/copy-reloc-so.so" "" ""
> > {copy-reloc-exe-eliminate.s} {{objdump -R copy-reloc-eliminate.d}} "copy-reloc-elimination"}
> > + {"Build .so with protected data" "-shared" "" "" {protected.s}
> > + {} "protected.so"}
> > {"ld-aarch64/so with global func" "-shared" "" "" {func-in-so.s}
> > {} "func-in-so.so"}
> > {"ld-aarch64/func sym hash opt for exe"
> > @@ -416,8 +418,15 @@ set aarch64elflinktests {
> > {} "libbti-plt-so.so"}
> > }
> >
> > +set aarch64elfcclinktests [list \
> > + [list "copy relocation on protected data" \
> > + "-no-pie tmpdir/copy-reloc-exe.o tmpdir/protected.so" "" \
> > + {} {{error_output copy-reloc-protected.d}} "copy-reloc-protected"]
> > +]
> > +
> > if [check_shared_lib_support] {
> > run_ld_link_tests $aarch64elflinktests
> > + run_cc_link_tests $aarch64elfcclinktests
> > }
> >
> > run_dump_test "bti-plt-3"
> > diff --git a/ld/testsuite/ld-aarch64/copy-reloc-protected.d b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
> > new file mode 100644
> > index 00000000000..99a356a3df6
> > --- /dev/null
> > +++ b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
> > @@ -0,0 +1,2 @@
> > +.*: tmpdir/copy-reloc-exe.o: copy relocation against non-copyable protected symbol `global_a'
> > +#...
> > diff --git a/ld/testsuite/ld-aarch64/protected.s b/ld/testsuite/ld-aarch64/protected.s
> > new file mode 100644
> > index 00000000000..eb3fb402dc4
> > --- /dev/null
> > +++ b/ld/testsuite/ld-aarch64/protected.s
> > @@ -0,0 +1,8 @@
> > +.global global_a
> > +.protected global_a
> > +.type global_a, %object
> > +.size global_a, 4
> > +
> > +.data
> > +global_a:
> > +.word 0xcafedead
> > --
> > 2.36
> >
Ping
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: PING: [PATCH] aarch64: Disallow copy relocations on protected data
[not found] ` <CAN30aBH68jT8ArEPEx_X-zVzuOmN3u_KV5HKc-Vgc2tVohJADw@mail.gmail.com>
@ 2022-06-15 22:28 ` Fangrui Song
2022-06-21 19:27 ` Fangrui Song
[not found] ` <DS7PR12MB57658D646BB4944D4BECAF84CBB39@DS7PR12MB5765.namprd12.prod.outlook.com>
0 siblings, 2 replies; 8+ messages in thread
From: Fangrui Song @ 2022-06-15 22:28 UTC (permalink / raw)
To: binutils, Adhemerval Zanella, Szabolcs Nagy, Nick Clifton
On 2022-05-31, Fangrui Song wrote:
>On Fri, May 27, 2022 at 3:43 PM Fangrui Song <i@maskray.me> wrote:
>>
>> On Mon, May 23, 2022 at 10:10 PM Fangrui Song <i@maskray.me> wrote:
>> >
>> > The behavior matches ld.lld which has been used on many aarch64 OSes.
>> >
>> > Rationale: copy relocations indicate that the executable and the shared object
>> > would access different copies, if the shared object does not reference the
>> > variable with GLOB_DAT.
>> >
>> > Note: x86 requires GNU_PROPERTY_NO_COPY_ON_PROTECTED to have the error.
>> > This is to largely due to GCC 5's
>> > "x86-64: Optimize access to globals in PIE with copy reloc" which started to use
>> > direct access relocations for external data symbols in -fpie mode.
>> >
>> > GCC's aarch64 port does not have the change. Nowadays with most builds switching
>> > to -fpie/-fpic, aarch64 mostly doesn't need to worry about copy relocations. So
>> > for aarch64 we simply don't check GNU_PROPERTY_NO_COPY_ON_PROTECTED.
>>
>> The diagnostic is requested on
>> https://sourceware.org/pipermail/libc-alpha/2022-May/139014.html
>>
>> > ---
>> > bfd/elfnn-aarch64.c | 28 ++++++++++++++++++-
>> > ld/testsuite/ld-aarch64/aarch64-elf.exp | 9 ++++++
>> > .../ld-aarch64/copy-reloc-protected.d | 2 ++
>> > ld/testsuite/ld-aarch64/protected.s | 8 ++++++
>> > 4 files changed, 46 insertions(+), 1 deletion(-)
>> > create mode 100644 ld/testsuite/ld-aarch64/copy-reloc-protected.d
>> > create mode 100644 ld/testsuite/ld-aarch64/protected.s
>> >
>> > diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
>> > index 4926bab9cf2..8896b0f78dd 100644
>> > --- a/bfd/elfnn-aarch64.c
>> > +++ b/bfd/elfnn-aarch64.c
>> > @@ -2579,6 +2579,9 @@ struct elf_aarch64_link_hash_entry
>> > this symbol. */
>> > unsigned int got_type;
>> >
>> > + /* TRUE if symbol is defined as a protected symbol. */
>> > + unsigned int def_protected : 1;
>> > +
>> > /* A pointer to the most recently used stub hash entry against this
>> > symbol. */
>> > struct elf_aarch64_stub_hash_entry *stub_cache;
>> > @@ -2855,9 +2858,16 @@ elfNN_aarch64_copy_indirect_symbol (struct bfd_link_info *info,
>> > static void
>> > elfNN_aarch64_merge_symbol_attribute (struct elf_link_hash_entry *h,
>> > unsigned int st_other,
>> > - bool definition ATTRIBUTE_UNUSED,
>> > + bool definition,
>> > bool dynamic ATTRIBUTE_UNUSED)
>> > {
>> > + if (definition)
>> > + {
>> > + struct elf_aarch64_link_hash_entry *eh
>> > + = (struct elf_aarch64_link_hash_entry *) h;
>> > + eh->def_protected = ELF_ST_VISIBILITY (st_other) == STV_PROTECTED;
>> > + }
>> > +
>> > unsigned int isym_sto = st_other & ~ELF_ST_VISIBILITY (-1);
>> > unsigned int h_sto = h->other & ~ELF_ST_VISIBILITY (-1);
>> >
>> > @@ -8701,6 +8711,22 @@ elfNN_aarch64_allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
>> > if (h->dyn_relocs == NULL)
>> > return true;
>> >
>> > + for (p = h->dyn_relocs; p != NULL; p = p->next)
>> > + if (eh->def_protected)
>> > + {
>> > + /* Disallow copy relocations against protected symbol. */
>> > + asection *s = p->sec->output_section;
>> > + if (s != NULL && (s->flags & SEC_READONLY) != 0)
>> > + {
>> > + info->callbacks->einfo
>> > + /* xgettext:c-format */
>> > + (_ ("%F%P: %pB: copy relocation against non-copyable "
>> > + "protected symbol `%s'\n"),
>> > + p->sec->owner, h->root.root.string);
>> > + return false;
>> > + }
>> > + }
>> > +
>> > /* In the shared -Bsymbolic case, discard space allocated for
>> > dynamic pc-relative relocs against symbols which turn out to be
>> > defined in regular objects. For the normal shared case, discard
>> > diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
>> > index 64476f111e0..31162277bd9 100644
>> > --- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
>> > +++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
>> > @@ -407,6 +407,8 @@ set aarch64elflinktests {
>> > {copy-reloc-exe-2.s} {{objdump -R copy-reloc-2.d}} "copy-reloc-2"}
>> > {"ld-aarch64/exe with copy relocation elimination" "-e0 tmpdir/copy-reloc-so.so" "" ""
>> > {copy-reloc-exe-eliminate.s} {{objdump -R copy-reloc-eliminate.d}} "copy-reloc-elimination"}
>> > + {"Build .so with protected data" "-shared" "" "" {protected.s}
>> > + {} "protected.so"}
>> > {"ld-aarch64/so with global func" "-shared" "" "" {func-in-so.s}
>> > {} "func-in-so.so"}
>> > {"ld-aarch64/func sym hash opt for exe"
>> > @@ -416,8 +418,15 @@ set aarch64elflinktests {
>> > {} "libbti-plt-so.so"}
>> > }
>> >
>> > +set aarch64elfcclinktests [list \
>> > + [list "copy relocation on protected data" \
>> > + "-no-pie tmpdir/copy-reloc-exe.o tmpdir/protected.so" "" \
>> > + {} {{error_output copy-reloc-protected.d}} "copy-reloc-protected"]
>> > +]
>> > +
>> > if [check_shared_lib_support] {
>> > run_ld_link_tests $aarch64elflinktests
>> > + run_cc_link_tests $aarch64elfcclinktests
>> > }
>> >
>> > run_dump_test "bti-plt-3"
>> > diff --git a/ld/testsuite/ld-aarch64/copy-reloc-protected.d b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
>> > new file mode 100644
>> > index 00000000000..99a356a3df6
>> > --- /dev/null
>> > +++ b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
>> > @@ -0,0 +1,2 @@
>> > +.*: tmpdir/copy-reloc-exe.o: copy relocation against non-copyable protected symbol `global_a'
>> > +#...
>> > diff --git a/ld/testsuite/ld-aarch64/protected.s b/ld/testsuite/ld-aarch64/protected.s
>> > new file mode 100644
>> > index 00000000000..eb3fb402dc4
>> > --- /dev/null
>> > +++ b/ld/testsuite/ld-aarch64/protected.s
>> > @@ -0,0 +1,8 @@
>> > +.global global_a
>> > +.protected global_a
>> > +.type global_a, %object
>> > +.size global_a, 4
>> > +
>> > +.data
>> > +global_a:
>> > +.word 0xcafedead
>> > --
>> > 2.36
>> >
glibc now has a diagnostic for copy relocations on a protected symbol
https://sourceware.org/git/?p=glibc.git;a=commit;h=7374c02b683b7110b853a32496a619410364d70b :)
Let's catch the issue on ld side, too.
Ping
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: PING: [PATCH] aarch64: Disallow copy relocations on protected data
2022-06-15 22:28 ` Fangrui Song
@ 2022-06-21 19:27 ` Fangrui Song
[not found] ` <DS7PR12MB57658D646BB4944D4BECAF84CBB39@DS7PR12MB5765.namprd12.prod.outlook.com>
1 sibling, 0 replies; 8+ messages in thread
From: Fangrui Song @ 2022-06-21 19:27 UTC (permalink / raw)
To: binutils, Adhemerval Zanella, Szabolcs Nagy, Nick Clifton
On Wed, Jun 15, 2022 at 3:28 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2022-05-31, Fangrui Song wrote:
> >On Fri, May 27, 2022 at 3:43 PM Fangrui Song <i@maskray.me> wrote:
> >>
> >> On Mon, May 23, 2022 at 10:10 PM Fangrui Song <i@maskray.me> wrote:
> >> >
> >> > The behavior matches ld.lld which has been used on many aarch64 OSes.
> >> >
> >> > Rationale: copy relocations indicate that the executable and the shared object
> >> > would access different copies, if the shared object does not reference the
> >> > variable with GLOB_DAT.
> >> >
> >> > Note: x86 requires GNU_PROPERTY_NO_COPY_ON_PROTECTED to have the error.
> >> > This is to largely due to GCC 5's
> >> > "x86-64: Optimize access to globals in PIE with copy reloc" which started to use
> >> > direct access relocations for external data symbols in -fpie mode.
> >> >
> >> > GCC's aarch64 port does not have the change. Nowadays with most builds switching
> >> > to -fpie/-fpic, aarch64 mostly doesn't need to worry about copy relocations. So
> >> > for aarch64 we simply don't check GNU_PROPERTY_NO_COPY_ON_PROTECTED.
> >>
> >> The diagnostic is requested on
> >> https://sourceware.org/pipermail/libc-alpha/2022-May/139014.html
> >>
> >> > ---
> >> > bfd/elfnn-aarch64.c | 28 ++++++++++++++++++-
> >> > ld/testsuite/ld-aarch64/aarch64-elf.exp | 9 ++++++
> >> > .../ld-aarch64/copy-reloc-protected.d | 2 ++
> >> > ld/testsuite/ld-aarch64/protected.s | 8 ++++++
> >> > 4 files changed, 46 insertions(+), 1 deletion(-)
> >> > create mode 100644 ld/testsuite/ld-aarch64/copy-reloc-protected.d
> >> > create mode 100644 ld/testsuite/ld-aarch64/protected.s
> >> >
> >> > diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
> >> > index 4926bab9cf2..8896b0f78dd 100644
> >> > --- a/bfd/elfnn-aarch64.c
> >> > +++ b/bfd/elfnn-aarch64.c
> >> > @@ -2579,6 +2579,9 @@ struct elf_aarch64_link_hash_entry
> >> > this symbol. */
> >> > unsigned int got_type;
> >> >
> >> > + /* TRUE if symbol is defined as a protected symbol. */
> >> > + unsigned int def_protected : 1;
> >> > +
> >> > /* A pointer to the most recently used stub hash entry against this
> >> > symbol. */
> >> > struct elf_aarch64_stub_hash_entry *stub_cache;
> >> > @@ -2855,9 +2858,16 @@ elfNN_aarch64_copy_indirect_symbol (struct bfd_link_info *info,
> >> > static void
> >> > elfNN_aarch64_merge_symbol_attribute (struct elf_link_hash_entry *h,
> >> > unsigned int st_other,
> >> > - bool definition ATTRIBUTE_UNUSED,
> >> > + bool definition,
> >> > bool dynamic ATTRIBUTE_UNUSED)
> >> > {
> >> > + if (definition)
> >> > + {
> >> > + struct elf_aarch64_link_hash_entry *eh
> >> > + = (struct elf_aarch64_link_hash_entry *) h;
> >> > + eh->def_protected = ELF_ST_VISIBILITY (st_other) == STV_PROTECTED;
> >> > + }
> >> > +
> >> > unsigned int isym_sto = st_other & ~ELF_ST_VISIBILITY (-1);
> >> > unsigned int h_sto = h->other & ~ELF_ST_VISIBILITY (-1);
> >> >
> >> > @@ -8701,6 +8711,22 @@ elfNN_aarch64_allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
> >> > if (h->dyn_relocs == NULL)
> >> > return true;
> >> >
> >> > + for (p = h->dyn_relocs; p != NULL; p = p->next)
> >> > + if (eh->def_protected)
> >> > + {
> >> > + /* Disallow copy relocations against protected symbol. */
> >> > + asection *s = p->sec->output_section;
> >> > + if (s != NULL && (s->flags & SEC_READONLY) != 0)
> >> > + {
> >> > + info->callbacks->einfo
> >> > + /* xgettext:c-format */
> >> > + (_ ("%F%P: %pB: copy relocation against non-copyable "
> >> > + "protected symbol `%s'\n"),
> >> > + p->sec->owner, h->root.root.string);
> >> > + return false;
> >> > + }
> >> > + }
> >> > +
> >> > /* In the shared -Bsymbolic case, discard space allocated for
> >> > dynamic pc-relative relocs against symbols which turn out to be
> >> > defined in regular objects. For the normal shared case, discard
> >> > diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
> >> > index 64476f111e0..31162277bd9 100644
> >> > --- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
> >> > +++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
> >> > @@ -407,6 +407,8 @@ set aarch64elflinktests {
> >> > {copy-reloc-exe-2.s} {{objdump -R copy-reloc-2.d}} "copy-reloc-2"}
> >> > {"ld-aarch64/exe with copy relocation elimination" "-e0 tmpdir/copy-reloc-so.so" "" ""
> >> > {copy-reloc-exe-eliminate.s} {{objdump -R copy-reloc-eliminate.d}} "copy-reloc-elimination"}
> >> > + {"Build .so with protected data" "-shared" "" "" {protected.s}
> >> > + {} "protected.so"}
> >> > {"ld-aarch64/so with global func" "-shared" "" "" {func-in-so.s}
> >> > {} "func-in-so.so"}
> >> > {"ld-aarch64/func sym hash opt for exe"
> >> > @@ -416,8 +418,15 @@ set aarch64elflinktests {
> >> > {} "libbti-plt-so.so"}
> >> > }
> >> >
> >> > +set aarch64elfcclinktests [list \
> >> > + [list "copy relocation on protected data" \
> >> > + "-no-pie tmpdir/copy-reloc-exe.o tmpdir/protected.so" "" \
> >> > + {} {{error_output copy-reloc-protected.d}} "copy-reloc-protected"]
> >> > +]
> >> > +
> >> > if [check_shared_lib_support] {
> >> > run_ld_link_tests $aarch64elflinktests
> >> > + run_cc_link_tests $aarch64elfcclinktests
> >> > }
> >> >
> >> > run_dump_test "bti-plt-3"
> >> > diff --git a/ld/testsuite/ld-aarch64/copy-reloc-protected.d b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
> >> > new file mode 100644
> >> > index 00000000000..99a356a3df6
> >> > --- /dev/null
> >> > +++ b/ld/testsuite/ld-aarch64/copy-reloc-protected.d
> >> > @@ -0,0 +1,2 @@
> >> > +.*: tmpdir/copy-reloc-exe.o: copy relocation against non-copyable protected symbol `global_a'
> >> > +#...
> >> > diff --git a/ld/testsuite/ld-aarch64/protected.s b/ld/testsuite/ld-aarch64/protected.s
> >> > new file mode 100644
> >> > index 00000000000..eb3fb402dc4
> >> > --- /dev/null
> >> > +++ b/ld/testsuite/ld-aarch64/protected.s
> >> > @@ -0,0 +1,8 @@
> >> > +.global global_a
> >> > +.protected global_a
> >> > +.type global_a, %object
> >> > +.size global_a, 4
> >> > +
> >> > +.data
> >> > +global_a:
> >> > +.word 0xcafedead
> >> > --
> >> > 2.36
> >> >
>
> glibc now has a diagnostic for copy relocations on a protected symbol
> https://sourceware.org/git/?p=glibc.git;a=commit;h=7374c02b683b7110b853a32496a619410364d70b :)
> Let's catch the issue on ld side, too.
>
Ping^2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: PING: [PATCH] aarch64: Disallow copy relocations on protected data
[not found] ` <DS7PR12MB57658D646BB4944D4BECAF84CBB39@DS7PR12MB5765.namprd12.prod.outlook.com>
@ 2022-06-22 10:55 ` Nick Clifton
2022-06-22 11:15 ` Szabolcs Nagy
0 siblings, 1 reply; 8+ messages in thread
From: Nick Clifton @ 2022-06-22 10:55 UTC (permalink / raw)
To: Fangrui Song, binutils, Adhemerval Zanella, Szabolcs Nagy
Hi Fangrui,
>>>>> bfd/elfnn-aarch64.c | 28 ++++++++++++++++++-
>>>>> ld/testsuite/ld-aarch64/aarch64-elf.exp | 9 ++++++
>>>>> .../ld-aarch64/copy-reloc-protected.d | 2 ++
>>>>> ld/testsuite/ld-aarch64/protected.s | 8 ++++++
> Ping^2
Sorry - I thought that this patch had already been approved.
Patch approved - please apply.
Cheers
Nick
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: PING: [PATCH] aarch64: Disallow copy relocations on protected data
2022-06-22 10:55 ` Nick Clifton
@ 2022-06-22 11:15 ` Szabolcs Nagy
2022-07-01 11:16 ` Matthias Klose
0 siblings, 1 reply; 8+ messages in thread
From: Szabolcs Nagy @ 2022-06-22 11:15 UTC (permalink / raw)
To: Nick Clifton; +Cc: Fangrui Song, binutils, Adhemerval Zanella
The 06/22/2022 11:55, Nick Clifton wrote:
> Hi Fangrui,
>
> > > > > > bfd/elfnn-aarch64.c | 28 ++++++++++++++++++-
> > > > > > ld/testsuite/ld-aarch64/aarch64-elf.exp | 9 ++++++
> > > > > > .../ld-aarch64/copy-reloc-protected.d | 2 ++
> > > > > > ld/testsuite/ld-aarch64/protected.s | 8 ++++++
>
> > Ping^2
>
> Sorry - I thought that this patch had already been approved.
>
> Patch approved - please apply.
i was waiting for the glibc side to decide how to deal with
this case.
but now all related patches are committed to glibc so this
can go in.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: PING: [PATCH] aarch64: Disallow copy relocations on protected data
2022-06-22 11:15 ` Szabolcs Nagy
@ 2022-07-01 11:16 ` Matthias Klose
0 siblings, 0 replies; 8+ messages in thread
From: Matthias Klose @ 2022-07-01 11:16 UTC (permalink / raw)
To: Szabolcs Nagy, Nick Clifton; +Cc: binutils
On 22.06.22 13:15, Szabolcs Nagy via Binutils wrote:
> The 06/22/2022 11:55, Nick Clifton wrote:
>> Hi Fangrui,
>>
>>>>>>> bfd/elfnn-aarch64.c | 28 ++++++++++++++++++-
>>>>>>> ld/testsuite/ld-aarch64/aarch64-elf.exp | 9 ++++++
>>>>>>> .../ld-aarch64/copy-reloc-protected.d | 2 ++
>>>>>>> ld/testsuite/ld-aarch64/protected.s | 8 ++++++
>>
>>> Ping^2
>>
>> Sorry - I thought that this patch had already been approved.
>>
>> Patch approved - please apply.
>
> i was waiting for the glibc side to decide how to deal with
> this case.
>
> but now all related patches are committed to glibc so this
> can go in.
this causes https://sourceware.org/bugzilla/show_bug.cgi?id=29310
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-07-01 11:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-24 5:10 [PATCH] aarch64: Disallow copy relocations on protected data Fangrui Song
2022-05-27 22:43 ` Fangrui Song
[not found] ` <CAN30aBHDJM4KJAJJ_=4=vKgJL-yow2roUEUa7LFaAFaivkVswg@mail.gmail.com>
2022-06-01 5:53 ` PING: " Fangrui Song
[not found] ` <CAN30aBH68jT8ArEPEx_X-zVzuOmN3u_KV5HKc-Vgc2tVohJADw@mail.gmail.com>
2022-06-15 22:28 ` Fangrui Song
2022-06-21 19:27 ` Fangrui Song
[not found] ` <DS7PR12MB57658D646BB4944D4BECAF84CBB39@DS7PR12MB5765.namprd12.prod.outlook.com>
2022-06-22 10:55 ` Nick Clifton
2022-06-22 11:15 ` Szabolcs Nagy
2022-07-01 11:16 ` Matthias Klose
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).