On 04/10/2020 10:09, Topi Miettinen via Libc-alpha wrote:
> Let's use mmap() for TCB, which makes the location of TCB random
> instead of always staying predictably next to data segment. Also
> improve the logic so that allocation of TCB can be assumed to fail
> insted of segfaulting.
>
> RFC: make independent of Linux.
The interface does provide the required independence, the issue is
adapt Hurd code to add a mmap interface that does no set errno.
>
> Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
We do not use SCO, but rather Copyright assignment.
> ---
> csu/libc-tls.c | 20 ++++++++++++++------
> sysdeps/unix/sysv/linux/mmap64.c | 19 +++++++++++++++++++
> sysdeps/unix/sysv/linux/mmap_internal.h | 3 +++
> 3 files changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/csu/libc-tls.c b/csu/libc-tls.c
> index 06e76bd395..59700c3a95 100644
> --- a/csu/libc-tls.c
> +++ b/csu/libc-tls.c
> @@ -24,6 +24,9 @@
> #include <stdio.h>
> #include <sys/param.h>
> #include <array_length.h>
> +#include <sys/mman.h>
> +#include <sysdep.h>
> +#include <mmap_internal.h>
>
> #ifdef SHARED
> #error makefile bug, this file is for static only
> @@ -134,25 +137,30 @@ __libc_setup_tls (void)
>
> /* We have to set up the TCB block which also (possibly) contains
> 'errno'. Therefore we avoid 'malloc' which might touch 'errno'.
> - Instead we use 'sbrk' which would only uses 'errno' if it fails.
> - In this case we are right away out of memory and the user gets
> - what she/he deserves. */
> + Instead we use 'internal_mmap' which does not use 'errno'. */
I think you mean '__mmap_internal' here, however I think it should be rename
__mmap64_internal since it does use off64_t (more below).
> +
> + int error = 0;
> +
> #if TLS_TCB_AT_TP
> /* Align the TCB offset to the maximum alignment, as
> _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 = __mmap_internal (NULL, tcb_offset + TLS_INIT_TCB_SIZE + max_align,
> + PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0, &error);
Line too long.
> #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 = __mmap_internal (NULL, tcb_offset + memsz + max_align
> + + TLS_PRE_TCB_SIZE + GLRO(dl_tls_static_surplus),
> + PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0, &error);
> tlsblock += TLS_PRE_TCB_SIZE;
> #else
> /* In case a model with a different layout for the TCB and DTV
> is defined add another #elif here and in the following #ifs. */
> # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
> #endif
> + if (error)
> + _startup_fatal ("Cannot allocate TCB");
No implicit checks. Also, since you are not really using its value I think
you just use a an automatic here to avoid its definition and check the
tlsblock against MAP_FAILED (it would need to move the tlsblock
adjustment for !TLS_DTV_AT_TP to after the check). Something like:
diff --git a/csu/libc-tls.c b/csu/libc-tls.c
index c3589f0a7d..69b5bd1301 100644
--- a/csu/libc-tls.c
+++ b/csu/libc-tls.c
@@ -135,24 +135,32 @@ __libc_setup_tls (void)
/* We have to set up the TCB block which also (possibly) contains
'errno'. Therefore we avoid 'malloc' which might touch 'errno'.
- Instead we use 'sbrk' which would only uses 'errno' if it fails.
- In this case we are right away out of memory and the user gets
- what she/he deserves. */
+ Instead we use 'mmap64_internal' which does not use 'errno'. */
#if TLS_TCB_AT_TP
/* Align the TCB offset to the maximum alignment, as
_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 = __mmap64_internal (NULL, tcb_offset + TLS_INIT_TCB_SIZE
+ + max_align,
+ PROT_READ | PROT_WRITE, MAP_ANONYMOUS
+ | MAP_PRIVATE, -1, 0, &(int){0});
#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 += TLS_PRE_TCB_SIZE;
+ tlsblock = __mmap64_internal (NULL, tcb_offset + memsz + max_align
+ + TLS_PRE_TCB_SIZE
+ + GLRO(dl_tls_static_surplus),
+ PROT_READ | PROT_WRITE, MAP_ANONYMOUS
+ | MAP_PRIVATE, -1, 0, &(int){0});
#else
/* In case a model with a different layout for the TCB and DTV
is defined add another #elif here and in the following #ifs. */
# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
+#endif
+ if (tlsblock == MAP_FAILED)
+ _startup_fatal ("Cannot allocate TCB");
+#if TLS_DTV_AT_TP
+ tlsblock += TLS_PRE_TCB_SIZE;
#endif
/* Align the TLS block. */
>
> /* Align the TLS block. */
> tlsblock = (void *) (((uintptr_t) tlsblock + max_align - 1)
> diff --git a/sysdeps/unix/sysv/linux/mmap64.c b/sysdeps/unix/sysv/linux/mmap64.c
> index 8074deb466..11f7c3f99b 100644
> --- a/sysdeps/unix/sysv/linux/mmap64.c
> +++ b/sysdeps/unix/sysv/linux/mmap64.c
> @@ -67,3 +67,22 @@ weak_alias (__mmap64, mmap)
> weak_alias (__mmap64, __mmap)
> libc_hidden_def (__mmap)
> #endif
> +
> +void *
> +__mmap_internal (void *addr, size_t len, int prot, int flags, int fd, off64_t offset, int *error)
> +{
> + unsigned long int ret;
> +#ifdef __NR_mmap2
> + ret = INTERNAL_SYSCALL_CALL (mmap2, addr, len, prot, flags, fd,
> + (off_t) (offset / MMAP2_PAGE_UNIT));
> +#else
> + ret = INTERNAL_SYSCALL_CALL (mmap, addr, len, prot, flags, fd, offset);
> +#endif
> + if (INTERNAL_SYSCALL_ERROR_P(ret))
> + {
> + *error = ret;
> + return MAP_FAILED;
> + }
> +
> + return (void *) ret;
> +}
The MMAP_CALL macro is used to allow an architecture to redefine if
required, and s390 does. And I think it would be better to refactor the code
to avoid a duplication of the syscall logic:
diff --git a/sysdeps/unix/sysv/linux/mmap.c b/sysdeps/unix/sysv/linux/mmap.c
index 22f276bb14..071c7462e0 100644
--- a/sysdeps/unix/sysv/linux/mmap.c
+++ b/sysdeps/unix/sysv/linux/mmap.c
@@ -39,12 +39,18 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
return (void *) INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL);
#ifdef __NR_mmap2
- return (void *) MMAP_CALL (mmap2, addr, len, prot, flags, fd,
- offset / (uint32_t) MMAP2_PAGE_UNIT);
+ long int r = MMAP_CALL (mmap2, addr, len, prot, flags, fd,
+ offset / (uint32_t) MMAP2_PAGE_UNIT);
#else
- return (void *) MMAP_CALL (mmap, addr, len, prot, flags, fd,
- MMAP_ADJUST_OFFSET (offset));
+ long int r = MMAP_CALL (mmap, addr, len, prot, flags, fd,
+ MMAP_ADJUST_OFFSET (offset));
#endif
+ if (INTERNAL_SYSCALL_ERROR_P (r))
+ {
+ __set_errno (INTERNAL_SYSCALL_ERRNO (r));
+ return MAP_FAILED;
+ }
+ return (void*) r;
}
weak_alias (__mmap, mmap)
libc_hidden_def (__mmap)
diff --git a/sysdeps/unix/sysv/linux/mmap64.c b/sysdeps/unix/sysv/linux/mmap64.c
index 8074deb466..8487096726 100644
--- a/sysdeps/unix/sysv/linux/mmap64.c
+++ b/sysdeps/unix/sysv/linux/mmap64.c
@@ -44,20 +44,33 @@
#endif
void *
-__mmap64 (void *addr, size_t len, int prot, int flags, int fd, off64_t offset)
+__mmap64_internal (void *addr, size_t len, int prot, int flags, int fd,
+ off64_t offset, int *err)
{
MMAP_CHECK_PAGE_UNIT ();
if (offset & MMAP_OFF_MASK)
- return (void *) INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL);
+ return (void *) -EINVAL;
MMAP_PREPARE (addr, len, prot, flags, fd, offset);
#ifdef __NR_mmap2
- return (void *) MMAP_CALL (mmap2, addr, len, prot, flags, fd,
- (off_t) (offset / MMAP2_PAGE_UNIT));
+ long int r = MMAP_CALL (mmap2, addr, len, prot, flags, fd,
+ (off_t) (offset / MMAP2_PAGE_UNIT));
#else
- return (void *) MMAP_CALL (mmap, addr, len, prot, flags, fd, offset);
+ long int r = MMAP_CALL (mmap, addr, len, prot, flags, fd, offset);
#endif
+ if (INTERNAL_SYSCALL_ERROR_P (r))
+ {
+ *err = INTERNAL_SYSCALL_ERRNO (r);
+ return MAP_FAILED;
+ }
+ return (void *) r;
+}
+
+void *
+__mmap64 (void *addr, size_t len, int prot, int flags, int fd, off64_t offset)
+{
+ return __mmap64_internal (addr, len, prot, flags, fd, offset, &errno);
}
weak_alias (__mmap64, mmap64)
libc_hidden_def (__mmap64)
diff --git a/sysdeps/unix/sysv/linux/mmap_internal.h b/sysdeps/unix/sysv/linux/mmap_internal.h
index d53f0c642a..5386b5eb63 100644
--- a/sysdeps/unix/sysv/linux/mmap_internal.h
+++ b/sysdeps/unix/sysv/linux/mmap_internal.h
@@ -43,7 +43,7 @@ static uint64_t page_unit;
/* An architecture may override this. */
#ifndef MMAP_CALL
# define MMAP_CALL(__nr, __addr, __len, __prot, __flags, __fd, __offset) \
- INLINE_SYSCALL_CALL (__nr, __addr, __len, __prot, __flags, __fd, __offset)
+ INTERNAL_SYSCALL_CALL (__nr, __addr, __len, __prot, __flags, __fd, __offset)
#endif
#endif /* MMAP_INTERNAL_LINUX_H */
diff --git a/sysdeps/unix/sysv/linux/s390/mmap_internal.h b/sysdeps/unix/sysv/linux/s390/mmap_internal.h
index 2884f2844b..4d5e0291c3 100644
--- a/sysdeps/unix/sysv/linux/s390/mmap_internal.h
+++ b/sysdeps/unix/sysv/linux/s390/mmap_internal.h
@@ -24,7 +24,7 @@
long int __args[6] = { (long int) (__addr), (long int) (__len), \
(long int) (__prot), (long int) (__flags), \
(long int) (__fd), (long int) (__offset) }; \
- INLINE_SYSCALL_CALL (__nr, __args); \
+ INTERNAL_SYSCALL_CALL (__nr, __args); \
})
#include_next <mmap_internal.h>
> diff --git a/sysdeps/unix/sysv/linux/mmap_internal.h b/sysdeps/unix/sysv/linux/mmap_internal.h
> index d53f0c642a..00fc14902e 100644
> --- a/sysdeps/unix/sysv/linux/mmap_internal.h
> +++ b/sysdeps/unix/sysv/linux/mmap_internal.h
> @@ -46,4 +46,7 @@ static uint64_t page_unit;
> INLINE_SYSCALL_CALL (__nr, __addr, __len, __prot, __flags, __fd, __offset)
> #endif
>
> +/* Internal version of mmap() which doesn't attempt to access errno */
> +void *__mmap_internal (void *addr, size_t len, int prot, int flags, int fd, off64_t offset, int *error);
> +
> #endif /* MMAP_INTERNAL_LINUX_H */
>
Move the internal definition to include/sys/mman.h instead, the
mmap_internal.h is a Linux one header meant to be used to
implement mmap itself.
@@ -24,6 +24,9 @@
#include <stdio.h>
#include <sys/param.h>
#include <array_length.h>
+#include <sys/mman.h>
+#include <sysdep.h>
+#include <mmap_internal.h>
#ifdef SHARED
#error makefile bug, this file is for static only
@@ -134,25 +137,30 @@ __libc_setup_tls (void)
/* We have to set up the TCB block which also (possibly) contains
'errno'. Therefore we avoid 'malloc' which might touch 'errno'.
- Instead we use 'sbrk' which would only uses 'errno' if it fails.
- In this case we are right away out of memory and the user gets
- what she/he deserves. */
+ Instead we use 'internal_mmap' which does not use 'errno'. */
+
+ int error = 0;
+
#if TLS_TCB_AT_TP
/* Align the TCB offset to the maximum alignment, as
_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 = __mmap_internal (NULL, tcb_offset + TLS_INIT_TCB_SIZE + max_align,
+ PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0, &error);
#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 = __mmap_internal (NULL, tcb_offset + memsz + max_align
+ + TLS_PRE_TCB_SIZE + GLRO(dl_tls_static_surplus),
+ PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0, &error);
tlsblock += TLS_PRE_TCB_SIZE;
#else
/* In case a model with a different layout for the TCB and DTV
is defined add another #elif here and in the following #ifs. */
# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
#endif
+ if (error)
+ _startup_fatal ("Cannot allocate TCB");
/* Align the TLS block. */
tlsblock = (void *) (((uintptr_t) tlsblock + max_align - 1)
@@ -67,3 +67,22 @@ weak_alias (__mmap64, mmap)
weak_alias (__mmap64, __mmap)
libc_hidden_def (__mmap)
#endif
+
+void *
+__mmap_internal (void *addr, size_t len, int prot, int flags, int fd, off64_t offset, int *error)
+{
+ unsigned long int ret;
+#ifdef __NR_mmap2
+ ret = INTERNAL_SYSCALL_CALL (mmap2, addr, len, prot, flags, fd,
+ (off_t) (offset / MMAP2_PAGE_UNIT));
+#else
+ ret = INTERNAL_SYSCALL_CALL (mmap, addr, len, prot, flags, fd, offset);
+#endif
+ if (INTERNAL_SYSCALL_ERROR_P(ret))
+ {
+ *error = ret;
+ return MAP_FAILED;
+ }
+
+ return (void *) ret;
+}
@@ -46,4 +46,7 @@ static uint64_t page_unit;
INLINE_SYSCALL_CALL (__nr, __addr, __len, __prot, __flags, __fd, __offset)
#endif
+/* Internal version of mmap() which doesn't attempt to access errno */
+void *__mmap_internal (void *addr, size_t len, int prot, int flags, int fd, off64_t offset, int *error);
+
#endif /* MMAP_INTERNAL_LINUX_H */