* [PATCH] riscv: remove DL_RO_DYN_SECTION
@ 2019-04-09 11:42 David Abdurachmanov
2019-04-09 12:07 ` Florian Weimer
2019-04-25 19:42 ` Palmer Dabbelt
0 siblings, 2 replies; 5+ messages in thread
From: David Abdurachmanov @ 2019-04-09 11:42 UTC (permalink / raw)
To: libc-alpha; +Cc: David Abdurachmanov, jimw, palmer, dj
While working on enabling D front-end (GDC) in GCC we noticed that druntime
was segfaulting if it is linked dynamically. This was tracked to
DL_RO_DYN_SECTION.
DL_RO_DYN_SECTION lines seem to be copied from MIPS file (which is the only
user of it), but the comment doesn't apply to RISC-V. There is no such
requirement in RISC-V ABI.
* sysdeps/riscv/ldsodefs.h: Remove DL_RO_DYN_SECTION as it is not
required by RISC-V ABI.
Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com>
Cc: jimw@sifive.com
Cc: palmer@sifive.com
Cc: dj@delorie.com
---
sysdeps/riscv/ldsodefs.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/sysdeps/riscv/ldsodefs.h b/sysdeps/riscv/ldsodefs.h
index f46cf4158e..5ec607e867 100644
--- a/sysdeps/riscv/ldsodefs.h
+++ b/sysdeps/riscv/ldsodefs.h
@@ -38,10 +38,6 @@ struct La_riscv_retval;
struct La_riscv_retval *, \
const char *);
-/* The RISC-V ABI specifies that the dynamic section has to be read-only. */
-
-#define DL_RO_DYN_SECTION 1
-
#include_next <ldsodefs.h>
#endif
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv: remove DL_RO_DYN_SECTION
2019-04-09 11:42 [PATCH] riscv: remove DL_RO_DYN_SECTION David Abdurachmanov
@ 2019-04-09 12:07 ` Florian Weimer
2019-04-10 0:51 ` Jim Wilson
2019-04-25 19:42 ` Palmer Dabbelt
1 sibling, 1 reply; 5+ messages in thread
From: Florian Weimer @ 2019-04-09 12:07 UTC (permalink / raw)
To: David Abdurachmanov; +Cc: libc-alpha, jimw, palmer, dj
* David Abdurachmanov:
> While working on enabling D front-end (GDC) in GCC we noticed that druntime
> was segfaulting if it is linked dynamically. This was tracked to
> DL_RO_DYN_SECTION.
Is it possible to extract a test case from that which we could put into
glibc?
Since this appears to be a user-visible bug, it needs a bug in
Bugzilla. Would you please file one?
Thanks,
Florian
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv: remove DL_RO_DYN_SECTION
2019-04-09 12:07 ` Florian Weimer
@ 2019-04-10 0:51 ` Jim Wilson
2019-05-29 22:03 ` Maciej Rozycki
0 siblings, 1 reply; 5+ messages in thread
From: Jim Wilson @ 2019-04-10 0:51 UTC (permalink / raw)
To: Florian Weimer
Cc: David Abdurachmanov, GNU C Library, Jim Wilson, Palmer Dabbelt,
DJ Delorie
On Tue, Apr 9, 2019 at 4:42 AM Florian Weimer <fweimer@redhat.com> wrote:
> * David Abdurachmanov:
Do you or your employer have copyright assignments? For a trivial
patch like this, a copyright assignment shouldn't be necessary, but
would be necessary if you want to contribute more patches.
> > While working on enabling D front-end (GDC) in GCC we noticed that druntime
> > was segfaulting if it is linked dynamically. This was tracked to
> > DL_RO_DYN_SECTION.
My analysis of this when it first came up...
I believe this was blindly copied from MIPS, and that no one on the
RISC-V side will know why it is there, because it was blindly copied
from MIPS.
Looking at the MIPS SVR4 ABI, I see
.dynamic This is the same as the generic ABI section of the same type, but
the MIPS-specific version does not include the SHF_WRITE at-
tribute.
I don't see any other reason in the spec for this to be read-only, but
making it read-only probably improves program security. I don't see
an equivalent statement in the RISC-V ABI.
Looking at bfd/elfxx-mips.c _bfd_mips_elf_create_dynamic_sections(), we see code
flags = (SEC_ALLOC | SEC_LOAD | SEC_HAS_CONTENTS | SEC_IN_MEMORY
| SEC_LINKER_CREATED | SEC_READONLY);
/* The psABI requires a read-only .dynamic section, but the VxWorks
EABI doesn't. */
if (!htab->is_vxworks)
{
s = bfd_get_linker_section (abfd, ".dynamic");
if (s != NULL)
{
if (! bfd_set_section_flags (abfd, s, flags))
return FALSE;
}
}
that forces .dynamic to be read-only everywhere except vxworks. I
don't see equivalent code in the RISC-V bfd port.
Looking at ld, in emulparams/, there are multiple mips scripts that
set TEXT_DYNAMIC, which forces .dynamic into the text segment instead
of the data segment. This is then unset in the vxworks scripts.
Interestingly, hppa also sets TEXT_DYNAMIC, but the hppa support in
glibc is probably not well maintained. Anyways, there is no
equivalent code for RISC-V.
So it appears unnecessary for the RISC-V glibc port.
If MIPS dynamic linking support is ever added to the druntime library,
then it will need the same change that you made for RISC-V, so in that
case it would not be a RISC-V specific change. However, there aren't
a lot of people doing MIPS development work anymore, so this may never
come up.
Jim
PS Since I wrote my analysis, MIPS has been resurrected, so maybe
someone will care about that again.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv: remove DL_RO_DYN_SECTION
2019-04-09 11:42 [PATCH] riscv: remove DL_RO_DYN_SECTION David Abdurachmanov
2019-04-09 12:07 ` Florian Weimer
@ 2019-04-25 19:42 ` Palmer Dabbelt
1 sibling, 0 replies; 5+ messages in thread
From: Palmer Dabbelt @ 2019-04-25 19:42 UTC (permalink / raw)
To: david.abdurachmanov; +Cc: libc-alpha, david.abdurachmanov, Jim Wilson, dj
On Tue, 09 Apr 2019 04:25:29 PDT (-0700), david.abdurachmanov@gmail.com wrote:
> While working on enabling D front-end (GDC) in GCC we noticed that druntime
> was segfaulting if it is linked dynamically. This was tracked to
> DL_RO_DYN_SECTION.
>
> DL_RO_DYN_SECTION lines seem to be copied from MIPS file (which is the only
> user of it), but the comment doesn't apply to RISC-V. There is no such
> requirement in RISC-V ABI.
>
> * sysdeps/riscv/ldsodefs.h: Remove DL_RO_DYN_SECTION as it is not
> required by RISC-V ABI.
>
> Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com>
> Cc: jimw@sifive.com
> Cc: palmer@sifive.com
> Cc: dj@delorie.com
> ---
> sysdeps/riscv/ldsodefs.h | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/sysdeps/riscv/ldsodefs.h b/sysdeps/riscv/ldsodefs.h
> index f46cf4158e..5ec607e867 100644
> --- a/sysdeps/riscv/ldsodefs.h
> +++ b/sysdeps/riscv/ldsodefs.h
> @@ -38,10 +38,6 @@ struct La_riscv_retval;
> struct La_riscv_retval *, \
> const char *);
>
> -/* The RISC-V ABI specifies that the dynamic section has to be read-only. */
> -
> -#define DL_RO_DYN_SECTION 1
> -
> #include_next <ldsodefs.h>
>
> #endif
Thanks. I went ahead and pushed it without a test case so we don't lose the
fix. I also opened a Bugzilla:
https://sourceware.org/bugzilla/show_bug.cgi?id=24484
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv: remove DL_RO_DYN_SECTION
2019-04-10 0:51 ` Jim Wilson
@ 2019-05-29 22:03 ` Maciej Rozycki
0 siblings, 0 replies; 5+ messages in thread
From: Maciej Rozycki @ 2019-05-29 22:03 UTC (permalink / raw)
To: Jim Wilson
Cc: Florian Weimer, David Abdurachmanov, GNU C Library,
Palmer Dabbelt, DJ Delorie
On Tue, 9 Apr 2019, Jim Wilson wrote:
> I believe this was blindly copied from MIPS, and that no one on the
> RISC-V side will know why it is there, because it was blindly copied
> from MIPS.
>
> Looking at the MIPS SVR4 ABI, I see
> .dynamic This is the same as the generic ABI section of the same type, but
> the MIPS-specific version does not include the SHF_WRITE at-
> tribute.
>
> I don't see any other reason in the spec for this to be read-only, but
> making it read-only probably improves program security. I don't see
> an equivalent statement in the RISC-V ABI.
For the record the MIPS port can get away with a read-only dynamic
segment (which is indeed somewhat advantageous security-wise), because it
does not use the DT_DEBUG entry which requires an update at runtime and
relies on DT_MIPS_RLD_MAP and (more recently for PIE) DT_MIPS_RLD_MAP_REL
instead. For DSOs DT_DEBUG is not used and therefore the dynamic segment
can be read-only regardless I believe.
Maybe we can arrange for a read-only dynamic segment in the RISC-V ABI
after all? At the time DT_MIPS_RLD_MAP_REL was being crafted I advocated
for a generic tag[1], but the idea was rejected. We can make our own
processor-specific tag, such as DT_RISCV_RLD_MAP_REL, or reconsider my
idea of DT_GNU_RLD_MAP (or DT_GNU_RLD_MAP_REL).
References:
[1] "Support shared library debug with MIPS PIE",
<https://sourceware.org/ml/binutils/2015-06/msg00227.html>
HTH,
Maciej
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-29 22:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-09 11:42 [PATCH] riscv: remove DL_RO_DYN_SECTION David Abdurachmanov
2019-04-09 12:07 ` Florian Weimer
2019-04-10 0:51 ` Jim Wilson
2019-05-29 22:03 ` Maciej Rozycki
2019-04-25 19:42 ` Palmer Dabbelt
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).