9 Sep
2022
9 Sep
'22
6:30 p.m.
Hello,
this series does a couple cleanups in the static PIE code and xtensa
code and enables static PIE support for the xtensa architecture.
Max Filippov (5):
static pie: fix building static PDE
static pie: replicate PERFORM_BOOTSTRAP_GOT conditions from
dl-startup.c
xtensa: make GOT protection adjustment conditional
xtensa: drop ARCH_NEEDS_BOOTSTRAP_RELOCS
xtensa: add static pie support
extra/Configs/Config.in | 4 +++-
ldso/ldso/xtensa/dl-startup.h | 19 ++++++++++++---
ldso/ldso/xtensa/dl-sysdep.h | 3 ---
libc/misc/internals/reloc_static_pie.c | 9 +++++---
libc/sysdeps/linux/xtensa/crt1.S | 27 ++++++++++++++++++++++
libpthread/nptl/sysdeps/generic/libc-tls.c | 7 ++++--
6 files changed, 57 insertions(+), 12 deletions(-)
--
2.30.2
9 Sep
9 Sep
6:30 p.m.
New subject: [PATCH 1/5] static pie: fix building static PDE
When uclibc is built with static PIE support the _dl_load_base variable
shared between the libc-tls.c and reloc_static_pie.c creates the
dependency that requires linking reloc_static_pie.o into static
position-dependent executables resulting in the following build errors:
gcc -static test.c -o test
...ld:
...usr/lib/libc.a(reloc_static_pie.os):(.text+0x0):
undefined reference to `_DYNAMIC'
Move _dl_load_base definition to libc-tls.c to resolve this dependency
and fix static PDE build.
Signed-off-by: Max Filippov <jcmvbkbc(a)gmail.com>
---
libc/misc/internals/reloc_static_pie.c | 4 ++--
libpthread/nptl/sysdeps/generic/libc-tls.c | 7 +++++--
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/libc/misc/internals/reloc_static_pie.c
b/libc/misc/internals/reloc_static_pie.c
index c0027de6fe06..ce42cb9b30df 100644
--- a/libc/misc/internals/reloc_static_pie.c
+++ b/libc/misc/internals/reloc_static_pie.c
@@ -25,7 +25,7 @@
#include <dl-startup.h>
#endif
-ElfW(Addr) _dl_load_base = NULL;
+extern ElfW(Addr) _dl_load_base;
void
reloc_static_pie (ElfW(Addr) load_addr);
@@ -107,4 +107,4 @@ reloc_static_pie(ElfW(Addr) load_addr)
#endif
}
_dl_load_base = load_addr;
-}
\ No newline at end of file
+}
diff --git a/libpthread/nptl/sysdeps/generic/libc-tls.c
b/libpthread/nptl/sysdeps/generic/libc-tls.c
index 54f3cb0c72b5..7cfe9ac1a85c 100644
--- a/libpthread/nptl/sysdeps/generic/libc-tls.c
+++ b/libpthread/nptl/sysdeps/generic/libc-tls.c
@@ -117,6 +117,10 @@ init_static_tls (size_t memsz, size_t align)
GL(dl_tls_static_nelem) = GL(dl_tls_max_dtv_idx);
}
+#if !defined(__FDPIC__) && !defined(SHARED) && defined(STATIC_PIE)
+ElfW(Addr) _dl_load_base;
+#endif
+
void __libc_setup_tls (size_t tcbsize, size_t tcbalign);
void
__libc_setup_tls (size_t tcbsize, size_t tcbalign)
@@ -143,8 +147,7 @@ __libc_setup_tls (size_t tcbsize, size_t tcbalign)
#else
initimage = (void *) phdr->p_vaddr;
#if !defined(SHARED) && defined(STATIC_PIE)
- extern ElfW(Addr) _dl_load_base;
- initimage += _dl_load_base;
+ initimage += _dl_load_base;
#endif
#endif
align = phdr->p_align;
--
2.30.2
6:30 p.m.
New subject: [PATCH 2/5] static pie: replicate PERFORM_BOOTSTRAP_GOT conditions from dl-startup.c
Code in reloc_static_pie.c is largely based on DL_START function from
dl-startup.c, but it's missing the condition that disables the rest of
relocation when PERFORM_BOOTSTRAP_GOT macro is defined. As a result when
building reloc_static_pie.c for xtensa it applies relative relocations
twice.
Replicate PERFORM_BOOTSTRAP_GOT conditions from dl-startup.c in
reloc_static_pie.c to allow using PERFORM_BOOTSTRAP_GOT macro as is.
Signed-off-by: Max Filippov <jcmvbkbc(a)gmail.com>
---
libc/misc/internals/reloc_static_pie.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libc/misc/internals/reloc_static_pie.c
b/libc/misc/internals/reloc_static_pie.c
index ce42cb9b30df..9f5366cc6ab7 100644
--- a/libc/misc/internals/reloc_static_pie.c
+++ b/libc/misc/internals/reloc_static_pie.c
@@ -54,6 +54,8 @@ reloc_static_pie(ElfW(Addr) load_addr)
#endif
+#if !defined(PERFORM_BOOTSTRAP_GOT) || defined(__avr32__) || defined(__mips__)
+
#if defined(ELF_MACHINE_PLTREL_OVERLAP)
# define INDX_MAX 1
#else
@@ -106,5 +108,6 @@ reloc_static_pie(ElfW(Addr) load_addr)
}
#endif
}
+#endif
_dl_load_base = load_addr;
}
--
2.30.2
6:30 p.m.
New subject: [PATCH 3/5] xtensa: make GOT protection adjustment conditional
Xtensa PERFORM_BOOTSTRAP_GOT macro uses mprotect to make bits of GOT
writable, but noMMU linux kernel returns ENOSYS to mprotect syscalls,
and syscall wrapper tries to update errno with the error code. This
happens well before the relocations are done and results in writes to
unrelated locations, memory corruption or protection violations.
Split GOT protection update from PERFORM_BOOTSTRAP_GOT and only do it
when building configuration with MMU support.
Signed-off-by: Max Filippov <jcmvbkbc(a)gmail.com>
---
ldso/ldso/xtensa/dl-startup.h | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/ldso/ldso/xtensa/dl-startup.h b/ldso/ldso/xtensa/dl-startup.h
index db223feadc90..19955ffc77a5 100644
--- a/ldso/ldso/xtensa/dl-startup.h
+++ b/ldso/ldso/xtensa/dl-startup.h
@@ -88,12 +88,11 @@ __asm__ (
/* Function calls are not safe until the GOT relocations have been done. */
#define NO_FUNCS_BEFORE_BOOTSTRAP
-#define PERFORM_BOOTSTRAP_GOT(tpnt) \
+#if defined(__ARCH_USE_MMU__)
+#define PERFORM_BOOTSTRAP_GOT_ADJUST_PROTECTION(tpnt) \
do { \
xtensa_got_location *got_loc; \
unsigned long l_addr = tpnt->loadaddr; \
- Elf32_Word relative_count; \
- unsigned long rel_addr; \
Elf32_Addr prev_got_start = 0, prev_got_end = 0; \
int x; \
\
@@ -125,7 +124,19 @@ do { \
prev_got_end - prev_got_start, \
PROT_READ | PROT_WRITE | PROT_EXEC); \
} \
+} while (0)
+#else
+#define PERFORM_BOOTSTRAP_GOT_ADJUST_PROTECTION(tpnt) \
+do { \
+} while (0)
+#endif
+
+#define PERFORM_BOOTSTRAP_GOT(tpnt) \
+do { \
+ Elf32_Word relative_count; \
+ unsigned long rel_addr; \
\
+ PERFORM_BOOTSTRAP_GOT_ADJUST_PROTECTION(tpnt); \
/* The following is a stripped down version of the code following \
the invocation of PERFORM_BOOTSTRAP_GOT in dl-startup.c. That \
code is skipped when PERFORM_BOOTSTRAP_GOT is defined, so it has \
--
2.30.2
6:30 p.m.
New subject: [PATCH 4/5] xtensa: drop ARCH_NEEDS_BOOTSTRAP_RELOCS
Xtensa does not define PERFORM_BOOTSTRAP_RELOC so it doesn't need
ARCH_NEEDS_BOOTSTRAP_RELOCS definition. Remove it.
Signed-off-by: Max Filippov <jcmvbkbc(a)gmail.com>
---
ldso/ldso/xtensa/dl-sysdep.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/ldso/ldso/xtensa/dl-sysdep.h b/ldso/ldso/xtensa/dl-sysdep.h
index d308237d39ed..6b908989a8f1 100644
--- a/ldso/ldso/xtensa/dl-sysdep.h
+++ b/ldso/ldso/xtensa/dl-sysdep.h
@@ -94,9 +94,6 @@ typedef struct xtensa_got_location_struct {
/* Used for error messages. */
#define ELF_TARGET "Xtensa"
-/* Need bootstrap relocations */
-#define ARCH_NEEDS_BOOTSTRAP_RELOCS
-
struct elf_resolve;
extern unsigned long _dl_linux_resolver (struct elf_resolve *, int);
--
2.30.2
6:30 p.m.
New subject: [PATCH 5/5] xtensa: add static pie support
Signed-off-by: Max Filippov <jcmvbkbc(a)gmail.com>
---
extra/Configs/Config.in | 4 +++-
ldso/ldso/xtensa/dl-startup.h | 2 ++
libc/misc/internals/reloc_static_pie.c | 2 +-
libc/sysdeps/linux/xtensa/crt1.S | 27 ++++++++++++++++++++++++++
4 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/extra/Configs/Config.in b/extra/Configs/Config.in
index 43c04fd0a271..dd1beaadcee3 100644
--- a/extra/Configs/Config.in
+++ b/extra/Configs/Config.in
@@ -324,7 +324,9 @@ config DOPIC
config STATIC_PIE
bool "Add support for Static Position Independent Executables (PIE)"
default n
- depends on DOPIC && !UCLIBC_FORMAT_FDPIC_ELF && (TARGET_arm ||
TARGET_i386 || TARGET_x86_64 || TARGET_aarch64 || TARGET_mips)
+ depends on DOPIC && !UCLIBC_FORMAT_FDPIC_ELF && \
+ (TARGET_arm || TARGET_i386 || TARGET_x86_64 || TARGET_aarch64 || \
+ TARGET_mips || TARGET_xtensa)
config ARCH_HAS_NO_SHARED
bool
diff --git a/ldso/ldso/xtensa/dl-startup.h b/ldso/ldso/xtensa/dl-startup.h
index 19955ffc77a5..1babaccea7e8 100644
--- a/ldso/ldso/xtensa/dl-startup.h
+++ b/ldso/ldso/xtensa/dl-startup.h
@@ -7,6 +7,7 @@
* Parts taken from glibc/sysdeps/xtensa/dl-machine.h.
*/
+#ifndef L_rcrt1
__asm__ (
" .text\n"
" .align 4\n"
@@ -81,6 +82,7 @@ __asm__ (
" addi a5, a5, 8\n"
" bnez a6, 3b\n"
" j .Lfixup_stack_ret");
+#endif
/* Get a pointer to the argv value. */
#define GET_ARGV(ARGVP, ARGS) ARGVP = (((unsigned long *) ARGS) + 1)
diff --git a/libc/misc/internals/reloc_static_pie.c
b/libc/misc/internals/reloc_static_pie.c
index 9f5366cc6ab7..81b235a51cb3 100644
--- a/libc/misc/internals/reloc_static_pie.c
+++ b/libc/misc/internals/reloc_static_pie.c
@@ -21,7 +21,7 @@
#include <dl-elf.h>
#include <ldso.h>
-#ifdef __mips__
+#if defined(__mips__) || defined(__xtensa__)
#include <dl-startup.h>
#endif
diff --git a/libc/sysdeps/linux/xtensa/crt1.S b/libc/sysdeps/linux/xtensa/crt1.S
index efbe264c03f0..3fa14ae583a9 100644
--- a/libc/sysdeps/linux/xtensa/crt1.S
+++ b/libc/sysdeps/linux/xtensa/crt1.S
@@ -76,9 +76,26 @@
.global _start
.type _start, @function
_start:
+#ifdef L_rcrt1
+ .begin no-transform
+ call0 1f
+.Lret_addr:
+ .end no-transform
+ .align 4
+1:
+#endif
#if defined(__XTENSA_WINDOWED_ABI__)
+#ifdef L_rcrt1
+ movi a6, .Lret_addr
+ sub a6, a0, a6
+ movi a0, 0
+ movi a4, reloc_static_pie
+ add a4, a4, a6
+ callx4 a4
+#else
/* Clear a0 to obviously mark the outermost frame. */
movi a0, 0
+#endif
/* Load up the user's main function. */
movi a6, main
@@ -106,8 +123,18 @@ _start:
movi a4, __uClibc_main
callx4 a4
#elif defined(__XTENSA_CALL0_ABI__)
+#ifdef L_rcrt1
+ mov a12, a2
+ movi a2, .Lret_addr
+ sub a2, a0, a2
+ movi a0, reloc_static_pie
+ add a0, a0, a2
+ callx0 a0
+ mov a7, a12
+#else
/* Setup the shared library termination function. */
mov a7, a2
+#endif
/* Load up the user's main function. */
movi a2, main
--
2.30.2
14 Sep
14 Sep
7:23 p.m.
New subject: [PATCH 5/5] xtensa: add static pie support
There is a lot to unpack here, so please forgive me while I try to format
this in a readable and concise way.
[Patch 1/5]
I thought this was already submitted and merged in? It looks identical to
"[PATCH v2] static pie: building static PDE", or am I mistaken?
[Patch 2/5]
I'm going to be honest with you, I left that check out because it only
affects xtensia, so it's funny to me that it would bite me this quickly.
That said, reading the code, I'm not sure why xtensia decided to call
elf_machine_relative in PERFORM_BOOTSTRAP_GOT and not use the default code
path with ELF_MACHINE_PLTREL_OVERLAP set to ensure only 1 iteration. I am
not very familiar with xtensia, so there may be some nuance that I'm not
aware of, but I would like your opinion on if we should change xtensia to
conform to how every other architecture seems to perform that relocation.
If we are going to keep the code the way it is, I would like to know
everyone's opinion on whether or not we should change that preprocessor
directive to be explicit on which architectures shouldn't do that section
of code? Currently it's in an allow list format with seemingly only xtensia
having a false condition, but that isn't very clear to me from an initial
read. Obviously that change would belong to a separate patch and shouldn't
hold this up.
[Patch 3/5]
looks good to me.
[Patch 4/5]
I think this removal further implies that we should move the
elf_machine_relative call out of PERFORM_BOOTSTRAP_GOT as I suggested above.
[Patch 5/5]
I am getting segfaults while still in __uClibc_main when using buildroot's
qemu_xtensa_lx60_defconfig and qemu-xtensa. I'm not sure the exact cause,
but I would guess that some functions aren't being relocated correctly or
the TLS wasn't updated correctly. It could also possibly be a problem with
the quick gcc patch I wrote to compile with -static-pie. If you have a gcc
patch that you think will fix this as well, I would be glad to test it.
Here is the stack trace up to where it calls 0x00:
#0 0x3efe58e9 in __h_errno_location () at libpthread/nptl/herrno.c:33
#1 0x3efe12aa in __uClibc_main (main=0x3efddfbc <main>, argc=0x1,
argv=0x3efdb314, app_init=0x3efdde14 <_init>, app_fini=0x3eff5864 <_fini>,
rtld_fini=0x0, stack_end=0x3efdb310) at
libc/misc/internals/__uClibc_main.c:514
#2 0x3efdde6c in _start ()
On Fri, Sep 9, 2022 at 2:30 PM Max Filippov <jcmvbkbc(a)gmail.com> wrote:
Signed-off-by: Max Filippov
<jcmvbkbc(a)gmail.com>
---
extra/Configs/Config.in | 4 +++-
ldso/ldso/xtensa/dl-startup.h | 2 ++
libc/misc/internals/reloc_static_pie.c | 2 +-
libc/sysdeps/linux/xtensa/crt1.S | 27 ++++++++++++++++++++++++++
4 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/extra/Configs/Config.in b/extra/Configs/Config.in
index 43c04fd0a271..dd1beaadcee3 100644
--- a/extra/Configs/Config.in
+++ b/extra/Configs/Config.in
@@ -324,7 +324,9 @@ config DOPIC
config STATIC_PIE
bool "Add support for Static Position Independent Executables
(PIE)"
default n
- depends on DOPIC && !UCLIBC_FORMAT_FDPIC_ELF && (TARGET_arm ||
TARGET_i386 || TARGET_x86_64 || TARGET_aarch64 || TARGET_mips)
+ depends on DOPIC && !UCLIBC_FORMAT_FDPIC_ELF && \
+ (TARGET_arm || TARGET_i386 || TARGET_x86_64 ||
TARGET_aarch64 || \
+ TARGET_mips || TARGET_xtensa)
config ARCH_HAS_NO_SHARED
bool
diff --git a/ldso/ldso/xtensa/dl-startup.h b/ldso/ldso/xtensa/dl-startup.h
index 19955ffc77a5..1babaccea7e8 100644
--- a/ldso/ldso/xtensa/dl-startup.h
+++ b/ldso/ldso/xtensa/dl-startup.h
@@ -7,6 +7,7 @@
* Parts taken from glibc/sysdeps/xtensa/dl-machine.h.
*/
+#ifndef L_rcrt1
__asm__ (
" .text\n"
" .align 4\n"
@@ -81,6 +82,7 @@ __asm__ (
" addi a5, a5, 8\n"
" bnez a6, 3b\n"
" j .Lfixup_stack_ret");
+#endif
/* Get a pointer to the argv value. */
#define GET_ARGV(ARGVP, ARGS) ARGVP = (((unsigned long *) ARGS) + 1)
diff --git a/libc/misc/internals/reloc_static_pie.c
b/libc/misc/internals/reloc_static_pie.c
index 9f5366cc6ab7..81b235a51cb3 100644
--- a/libc/misc/internals/reloc_static_pie.c
+++ b/libc/misc/internals/reloc_static_pie.c
@@ -21,7 +21,7 @@
#include <dl-elf.h>
#include <ldso.h>
-#ifdef __mips__
+#if defined(__mips__) || defined(__xtensa__)
#include <dl-startup.h>
#endif
diff --git a/libc/sysdeps/linux/xtensa/crt1.S
b/libc/sysdeps/linux/xtensa/crt1.S
index efbe264c03f0..3fa14ae583a9 100644
--- a/libc/sysdeps/linux/xtensa/crt1.S
+++ b/libc/sysdeps/linux/xtensa/crt1.S
@@ -76,9 +76,26 @@
.global _start
.type _start, @function
_start:
+#ifdef L_rcrt1
+ .begin no-transform
+ call0 1f
+.Lret_addr:
+ .end no-transform
+ .align 4
+1:
+#endif
#if defined(__XTENSA_WINDOWED_ABI__)
+#ifdef L_rcrt1
+ movi a6, .Lret_addr
+ sub a6, a0, a6
+ movi a0, 0
+ movi a4, reloc_static_pie
+ add a4, a4, a6
+ callx4 a4
+#else
/* Clear a0 to obviously mark the outermost frame. */
movi a0, 0
+#endif
/* Load up the user's main function. */
movi a6, main
@@ -106,8 +123,18 @@ _start:
movi a4, __uClibc_main
callx4 a4
#elif defined(__XTENSA_CALL0_ABI__)
+#ifdef L_rcrt1
+ mov a12, a2
+ movi a2, .Lret_addr
+ sub a2, a0, a2
+ movi a0, reloc_static_pie
+ add a0, a0, a2
+ callx0 a0
+ mov a7, a12
+#else
/* Setup the shared library termination function. */
mov a7, a2
+#endif
/* Load up the user's main function. */
movi a2, main
--
2.30.2
7:48 p.m.
New subject: [PATCH 5/5] xtensa: add static pie support
Hi linted,
thank you for taking a look.
On Wed, Sep 14, 2022 at 12:23 PM linted <linted90(a)gmail.com> wrote:
[Patch 1/5]
I thought this was already submitted and merged in? It looks identical to
"[PATCH v2] static pie: building static PDE", or am I mistaken?
Yes, it's the exact same patch. I'm looking at this uClibc-ng repository:
https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/
and it's not there, so I included it for completeness.
[Patch 2/5]
I'm going to be honest with you, I left that check out because it only affects
xtensia, so it's funny to me that it would bite me this quickly. That said,
reading the code, I'm not sure why xtensia decided to call elf_machine_relative
in PERFORM_BOOTSTRAP_GOT and not use the default code path with
ELF_MACHINE_PLTREL_OVERLAP set to ensure only 1 iteration.
I am not very familiar with xtensia, so there may be some nuance that I'm
not aware of, but I would like your opinion on if we should change xtensia
to conform to how every other architecture seems to perform that relocation.
Yes, I stumbled here too and the same thought crossed my mind. I guess
I should do another pass, try to understand why it's done that way and
clean it up.
[Patch 3/5]
looks good to me.
[Patch 4/5]
I think this removal further implies that we should move the
elf_machine_relative call out of PERFORM_BOOTSTRAP_GOT
as I suggested above.
[Patch 5/5]
I am getting segfaults while still in __uClibc_main when using
buildroot's qemu_xtensa_lx60_defconfig and qemu-xtensa.
I'm not sure the exact cause, but I would guess that some functions
aren't being relocated correctly or the TLS wasn't updated correctly.
It could also possibly be a problem with the quick gcc patch I wrote to
compile with -static-pie. If you have a gcc patch that you think will fix
this as well, I would be glad to test it.
You're right, the following recent binutils and gcc changes are
required for generation of correct relocations and recognition of
the -static-pie option:
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=658ba81aef5e8…
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9e0c2696724d4d004ea189a69…
--
Thanks.
-- Max
8:15 p.m.
New subject: [PATCH 5/5] xtensa: add static pie support
On 9/14/2022 1:23 PM, linted wrote:
[Patch 5/5]
I am getting segfaults while still in __uClibc_main when using
buildroot's qemu_xtensa_lx60_defconfig and qemu-xtensa. I'm not sure
the exact cause, but I would guess that some functions aren't being
relocated correctly or the TLS wasn't updated correctly. It could also
possibly be a problem with the quick gcc patch I wrote to compile with
-static-pie. If you have a gcc patch that you think will fix this as
well, I would be glad to test it.
I've had -static-pie binaries segfault with
qemu while they ran fine on
actual hardware. I was using Debian 10 so maybe that's because it's an
old version of qemu, and maybe newer versions can handle this better?
Lance
15 Sep
15 Sep
5:43 p.m.
New subject: [PATCH 5/5] xtensa: add static pie support
> [Patch 5/5]
> I am getting segfaults while still in __uClibc_main when using
> buildroot's qemu_xtensa_lx60_defconfig and qemu-xtensa. I'm not sure
> the exact cause, but I would guess that some functions aren't being
> relocated correctly or the TLS wasn't updated correctly. It could also
> possibly be a problem with the quick gcc patch I wrote to compile with
> -static-pie. If you have a gcc patch that you think will fix this as
> well, I would be glad to test it.
You're right, the following recent binutils and
gcc changes are
required for generation of correct relocations and recognition of
the -static-pie option:
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=658ba81aef5e8…
<https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=658ba81aef5e85a08d67eb211a43c6db775a36b3
598
days inactive
604
days old
9 comments
3 participants
participants (3)
-
Lance Fredrickson -
linted -
Max Filippov