* [PATCH v5 1/2] Linux: Introduce __brk_call for invoking the brk system call
@ 2022-05-09 18:18 Florian Weimer
2022-05-09 18:18 ` [PATCH v5 2/2] csu: Implement and use _dl_early_allocate during static startup Florian Weimer
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Florian Weimer @ 2022-05-09 18:18 UTC (permalink / raw)
To: libc-alpha
Alpha and sparc can now use the generic implementation.
---
sysdeps/unix/sysv/linux/alpha/brk_call.h | 28 +++++++++
sysdeps/unix/sysv/linux/brk.c | 3 +-
sysdeps/unix/sysv/linux/brk_call.h | 25 ++++++++
sysdeps/unix/sysv/linux/sparc/brk.c | 58 -------------------
.../linux/{alpha/brk.c => sparc/brk_call.h} | 35 +++++------
5 files changed, 71 insertions(+), 78 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/alpha/brk_call.h
create mode 100644 sysdeps/unix/sysv/linux/brk_call.h
delete mode 100644 sysdeps/unix/sysv/linux/sparc/brk.c
rename sysdeps/unix/sysv/linux/{alpha/brk.c => sparc/brk_call.h} (61%)
diff --git a/sysdeps/unix/sysv/linux/alpha/brk_call.h b/sysdeps/unix/sysv/linux/alpha/brk_call.h
new file mode 100644
index 0000000000..b8088cf13f
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/alpha/brk_call.h
@@ -0,0 +1,28 @@
+/* Invoke the brk system call. Alpha version.
+ Copyright (C) 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/>. */
+
+static inline void *
+__brk_call (void *addr)
+{
+ unsigned long int result = INTERNAL_SYSCALL_CALL (brk, addr);
+ if (result == -ENOMEM)
+ /* Mimic the default error reporting behavior. */
+ return addr;
+ else
+ return (void *) result;
+}
diff --git a/sysdeps/unix/sysv/linux/brk.c b/sysdeps/unix/sysv/linux/brk.c
index abbabc9e8b..9264a5a4a2 100644
--- a/sysdeps/unix/sysv/linux/brk.c
+++ b/sysdeps/unix/sysv/linux/brk.c
@@ -19,6 +19,7 @@
#include <errno.h>
#include <unistd.h>
#include <sysdep.h>
+#include <brk_call.h>
/* This must be initialized data because commons can't have aliases. */
void *__curbrk = 0;
@@ -33,7 +34,7 @@ weak_alias (__curbrk, ___brk_addr)
int
__brk (void *addr)
{
- __curbrk = (void *) INTERNAL_SYSCALL_CALL (brk, addr);
+ __curbrk = __brk_call (addr);
if (__curbrk < addr)
{
__set_errno (ENOMEM);
diff --git a/sysdeps/unix/sysv/linux/brk_call.h b/sysdeps/unix/sysv/linux/brk_call.h
new file mode 100644
index 0000000000..72370c25d7
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/brk_call.h
@@ -0,0 +1,25 @@
+/* Invoke the brk system call. Generic Linux version.
+ Copyright (C) 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/>. */
+
+static inline void *
+__brk_call (void *addr)
+{
+ /* The default implementation reports errors through an unchanged
+ break. */
+ return (void *) INTERNAL_SYSCALL_CALL (brk, addr);
+}
diff --git a/sysdeps/unix/sysv/linux/sparc/brk.c b/sysdeps/unix/sysv/linux/sparc/brk.c
deleted file mode 100644
index c5c1ee0282..0000000000
--- a/sysdeps/unix/sysv/linux/sparc/brk.c
+++ /dev/null
@@ -1,58 +0,0 @@
-/* Change data segment. Linux SPARC version.
- Copyright (C) 2021-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 <errno.h>
-#include <unistd.h>
-#include <sysdep.h>
-
-/* This must be initialized data because commons can't have aliases. */
-void *__curbrk = 0;
-
-#if HAVE_INTERNAL_BRK_ADDR_SYMBOL
-/* Old braindamage in GCC's crtstuff.c requires this symbol in an attempt
- to work around different old braindamage in the old Linux ELF dynamic
- linker. */
-weak_alias (__curbrk, ___brk_addr)
-#endif
-
-#ifdef __arch64__
-# define SYSCALL_NUM "0x6d"
-#else
-# define SYSCALL_NUM "0x10"
-#endif
-
-int
-__brk (void *addr)
-{
- register long int g1 asm ("g1") = __NR_brk;
- register long int o0 asm ("o0") = (long int) addr;
- asm volatile ("ta " SYSCALL_NUM
- : "=r"(o0)
- : "r"(g1), "0"(o0)
- : "cc");
- __curbrk = (void *) o0;
-
- if (__curbrk < addr)
- {
- __set_errno (ENOMEM);
- return -1;
- }
-
- return 0;
-}
-weak_alias (__brk, brk)
diff --git a/sysdeps/unix/sysv/linux/alpha/brk.c b/sysdeps/unix/sysv/linux/sparc/brk_call.h
similarity index 61%
rename from sysdeps/unix/sysv/linux/alpha/brk.c
rename to sysdeps/unix/sysv/linux/sparc/brk_call.h
index 32082a4fae..59ce521660 100644
--- a/sysdeps/unix/sysv/linux/alpha/brk.c
+++ b/sysdeps/unix/sysv/linux/sparc/brk_call.h
@@ -1,5 +1,5 @@
-/* Change data segment size. Linux/Alpha.
- Copyright (C) 2020-2022 Free Software Foundation, Inc.
+/* Invoke the brk system call. Sparc version.
+ Copyright (C) 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
@@ -16,23 +16,20 @@
License along with the GNU C Library. If not, see
<https://www.gnu.org/licenses/>. */
-#include <errno.h>
-#include <unistd.h>
-#include <sysdep.h>
+#ifdef __arch64__
+# define SYSCALL_NUM "0x6d"
+#else
+# define SYSCALL_NUM "0x10"
+#endif
-void *__curbrk = 0;
-
-int
-__brk (void *addr)
+static inline void *
+__brk_call (void *addr)
{
- /* Alpha brk returns -ENOMEM in case of failure. */
- __curbrk = (void *) INTERNAL_SYSCALL_CALL (brk, addr);
- if ((unsigned long) __curbrk == -ENOMEM)
- {
- __set_errno (ENOMEM);
- return -1;
- }
-
- return 0;
+ register long int g1 asm ("g1") = __NR_brk;
+ register long int o0 asm ("o0") = (long int) addr;
+ asm volatile ("ta " SYSCALL_NUM
+ : "=r"(o0)
+ : "r"(g1), "0"(o0)
+ : "cc");
+ return (void *) o0;
}
-weak_alias (__brk, brk)
base-commit: a2a6bce7d7e52c1c34369a7da62c501cc350bc31
--
2.35.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 2/2] csu: Implement and use _dl_early_allocate during static startup
2022-05-09 18:18 [PATCH v5 1/2] Linux: Introduce __brk_call for invoking the brk system call Florian Weimer
@ 2022-05-09 18:18 ` Florian Weimer
2022-05-12 13:15 ` Adhemerval Zanella
2022-05-12 13:01 ` [PATCH v5 1/2] Linux: Introduce __brk_call for invoking the brk system call Adhemerval Zanella
2022-05-12 13:25 ` Andreas Schwab
2 siblings, 1 reply; 7+ messages in thread
From: Florian Weimer @ 2022-05-09 18:18 UTC (permalink / raw)
To: libc-alpha
This implements mmap fallback for a brk failure during TLS
allocation.
scripts/tls-elf-edit.py is updated to support the new patching method.
The script no longer requires that in the input object is of ET_DYN
type.
---
v5: Use __brk_call.
csu/libc-tls.c | 11 ++-
elf/Makefile | 19 +++++
elf/dl-early_allocate.c | 30 ++++++++
elf/tst-tls-allocation-failure-static.c | 31 ++++++++
scripts/tst-elf-edit.py | 34 +++++++--
sysdeps/generic/ldsodefs.h | 5 ++
sysdeps/unix/sysv/linux/dl-early_allocate.c | 82 +++++++++++++++++++++
7 files changed, 202 insertions(+), 10 deletions(-)
create mode 100644 elf/dl-early_allocate.c
create mode 100644 elf/tst-tls-allocation-failure-static.c
create mode 100644 sysdeps/unix/sysv/linux/dl-early_allocate.c
diff --git a/csu/libc-tls.c b/csu/libc-tls.c
index bef92a7568..0a216c5502 100644
--- a/csu/libc-tls.c
+++ b/csu/libc-tls.c
@@ -145,11 +145,16 @@ __libc_setup_tls (void)
_dl_allocate_tls_storage (in elf/dl-tls.c) does using __libc_memalign
and dl_tls_static_align. */
tcb_offset = roundup (memsz + GLRO(dl_tls_static_surplus), max_align);
- tlsblock = __sbrk (tcb_offset + TLS_INIT_TCB_SIZE + max_align);
+ tlsblock = _dl_early_allocate (tcb_offset + TLS_INIT_TCB_SIZE + max_align);
+ if (tlsblock == NULL)
+ _startup_fatal ("Fatal glibc error: Cannot allocate TLS block\n");
#elif TLS_DTV_AT_TP
tcb_offset = roundup (TLS_INIT_TCB_SIZE, align ?: 1);
- tlsblock = __sbrk (tcb_offset + memsz + max_align
- + TLS_PRE_TCB_SIZE + GLRO(dl_tls_static_surplus));
+ tlsblock = _dl_early_allocate (tcb_offset + memsz + max_align
+ + TLS_PRE_TCB_SIZE
+ + GLRO(dl_tls_static_surplus));
+ if (tlsblock == NULL)
+ _startup_fatal ("Fatal glibc error: Cannot allocate TLS block\n");
tlsblock += TLS_PRE_TCB_SIZE;
#else
/* In case a model with a different layout for the TCB and DTV
diff --git a/elf/Makefile b/elf/Makefile
index fc9860edee..ce3345ed92 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -33,6 +33,7 @@ routines = \
$(all-dl-routines) \
dl-addr \
dl-addr-obj \
+ dl-early_allocate \
dl-error \
dl-iteratephdr \
dl-libc \
@@ -108,6 +109,7 @@ all-dl-routines = $(dl-routines) $(sysdep-dl-routines)
# But they are absent from the shared libc, because that code is in ld.so.
elide-routines.os = \
$(all-dl-routines) \
+ dl-early_allocate \
dl-exception \
dl-origin \
dl-reloc-static-pie \
@@ -276,6 +278,7 @@ tests-static-normal := \
tst-linkall-static \
tst-single_threaded-pthread-static \
tst-single_threaded-static \
+ tst-tls-allocation-failure-static \
tst-tlsalign-extern-static \
tst-tlsalign-static \
# tests-static-normal
@@ -1213,6 +1216,10 @@ $(objpfx)tst-glibcelf.out: tst-glibcelf.py elf.h $(..)/scripts/glibcelf.py \
--cc="$(CC) $(patsubst -DMODULE_NAME=%,-DMODULE_NAME=testsuite,$(CPPFLAGS))" \
< /dev/null > $@ 2>&1; $(evaluate-test)
+ifeq ($(run-built-tests),yes)
+tests-special += $(objpfx)tst-tls-allocation-failure-static-patched.out
+endif
+
# The test requires shared _and_ PIE because the executable
# unit test driver must be able to link with the shared object
# that is going to eventually go into an installed DSO.
@@ -2937,3 +2944,15 @@ $(eval $(call tst-trace-skeleton,4,\
$(objpfx)libtracemod2:$(objpfx)libtracemod3:$(objpfx)libtracemod4))
$(eval $(call tst-trace-skeleton,5,\
$(objpfx)libtracemod2:$(objpfx)libtracemod3:$(objpfx)libtracemod4:$(objpfx)libtracemod5))
+
+$(objpfx)tst-tls-allocation-failure-static-patched: \
+ $(objpfx)tst-tls-allocation-failure-static $(..)scripts/tst-elf-edit.py
+ cp $< $@
+ $(PYTHON) $(..)scripts/tst-elf-edit.py --maximize-tls-size $@
+
+$(objpfx)tst-tls-allocation-failure-static-patched.out: \
+ $(objpfx)tst-tls-allocation-failure-static-patched
+ $< > $@ 2>&1; echo "status: $$?" >> $@
+ grep -q '^Fatal glibc error: Cannot allocate TLS block$$' $@ \
+ && grep -q '^status: 127$$' $@; \
+ $(evaluate-test)
diff --git a/elf/dl-early_allocate.c b/elf/dl-early_allocate.c
new file mode 100644
index 0000000000..61677aaa03
--- /dev/null
+++ b/elf/dl-early_allocate.c
@@ -0,0 +1,30 @@
+/* Early memory allocation for the dynamic loader. Generic version.
+ Copyright (C) 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 <ldsodefs.h>
+#include <stddef.h>
+#include <unistd.h>
+
+void *
+_dl_early_allocate (size_t size)
+{
+ void *result = __sbrk (size);
+ if (result == (void *) -1)
+ result = NULL;
+ return result;
+}
diff --git a/elf/tst-tls-allocation-failure-static.c b/elf/tst-tls-allocation-failure-static.c
new file mode 100644
index 0000000000..8de831b246
--- /dev/null
+++ b/elf/tst-tls-allocation-failure-static.c
@@ -0,0 +1,31 @@
+/* Base for test program with impossiblyh large PT_TLS segment.
+ Copyright (C) 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/>. */
+
+/* The test actual binary is patched using scripts/tst-elf-edit.py
+ --maximize-tls-size, and this introduces the expected test
+ allocation failure due to an excessive PT_LS p_memsz value.
+
+ Patching the binary is required because on some 64-bit targets, TLS
+ relocations can only cover a 32-bit range, and glibc-internal TLS
+ variables such as errno end up outside that range. */
+
+int
+main (void)
+{
+ return 0;
+}
diff --git a/scripts/tst-elf-edit.py b/scripts/tst-elf-edit.py
index a514179bbf..0e19ce1e73 100644
--- a/scripts/tst-elf-edit.py
+++ b/scripts/tst-elf-edit.py
@@ -43,9 +43,11 @@ EI_DATA=5
ELFDATA2LSB=b'\x01'
ELFDATA2MSB=b'\x02'
+ET_EXEC=2
ET_DYN=3
PT_LOAD=1
+PT_TLS=7
def elf_types_fmts(e_ident):
endian = '<' if e_ident[EI_DATA] == ELFDATA2LSB else '>'
@@ -146,8 +148,15 @@ def elf_edit_align(phdr, align):
else:
phdr.p_align = int(align)
+def elf_edit_maximize_tls_size(phdr, elfclass):
+ if elfclass == ELFCLASS32:
+ # It is possible that the kernel can allocate half of the
+ # address space, so use something larger.
+ phdr.p_memsz = 0xfff00000
+ else:
+ phdr.p_memsz = 1 << 63
-def elf_edit(f, align):
+def elf_edit(f, opts):
ei_nident_fmt = 'c' * EI_NIDENT
ei_nident_len = struct.calcsize(ei_nident_fmt)
@@ -172,24 +181,35 @@ def elf_edit(f, align):
ehdr = Elf_Ehdr(e_ident)
ehdr.read(f)
- if ehdr.e_type != ET_DYN:
- error('{}: not a shared library'.format(f.name))
+ if ehdr.e_type not in (ET_EXEC, ET_DYN):
+ error('{}: not an executable or shared library'.format(f.name))
phdr = Elf_Phdr(e_ident)
+ maximize_tls_size_done = False
for i in range(0, ehdr.e_phnum):
f.seek(ehdr.e_phoff + i * phdr.len)
phdr.read(f)
- if phdr.p_type == PT_LOAD:
- elf_edit_align(phdr, align)
+ if phdr.p_type == PT_LOAD and opts.align is not None:
+ elf_edit_align(phdr, opts.align)
+ f.seek(ehdr.e_phoff + i * phdr.len)
+ phdr.write(f)
+ break
+ if phdr.p_type == PT_TLS and opts.maximize_tls_size:
+ elf_edit_maximize_tls_size(phdr, e_ident[EI_CLASS])
f.seek(ehdr.e_phoff + i * phdr.len)
phdr.write(f)
+ maximize_tls_size_done = True
break
+ if opts.maximize_tls_size and not maximize_tls_size_done:
+ error('{}: TLS maximum size was not updated'.format(f.name))
def get_parser():
parser = argparse.ArgumentParser(description=__doc__)
- parser.add_argument('-a', dest='align', required=True,
+ parser.add_argument('-a', dest='align',
help='How to set the LOAD alignment')
+ parser.add_argument('--maximize-tls-size', action='store_true',
+ help='Set maximum PT_TLS size')
parser.add_argument('output',
help='ELF file to edit')
return parser
@@ -199,7 +219,7 @@ def main(argv):
parser = get_parser()
opts = parser.parse_args(argv)
with open(opts.output, 'r+b') as fout:
- elf_edit(fout, opts.align)
+ elf_edit(fout, opts)
if __name__ == '__main__':
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 4a5e698db2..5d0369358d 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1211,6 +1211,11 @@ extern struct link_map * _dl_get_dl_main_map (void)
# endif
#endif
+/* Perform early memory allocation, avoding a TCB dependency.
+ Terminate the process if allocation fails. May attempt to use
+ brk. */
+void *_dl_early_allocate (size_t size) attribute_hidden;
+
/* Initialize the DSO sort algorithm to use. */
#if !HAVE_TUNABLES
static inline void
diff --git a/sysdeps/unix/sysv/linux/dl-early_allocate.c b/sysdeps/unix/sysv/linux/dl-early_allocate.c
new file mode 100644
index 0000000000..52c538e85a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/dl-early_allocate.c
@@ -0,0 +1,82 @@
+/* Early memory allocation for the dynamic loader. Generic version.
+ Copyright (C) 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/>. */
+
+/* Mark symbols hidden in static PIE for early self relocation to work. */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
+#include <startup.h>
+
+#include <ldsodefs.h>
+#include <stddef.h>
+#include <string.h>
+#include <sysdep.h>
+#include <unistd.h>
+
+#include <brk_call.h>
+#include <mmap_call.h>
+
+/* Defined in brk.c. */
+extern void *__curbrk;
+
+void *
+_dl_early_allocate (size_t size)
+{
+ void *result;
+
+ if (__curbrk != NULL)
+ /* If the break has been initialized, brk must have run before,
+ so just call it once more. */
+ {
+ result = __sbrk (size);
+ if (result == (void *) -1)
+ result = NULL;
+ }
+ else
+ {
+ /* If brk has not been invoked, there is no need to update
+ __curbrk. The first call to brk will take care of that. */
+ void *previous = __brk_call (0);
+ result = __brk_call (previous + size);
+ if (result == previous)
+ result = NULL;
+ else
+ result = previous;
+ }
+
+ /* If brk fails, fall back to mmap. This can happen due to
+ unfortunate ASLR layout decisions and kernel bugs, particularly
+ for static PIE. */
+ if (result == NULL)
+ {
+ long int ret;
+ int prot = PROT_READ | PROT_WRITE;
+ int flags = MAP_PRIVATE | MAP_ANONYMOUS;
+#ifdef __NR_mmap2
+ ret = MMAP_CALL_INTERNAL (mmap2, 0, size, prot, flags, -1, 0);
+#else
+ ret = MMAP_CALL_INTERNAL (mmap, 0, size, prot, flags, -1, 0);
+#endif
+ if (INTERNAL_SYSCALL_ERROR_P (ret))
+ result = NULL;
+ else
+ result = (void *) ret;
+ }
+
+ return result;
+}
--
2.35.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/2] Linux: Introduce __brk_call for invoking the brk system call
2022-05-09 18:18 [PATCH v5 1/2] Linux: Introduce __brk_call for invoking the brk system call Florian Weimer
2022-05-09 18:18 ` [PATCH v5 2/2] csu: Implement and use _dl_early_allocate during static startup Florian Weimer
@ 2022-05-12 13:01 ` Adhemerval Zanella
2022-05-12 13:25 ` Andreas Schwab
2 siblings, 0 replies; 7+ messages in thread
From: Adhemerval Zanella @ 2022-05-12 13:01 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 09/05/2022 15:18, Florian Weimer via Libc-alpha wrote:
> Alpha and sparc can now use the generic implementation.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/unix/sysv/linux/alpha/brk_call.h | 28 +++++++++
> sysdeps/unix/sysv/linux/brk.c | 3 +-
> sysdeps/unix/sysv/linux/brk_call.h | 25 ++++++++
> sysdeps/unix/sysv/linux/sparc/brk.c | 58 -------------------
> .../linux/{alpha/brk.c => sparc/brk_call.h} | 35 +++++------
> 5 files changed, 71 insertions(+), 78 deletions(-)
> create mode 100644 sysdeps/unix/sysv/linux/alpha/brk_call.h
> create mode 100644 sysdeps/unix/sysv/linux/brk_call.h
> delete mode 100644 sysdeps/unix/sysv/linux/sparc/brk.c
> rename sysdeps/unix/sysv/linux/{alpha/brk.c => sparc/brk_call.h} (61%)
>
> diff --git a/sysdeps/unix/sysv/linux/alpha/brk_call.h b/sysdeps/unix/sysv/linux/alpha/brk_call.h
> new file mode 100644
> index 0000000000..b8088cf13f
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/alpha/brk_call.h
> @@ -0,0 +1,28 @@
> +/* Invoke the brk system call. Alpha version.
> + Copyright (C) 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/>. */
> +
> +static inline void *
> +__brk_call (void *addr)
> +{
> + unsigned long int result = INTERNAL_SYSCALL_CALL (brk, addr);
> + if (result == -ENOMEM)
> + /* Mimic the default error reporting behavior. */
> + return addr;
> + else
> + return (void *) result;
> +}
> diff --git a/sysdeps/unix/sysv/linux/brk.c b/sysdeps/unix/sysv/linux/brk.c
> index abbabc9e8b..9264a5a4a2 100644
> --- a/sysdeps/unix/sysv/linux/brk.c
> +++ b/sysdeps/unix/sysv/linux/brk.c
> @@ -19,6 +19,7 @@
> #include <errno.h>
> #include <unistd.h>
> #include <sysdep.h>
> +#include <brk_call.h>
>
> /* This must be initialized data because commons can't have aliases. */
> void *__curbrk = 0;
> @@ -33,7 +34,7 @@ weak_alias (__curbrk, ___brk_addr)
> int
> __brk (void *addr)
> {
> - __curbrk = (void *) INTERNAL_SYSCALL_CALL (brk, addr);
> + __curbrk = __brk_call (addr);
> if (__curbrk < addr)
> {
> __set_errno (ENOMEM);
> diff --git a/sysdeps/unix/sysv/linux/brk_call.h b/sysdeps/unix/sysv/linux/brk_call.h
> new file mode 100644
> index 0000000000..72370c25d7
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/brk_call.h
> @@ -0,0 +1,25 @@
> +/* Invoke the brk system call. Generic Linux version.
> + Copyright (C) 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/>. */
> +
> +static inline void *
> +__brk_call (void *addr)
> +{
> + /* The default implementation reports errors through an unchanged
> + break. */
> + return (void *) INTERNAL_SYSCALL_CALL (brk, addr);
> +}
> diff --git a/sysdeps/unix/sysv/linux/sparc/brk.c b/sysdeps/unix/sysv/linux/sparc/brk.c
> deleted file mode 100644
> index c5c1ee0282..0000000000
> --- a/sysdeps/unix/sysv/linux/sparc/brk.c
> +++ /dev/null
> @@ -1,58 +0,0 @@
> -/* Change data segment. Linux SPARC version.
> - Copyright (C) 2021-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 <errno.h>
> -#include <unistd.h>
> -#include <sysdep.h>
> -
> -/* This must be initialized data because commons can't have aliases. */
> -void *__curbrk = 0;
> -
> -#if HAVE_INTERNAL_BRK_ADDR_SYMBOL
> -/* Old braindamage in GCC's crtstuff.c requires this symbol in an attempt
> - to work around different old braindamage in the old Linux ELF dynamic
> - linker. */
> -weak_alias (__curbrk, ___brk_addr)
> -#endif
> -
> -#ifdef __arch64__
> -# define SYSCALL_NUM "0x6d"
> -#else
> -# define SYSCALL_NUM "0x10"
> -#endif
> -
> -int
> -__brk (void *addr)
> -{
> - register long int g1 asm ("g1") = __NR_brk;
> - register long int o0 asm ("o0") = (long int) addr;
> - asm volatile ("ta " SYSCALL_NUM
> - : "=r"(o0)
> - : "r"(g1), "0"(o0)
> - : "cc");
> - __curbrk = (void *) o0;
> -
> - if (__curbrk < addr)
> - {
> - __set_errno (ENOMEM);
> - return -1;
> - }
> -
> - return 0;
> -}
> -weak_alias (__brk, brk)
> diff --git a/sysdeps/unix/sysv/linux/alpha/brk.c b/sysdeps/unix/sysv/linux/sparc/brk_call.h
> similarity index 61%
> rename from sysdeps/unix/sysv/linux/alpha/brk.c
> rename to sysdeps/unix/sysv/linux/sparc/brk_call.h
> index 32082a4fae..59ce521660 100644
> --- a/sysdeps/unix/sysv/linux/alpha/brk.c
> +++ b/sysdeps/unix/sysv/linux/sparc/brk_call.h
> @@ -1,5 +1,5 @@
> -/* Change data segment size. Linux/Alpha.
> - Copyright (C) 2020-2022 Free Software Foundation, Inc.
> +/* Invoke the brk system call. Sparc version.
> + Copyright (C) 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
> @@ -16,23 +16,20 @@
> License along with the GNU C Library. If not, see
> <https://www.gnu.org/licenses/>. */
>
> -#include <errno.h>
> -#include <unistd.h>
> -#include <sysdep.h>
> +#ifdef __arch64__
> +# define SYSCALL_NUM "0x6d"
> +#else
> +# define SYSCALL_NUM "0x10"
> +#endif
>
> -void *__curbrk = 0;
> -
> -int
> -__brk (void *addr)
> +static inline void *
> +__brk_call (void *addr)
> {
> - /* Alpha brk returns -ENOMEM in case of failure. */
> - __curbrk = (void *) INTERNAL_SYSCALL_CALL (brk, addr);
> - if ((unsigned long) __curbrk == -ENOMEM)
> - {
> - __set_errno (ENOMEM);
> - return -1;
> - }
> -
> - return 0;
> + register long int g1 asm ("g1") = __NR_brk;
> + register long int o0 asm ("o0") = (long int) addr;
> + asm volatile ("ta " SYSCALL_NUM
> + : "=r"(o0)
> + : "r"(g1), "0"(o0)
> + : "cc");
> + return (void *) o0;
> }
> -weak_alias (__brk, brk)
>
> base-commit: a2a6bce7d7e52c1c34369a7da62c501cc350bc31
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] csu: Implement and use _dl_early_allocate during static startup
2022-05-09 18:18 ` [PATCH v5 2/2] csu: Implement and use _dl_early_allocate during static startup Florian Weimer
@ 2022-05-12 13:15 ` Adhemerval Zanella
2022-05-12 18:37 ` Florian Weimer
0 siblings, 1 reply; 7+ messages in thread
From: Adhemerval Zanella @ 2022-05-12 13:15 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 09/05/2022 15:18, Florian Weimer via Libc-alpha wrote:
> This implements mmap fallback for a brk failure during TLS
> allocation.
>
> scripts/tls-elf-edit.py is updated to support the new patching method.
> The script no longer requires that in the input object is of ET_DYN
> type.
> ---
> v5: Use __brk_call.
> csu/libc-tls.c | 11 ++-
> elf/Makefile | 19 +++++
> elf/dl-early_allocate.c | 30 ++++++++
> elf/tst-tls-allocation-failure-static.c | 31 ++++++++
> scripts/tst-elf-edit.py | 34 +++++++--
> sysdeps/generic/ldsodefs.h | 5 ++
> sysdeps/unix/sysv/linux/dl-early_allocate.c | 82 +++++++++++++++++++++
> 7 files changed, 202 insertions(+), 10 deletions(-)
> create mode 100644 elf/dl-early_allocate.c
> create mode 100644 elf/tst-tls-allocation-failure-static.c
> create mode 100644 sysdeps/unix/sysv/linux/dl-early_allocate.c
>
> diff --git a/csu/libc-tls.c b/csu/libc-tls.c
> index bef92a7568..0a216c5502 100644
> --- a/csu/libc-tls.c
> +++ b/csu/libc-tls.c
> @@ -145,11 +145,16 @@ __libc_setup_tls (void)
> _dl_allocate_tls_storage (in elf/dl-tls.c) does using __libc_memalign
> and dl_tls_static_align. */
> tcb_offset = roundup (memsz + GLRO(dl_tls_static_surplus), max_align);
> - tlsblock = __sbrk (tcb_offset + TLS_INIT_TCB_SIZE + max_align);
> + tlsblock = _dl_early_allocate (tcb_offset + TLS_INIT_TCB_SIZE + max_align);
> + if (tlsblock == NULL)
> + _startup_fatal ("Fatal glibc error: Cannot allocate TLS block\n");
> #elif TLS_DTV_AT_TP
> tcb_offset = roundup (TLS_INIT_TCB_SIZE, align ?: 1);
> - tlsblock = __sbrk (tcb_offset + memsz + max_align
> - + TLS_PRE_TCB_SIZE + GLRO(dl_tls_static_surplus));
> + tlsblock = _dl_early_allocate (tcb_offset + memsz + max_align
> + + TLS_PRE_TCB_SIZE
> + + GLRO(dl_tls_static_surplus));
> + if (tlsblock == NULL)
> + _startup_fatal ("Fatal glibc error: Cannot allocate TLS block\n");
> tlsblock += TLS_PRE_TCB_SIZE;
> #else
> /* In case a model with a different layout for the TCB and DTV
> diff --git a/elf/Makefile b/elf/Makefile
> index fc9860edee..ce3345ed92 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -33,6 +33,7 @@ routines = \
> $(all-dl-routines) \
> dl-addr \
> dl-addr-obj \
> + dl-early_allocate \
> dl-error \
> dl-iteratephdr \
> dl-libc \
> @@ -108,6 +109,7 @@ all-dl-routines = $(dl-routines) $(sysdep-dl-routines)
> # But they are absent from the shared libc, because that code is in ld.so.
> elide-routines.os = \
> $(all-dl-routines) \
> + dl-early_allocate \
> dl-exception \
> dl-origin \
> dl-reloc-static-pie \
> @@ -276,6 +278,7 @@ tests-static-normal := \
> tst-linkall-static \
> tst-single_threaded-pthread-static \
> tst-single_threaded-static \
> + tst-tls-allocation-failure-static \
> tst-tlsalign-extern-static \
> tst-tlsalign-static \
> # tests-static-normal
> @@ -1213,6 +1216,10 @@ $(objpfx)tst-glibcelf.out: tst-glibcelf.py elf.h $(..)/scripts/glibcelf.py \
> --cc="$(CC) $(patsubst -DMODULE_NAME=%,-DMODULE_NAME=testsuite,$(CPPFLAGS))" \
> < /dev/null > $@ 2>&1; $(evaluate-test)
>
> +ifeq ($(run-built-tests),yes)
> +tests-special += $(objpfx)tst-tls-allocation-failure-static-patched.out
> +endif
> +
> # The test requires shared _and_ PIE because the executable
> # unit test driver must be able to link with the shared object
> # that is going to eventually go into an installed DSO.
> @@ -2937,3 +2944,15 @@ $(eval $(call tst-trace-skeleton,4,\
> $(objpfx)libtracemod2:$(objpfx)libtracemod3:$(objpfx)libtracemod4))
> $(eval $(call tst-trace-skeleton,5,\
> $(objpfx)libtracemod2:$(objpfx)libtracemod3:$(objpfx)libtracemod4:$(objpfx)libtracemod5))
> +
> +$(objpfx)tst-tls-allocation-failure-static-patched: \
> + $(objpfx)tst-tls-allocation-failure-static $(..)scripts/tst-elf-edit.py
> + cp $< $@
> + $(PYTHON) $(..)scripts/tst-elf-edit.py --maximize-tls-size $@
> +
> +$(objpfx)tst-tls-allocation-failure-static-patched.out: \
> + $(objpfx)tst-tls-allocation-failure-static-patched
> + $< > $@ 2>&1; echo "status: $$?" >> $@
> + grep -q '^Fatal glibc error: Cannot allocate TLS block$$' $@ \
> + && grep -q '^status: 127$$' $@; \
> + $(evaluate-test)
> diff --git a/elf/dl-early_allocate.c b/elf/dl-early_allocate.c
> new file mode 100644
> index 0000000000..61677aaa03
> --- /dev/null
> +++ b/elf/dl-early_allocate.c
> @@ -0,0 +1,30 @@
> +/* Early memory allocation for the dynamic loader. Generic version.
> + Copyright (C) 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 <ldsodefs.h>
> +#include <stddef.h>
> +#include <unistd.h>
> +
> +void *
> +_dl_early_allocate (size_t size)
> +{
> + void *result = __sbrk (size);
> + if (result == (void *) -1)
> + result = NULL;
> + return result;
> +}
> diff --git a/elf/tst-tls-allocation-failure-static.c b/elf/tst-tls-allocation-failure-static.c
> new file mode 100644
> index 0000000000..8de831b246
> --- /dev/null
> +++ b/elf/tst-tls-allocation-failure-static.c
> @@ -0,0 +1,31 @@
> +/* Base for test program with impossiblyh large PT_TLS segment.
> + Copyright (C) 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/>. */
> +
> +/* The test actual binary is patched using scripts/tst-elf-edit.py
> + --maximize-tls-size, and this introduces the expected test
> + allocation failure due to an excessive PT_LS p_memsz value.
> +
> + Patching the binary is required because on some 64-bit targets, TLS
> + relocations can only cover a 32-bit range, and glibc-internal TLS
> + variables such as errno end up outside that range. */
> +
> +int
> +main (void)
> +{
> + return 0;
> +}
> diff --git a/scripts/tst-elf-edit.py b/scripts/tst-elf-edit.py
> index a514179bbf..0e19ce1e73 100644
> --- a/scripts/tst-elf-edit.py
> +++ b/scripts/tst-elf-edit.py
> @@ -43,9 +43,11 @@ EI_DATA=5
> ELFDATA2LSB=b'\x01'
> ELFDATA2MSB=b'\x02'
>
> +ET_EXEC=2
> ET_DYN=3
>
> PT_LOAD=1
> +PT_TLS=7
>
> def elf_types_fmts(e_ident):
> endian = '<' if e_ident[EI_DATA] == ELFDATA2LSB else '>'
> @@ -146,8 +148,15 @@ def elf_edit_align(phdr, align):
> else:
> phdr.p_align = int(align)
>
> +def elf_edit_maximize_tls_size(phdr, elfclass):
> + if elfclass == ELFCLASS32:
> + # It is possible that the kernel can allocate half of the
> + # address space, so use something larger.
> + phdr.p_memsz = 0xfff00000
> + else:
> + phdr.p_memsz = 1 << 63
>
> -def elf_edit(f, align):
> +def elf_edit(f, opts):
> ei_nident_fmt = 'c' * EI_NIDENT
> ei_nident_len = struct.calcsize(ei_nident_fmt)
>
> @@ -172,24 +181,35 @@ def elf_edit(f, align):
>
> ehdr = Elf_Ehdr(e_ident)
> ehdr.read(f)
> - if ehdr.e_type != ET_DYN:
> - error('{}: not a shared library'.format(f.name))
> + if ehdr.e_type not in (ET_EXEC, ET_DYN):
> + error('{}: not an executable or shared library'.format(f.name))
>
> phdr = Elf_Phdr(e_ident)
> + maximize_tls_size_done = False
> for i in range(0, ehdr.e_phnum):
> f.seek(ehdr.e_phoff + i * phdr.len)
> phdr.read(f)
> - if phdr.p_type == PT_LOAD:
> - elf_edit_align(phdr, align)
> + if phdr.p_type == PT_LOAD and opts.align is not None:
> + elf_edit_align(phdr, opts.align)
> + f.seek(ehdr.e_phoff + i * phdr.len)
> + phdr.write(f)
> + break
> + if phdr.p_type == PT_TLS and opts.maximize_tls_size:
> + elf_edit_maximize_tls_size(phdr, e_ident[EI_CLASS])
> f.seek(ehdr.e_phoff + i * phdr.len)
> phdr.write(f)
> + maximize_tls_size_done = True
> break
>
> + if opts.maximize_tls_size and not maximize_tls_size_done:
> + error('{}: TLS maximum size was not updated'.format(f.name))
>
> def get_parser():
> parser = argparse.ArgumentParser(description=__doc__)
> - parser.add_argument('-a', dest='align', required=True,
> + parser.add_argument('-a', dest='align',
> help='How to set the LOAD alignment')
> + parser.add_argument('--maximize-tls-size', action='store_true',
> + help='Set maximum PT_TLS size')
> parser.add_argument('output',
> help='ELF file to edit')
> return parser
> @@ -199,7 +219,7 @@ def main(argv):
> parser = get_parser()
> opts = parser.parse_args(argv)
> with open(opts.output, 'r+b') as fout:
> - elf_edit(fout, opts.align)
> + elf_edit(fout, opts)
>
>
> if __name__ == '__main__':
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 4a5e698db2..5d0369358d 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -1211,6 +1211,11 @@ extern struct link_map * _dl_get_dl_main_map (void)
> # endif
> #endif
>
> +/* Perform early memory allocation, avoding a TCB dependency.
> + Terminate the process if allocation fails. May attempt to use
> + brk. */
> +void *_dl_early_allocate (size_t size) attribute_hidden;
> +
> /* Initialize the DSO sort algorithm to use. */
> #if !HAVE_TUNABLES
> static inline void
> diff --git a/sysdeps/unix/sysv/linux/dl-early_allocate.c b/sysdeps/unix/sysv/linux/dl-early_allocate.c
> new file mode 100644
> index 0000000000..52c538e85a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/dl-early_allocate.c
> @@ -0,0 +1,82 @@
> +/* Early memory allocation for the dynamic loader. Generic version.
> + Copyright (C) 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/>. */
> +
> +/* Mark symbols hidden in static PIE for early self relocation to work. */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
> +#include <startup.h>
> +
> +#include <ldsodefs.h>
> +#include <stddef.h>
> +#include <string.h>
> +#include <sysdep.h>
> +#include <unistd.h>
> +
> +#include <brk_call.h>
> +#include <mmap_call.h>
> +
> +/* Defined in brk.c. */
> +extern void *__curbrk;
> +
> +void *
> +_dl_early_allocate (size_t size)
> +{
> + void *result;
> +
> + if (__curbrk != NULL)
> + /* If the break has been initialized, brk must have run before,
> + so just call it once more. */
> + {
> + result = __sbrk (size);
> + if (result == (void *) -1)
> + result = NULL;
> + }
> + else
> + {
> + /* If brk has not been invoked, there is no need to update
> + __curbrk. The first call to brk will take care of that. */
> + void *previous = __brk_call (0);
> + result = __brk_call (previous + size);
> + if (result == previous)
> + result = NULL;
> + else
> + result = previous;
> + }
Why do you need to avoid update __curbrk here? Otherwise it seems that a
__sbrk() should be suffice here.
Rest looks ok.
> +
> + /* If brk fails, fall back to mmap. This can happen due to
> + unfortunate ASLR layout decisions and kernel bugs, particularly
> + for static PIE. */
> + if (result == NULL)
> + {
> + long int ret;
> + int prot = PROT_READ | PROT_WRITE;
> + int flags = MAP_PRIVATE | MAP_ANONYMOUS;
> +#ifdef __NR_mmap2
> + ret = MMAP_CALL_INTERNAL (mmap2, 0, size, prot, flags, -1, 0);
> +#else
> + ret = MMAP_CALL_INTERNAL (mmap, 0, size, prot, flags, -1, 0);
> +#endif
> + if (INTERNAL_SYSCALL_ERROR_P (ret))
> + result = NULL;
> + else
> + result = (void *) ret;
> + }
> +
> + return result;
> +}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/2] Linux: Introduce __brk_call for invoking the brk system call
2022-05-09 18:18 [PATCH v5 1/2] Linux: Introduce __brk_call for invoking the brk system call Florian Weimer
2022-05-09 18:18 ` [PATCH v5 2/2] csu: Implement and use _dl_early_allocate during static startup Florian Weimer
2022-05-12 13:01 ` [PATCH v5 1/2] Linux: Introduce __brk_call for invoking the brk system call Adhemerval Zanella
@ 2022-05-12 13:25 ` Andreas Schwab
2 siblings, 0 replies; 7+ messages in thread
From: Andreas Schwab @ 2022-05-12 13:25 UTC (permalink / raw)
To: Florian Weimer via Libc-alpha; +Cc: Florian Weimer
On Mai 09 2022, Florian Weimer via Libc-alpha wrote:
> Alpha and sparc can now use the generic implementation.
Ok.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] csu: Implement and use _dl_early_allocate during static startup
2022-05-12 13:15 ` Adhemerval Zanella
@ 2022-05-12 18:37 ` Florian Weimer
2022-05-16 13:04 ` Adhemerval Zanella
0 siblings, 1 reply; 7+ messages in thread
From: Florian Weimer @ 2022-05-12 18:37 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
* Adhemerval Zanella:
>> +void *
>> +_dl_early_allocate (size_t size)
>> +{
>> + void *result;
>> +
>> + if (__curbrk != NULL)
>> + /* If the break has been initialized, brk must have run before,
>> + so just call it once more. */
>> + {
>> + result = __sbrk (size);
>> + if (result == (void *) -1)
>> + result = NULL;
>> + }
>> + else
>> + {
>> + /* If brk has not been invoked, there is no need to update
>> + __curbrk. The first call to brk will take care of that. */
>> + void *previous = __brk_call (0);
>> + result = __brk_call (previous + size);
>> + if (result == previous)
>> + result = NULL;
>> + else
>> + result = previous;
>> + }
>
> Why do you need to avoid update __curbrk here? Otherwise it seems that a
> __sbrk() should be suffice here.
A subsequent call to _dl_early_allocate would then take the first (sbrk)
branch, which may or may not be correct, depending on whether the TCB
has been initialized at that point or not.
Thanks,
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] csu: Implement and use _dl_early_allocate during static startup
2022-05-12 18:37 ` Florian Weimer
@ 2022-05-16 13:04 ` Adhemerval Zanella
0 siblings, 0 replies; 7+ messages in thread
From: Adhemerval Zanella @ 2022-05-16 13:04 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 12/05/2022 15:37, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>>> +void *
>>> +_dl_early_allocate (size_t size)
>>> +{
>>> + void *result;
>>> +
>>> + if (__curbrk != NULL)
>>> + /* If the break has been initialized, brk must have run before,
>>> + so just call it once more. */
>>> + {
>>> + result = __sbrk (size);
>>> + if (result == (void *) -1)
>>> + result = NULL;
>>> + }
>>> + else
>>> + {
>>> + /* If brk has not been invoked, there is no need to update
>>> + __curbrk. The first call to brk will take care of that. */
>>> + void *previous = __brk_call (0);
>>> + result = __brk_call (previous + size);
>>> + if (result == previous)
>>> + result = NULL;
>>> + else
>>> + result = previous;
>>> + }
>>
>> Why do you need to avoid update __curbrk here? Otherwise it seems that a
>> __sbrk() should be suffice here.
>
> A subsequent call to _dl_early_allocate would then take the first (sbrk)
> branch, which may or may not be correct, depending on whether the TCB
> has been initialized at that point or not.
OK, LGTM then. I think I might try later to consolidate the mmap
syscall, although not sure if it would be worth.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-05-16 13:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-09 18:18 [PATCH v5 1/2] Linux: Introduce __brk_call for invoking the brk system call Florian Weimer
2022-05-09 18:18 ` [PATCH v5 2/2] csu: Implement and use _dl_early_allocate during static startup Florian Weimer
2022-05-12 13:15 ` Adhemerval Zanella
2022-05-12 18:37 ` Florian Weimer
2022-05-16 13:04 ` Adhemerval Zanella
2022-05-12 13:01 ` [PATCH v5 1/2] Linux: Introduce __brk_call for invoking the brk system call Adhemerval Zanella
2022-05-12 13:25 ` Andreas Schwab
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).