I did a little bit of research, and you are completely right, I should have been using DYNAMIC_SIZE rather than the static 36. I'm not entirely sure what to do about the format of the comments. It seems to be a tabs vs spaces issue. Both files swap between tabs and spaces in different places throughout the file. I think that if I swap over to spaces it might make it look a little cleaner though. I'll resubmit with the other changes you suggested.
On Sat, Jul 16, 2022 at 5:27 AM Yann Sionneau <[email protected]> wrote: > Hello, > > Looks great :) > > Comments inline: > > Le 15/07/2022 à 22:26, linted a écrit : > > Sujet : > > [uclibc-ng-devel] [Patch v2] Static PIE support for i386, x86_64, and arm > > De : > > linted <[email protected]> > > Date : > > 15/07/2022, 22:26 > > > > Pour : > > [email protected] > > > > > > Hello, > > This is an updated patch for static pie support on i386, x86_64, and > > arm. I added dependency checks to ensure it only affects supported > > architectures. I also added a check to ensure an MMU is present on arm > > since there is no support for FDPIC yet. > > > > 0001-Added-static-pie-support-for-i368-x86_64-and-arm.patch > > > > From 98313d17220486a75a0e552df045c6752a5ad25b Mon Sep 17 00:00:00 2001 > > From: linted<[email protected]> > > Date: Fri, 15 Jul 2022 16:15:14 -0400 > > Subject: [PATCH] Added static pie support for i368, x86_64 and arm > Maybe add some more description of how this is done in the commit message? > > > > Signed-off-by: linted<[email protected]> > > --- > > Makerules | 5 ++++ > > extra/Configs/Config.in | 7 ++++- > > libc/misc/internals/Makefile.in | 1 + > > libc/misc/internals/reloc_static_pie.c | 39 ++++++++++++++++++++++++++ > > libc/sysdeps/linux/arm/crt1.S | 15 ++++++++++ > > libc/sysdeps/linux/i386/crt1.S | 20 +++++++++++++ > > libc/sysdeps/linux/x86_64/crt1.S | 16 ++++++++++- > > 7 files changed, 101 insertions(+), 2 deletions(-) > > create mode 100644 libc/misc/internals/reloc_static_pie.c > > > > diff --git a/Makerules b/Makerules > > index fd40e6c7b..845d81897 100644 > > --- a/Makerules > > +++ b/Makerules > > @@ -405,8 +405,13 @@ else > > CRTS=$(top_builddir)lib/$(CRT).o > > endif > > > > +ifeq ($(STATIC_PIE),y) > > +CRTS+=$(top_builddir)lib/r$(CRT).o > > +endif > > + > > ASFLAGS-$(CRT).o := -DL_$(CRT) > > ASFLAGS-S$(CRT).o := $(PIEFLAG) -DL_S$(CRT) > > +ASFLAGS-r$(CRT).o := $(PIEFLAG) -DL_r$(CRT) > > $(CRTS): $(top_srcdir)libc/sysdeps/linux/$(TARGET_ARCH)/$(CRT).S > > $(compile.S) > > $(Q)$(STRIPTOOL) -x -R .note -R .comment $@ > > diff --git a/extra/Configs/Config.in b/extra/Configs/Config.in > > index a58ceb265..ac5c494c3 100644 > > --- a/extra/Configs/Config.in > > +++ b/extra/Configs/Config.in > > @@ -301,6 +301,11 @@ config DOPIC > > If you wish to build all of uClibc as PIC objects, then answer Y > here. > > If you are unsure, then you should answer N. > > > > +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) > > + > > config ARCH_HAS_NO_SHARED > > bool > > > > @@ -1757,7 +1762,7 @@ config UCLIBC_HAS_STDIO_AUTO_RW_TRANSITION > > fflush function or to a file positioning function (fseek, > fsetpos, > > or rewind), and input shall not be directly followed by output > without > > an intervening call to a file positioning function, unless the > input > > - operation encounters end�of�file. > > + operation encounters end�of�file. > This seems like a spurious modification due to the text editor? > > > > Most people will answer Y. > > > > diff --git a/libc/misc/internals/Makefile.in > b/libc/misc/internals/Makefile.in > > index a8e4e36f9..4a6e73d2d 100644 > > --- a/libc/misc/internals/Makefile.in > > +++ b/libc/misc/internals/Makefile.in > > @@ -34,6 +34,7 @@ libc-static-$(UCLIBC_FORMAT_FLAT_SEP_DATA) += \ > > libc-static-$(UCLIBC_FORMAT_SHARED_FLAT) += \ > > $(MISC_INTERNALS_OUT)/shared_flat_initfini.o \ > > $(MISC_INTERNALS_OUT)/shared_flat_add_library.o > > +libc-static-$(STATIC_PIE) += $(MISC_INTERNALS_OUT)/reloc_static_pie.o > > libc-shared-$(UCLIBC_FORMAT_SHARED_FLAT) += \ > > $(MISC_INTERNALS_OUT)/shared_flat_initfini.os \ > > $(MISC_INTERNALS_OUT)/shared_flat_add_library.os > > diff --git a/libc/misc/internals/reloc_static_pie.c > b/libc/misc/internals/reloc_static_pie.c > > new file mode 100644 > > index 000000000..0736bc8a1 > > --- /dev/null > > +++ b/libc/misc/internals/reloc_static_pie.c > > @@ -0,0 +1,39 @@ > > +/* Support for relocating static PIE. > > + Copyright (C) 2017-2022 Free Software Foundation, Inc. > > + This file is part of the GNU C Library. > > + > > + The GNU C Library is free software; you can redistribute it and/or > > + modify it under the terms of the GNU Lesser General Public > > + License as published by the Free Software Foundation; either > > + version 2.1 of the License, or (at your option) any later version. > > + > > + The GNU C Library is distributed in the hope that it will be useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + Lesser General Public License for more details. > > + > > + You should have received a copy of the GNU Lesser General Public > > + License along with the GNU C Library; if not, see > > +<https://www.gnu.org/licenses/>. */ > > + > > +#include <link.h> > > +#include <elf.h> > > +#include <dl-elf.h> > > + > > +void > > +reloc_static_pie (ElfW(Addr) load_addr) > > +{ > > + /* Read our own dynamic section and fill in the info array. */ > > + ElfW(Dyn) * dyn_addr = ((void *) load_addr + elf_machine_dynamic > ()); > > + > > + unsigned long dynamic_info[36] = {0}; > > Could you use some generic define for the size of dynamic_info instead > of the hard coded 36? > > Like DYNAMIC_SIZE for instance? (I haven't checked if it's the correct > value for you) > > > + > > + /* Use the underlying function to avoid TLS access before > initialization */ > > + __dl_parse_dynamic_info(dyn_addr, dynamic_info, NULL, load_addr); > > + > > + ElfW(Word) relative_count = dynamic_info[DT_RELCONT_IDX]; > > + ElfW(Addr) rel_addr = dynamic_info[DT_RELOC_TABLE_ADDR]; > Maybe declare relative_count and rel_addr at the beginning of the function? > > + elf_machine_relative(load_addr, rel_addr, relative_count); > > + > > + return; > I would remove the return. > > +} > > diff --git a/libc/sysdeps/linux/arm/crt1.S > b/libc/sysdeps/linux/arm/crt1.S > > index a1d7f0f23..adaf4b983 100644 > > --- a/libc/sysdeps/linux/arm/crt1.S > > +++ b/libc/sysdeps/linux/arm/crt1.S > > @@ -246,6 +246,18 @@ _start: > > mov lr, #0 > > > > #ifdef __ARCH_USE_MMU__ > > +#ifdef L_rcrt1 > > + /* We don't need to save a1 since no dynamic linker should have > run */ > > + ldr a1, .L_GOT /* Get value at .L_GOT + 0 (offset to > GOT)*/ > > + adr a2, .L_GOT /* Get address of .L_GOT */ > > + ldr a3, .L_GOT+16 /* get value of _start(GOT) stored in > .L_GOT */ > > + adr a4, _start /* get address of _start after relocation > (changes to pc - ~30 or so) */ > > + add a1, a1, a2 /* calculate where the GOT is */ > > + ldr a2, [a1, a3] /* GOT + _start(GOT) = offset of _start > from begin of file */ > > + sub a1, a4, a2 /* current addr of _start - offset from > beginning of file = load addr */ > > + bl reloc_static_pie > > + mov a1, #0 /* Clean up a1 so that a random > address won't get called at the end of program */ > > +#endif > > /* Pop argc off the stack and save a pointer to argv */ > > ldr a2, [sp], #4 > > mov a3, sp > > @@ -309,6 +321,9 @@ _start: > > .word _fini(GOT) > > .word _init(GOT) > > .word main(GOT) > > +#ifdef L_rcrt1 > > + .word _start(GOT) > > +#endif > > #endif > > #endif > > > > diff --git a/libc/sysdeps/linux/i386/crt1.S > b/libc/sysdeps/linux/i386/crt1.S > > index 35a6552e8..4e4cef3ce 100644 > > --- a/libc/sysdeps/linux/i386/crt1.S > > +++ b/libc/sysdeps/linux/i386/crt1.S > > @@ -67,6 +67,9 @@ > > #endif > > .type main,%function > > .type __uClibc_main,%function > > +#ifdef L_rcrt1 > > +.type reloc_static_pie,%function > > +#endif > > _start: > > /* Clear the frame pointer. The ABI suggests this be done, to mark > > the outermost frame obviously. */ > > @@ -100,6 +103,23 @@ _start: > > pop %ebx > > addl $_GLOBAL_OFFSET_TABLE_+[.-.L0],%ebx > > > > +#ifdef L_rcrt1 > > + /* We cannot rely on _DYNAMIC being usable here due to RELRO. > > + Instead we calculate the load address based off a symbol > > + that we know will exist, _start. */ > nitpick: the comments of the assembly code for arm crt1.S are well > aligned whereas those for i386/x86_64 are not well aligned. > > + pushl %ecx /* Save > ecx so it won't get clobbered */ > > + pushl %ebx /* Save > ebx so it won't get clobbered */ > > + xorl %ecx, %ecx /* Clear ecx */ > > + addl _start@GOT(%ebx), %ecx /* Get the offset of _start */ > > + movl _start@GOT(%ebx), %eax /* Get the run time address of > _start */ > > + subl %ecx, %eax /* Subtract to > find the load address */ > > + pushl %eax /* Pass > the load address */ > > + call reloc_static_pie@PLT > > + popl %eax /* Clean > up from function call */ > > + popl %ebx /* Restore > the GOT address */ > > + popl %ecx /* restore > ecx */ > > +#endif > > + > > /* Push address of our own entry points to .fini and .init. */ > > pushl _fini@GOT(%ebx) > > pushl _init@GOT(%ebx) > > diff --git a/libc/sysdeps/linux/x86_64/crt1.S > b/libc/sysdeps/linux/x86_64/crt1.S > > index 87777dd5d..04536d07f 100644 > > --- a/libc/sysdeps/linux/x86_64/crt1.S > > +++ b/libc/sysdeps/linux/x86_64/crt1.S > > @@ -80,6 +80,20 @@ _start: > > the outermost frame obviously. */ > > xorl %ebp, %ebp > > > > +#ifdef L_rcrt1 > > + pushq %rdi /* > save rdi (but should be 0...) */ > > + pushq %rdx /* > store rdx (rtld_fini) */ > > + xorq %rcx, %rcx /* ensure > rcx is 0 */ > > + addq _start@GOTPCREL(%rip), %rcx /* get offset of _start > from beginning of file */ > > + movq _start@GOTPCREL(%rip), %rax /* get run time address > of _start */ > > + subq %rcx, %rax /* > calculate run time load offset */ > > + movq %rax, %rdi /* load > offset -> param 1 */ > > + call reloc_static_pie /* relocate > dynamic addrs */ > > + xorq %rax, %rax /* cleanup > */ > > + popq %rdx > > + popq %rdi > > +#endif > > + > > /* Extract the arguments as encoded on the stack and set up > > the arguments for __libc_start_main (int (*main) (int, char **, > char **), > > int argc, char *argv, > > @@ -107,7 +121,7 @@ _start: > > which grow downwards). */ > > pushq %rsp > > > > -#if defined(L_Scrt1) > > +#if defined(L_Scrt1) || defined(L_rcrt1) > > /* Give address for main() */ > > movq main@GOTPCREL(%rip), %rdi > > > > -- 2.34.1 > > > > _______________________________________________ > > devel mailing list [email protected] > > To unsubscribe send an email [email protected] > > Pièces jointes : > > > > 0001-Added-static-pie-support-for-i368-x86_64-and-arm.patch 8,2 Ko > > > _______________________________________________ > devel mailing list -- [email protected] > To unsubscribe send an email to [email protected] >
_______________________________________________ devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
