Re: git: 95335dd3c19e - main - rtld: introduce STATIC_TLS_EXTRA

From: Benjamin Kaduk <bjkfbsd_at_gmail.com>
Date: Mon, 30 Oct 2023 17:56:52 UTC
On Mon, Oct 30, 2023 at 10:42 AM Stephen J. Kiernan <stevek@freebsd.org>
wrote:

> The branch main has been updated by stevek:
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=95335dd3c19e0ade161bb4dc8462fc3d045ce4f8
>
> commit 95335dd3c19e0ade161bb4dc8462fc3d045ce4f8
> Author:     Stephen J. Kiernan <stevek@FreeBSD.org>
> AuthorDate: 2023-10-29 21:13:10 +0000
> Commit:     Stephen J. Kiernan <stevek@FreeBSD.org>
> CommitDate: 2023-10-30 17:42:05 +0000
>
>     rtld: introduce STATIC_TLS_EXTRA
>
>     The new STATIC_TLS_EXTRA variable provides a means for applications
>     to increases the size of the extra static TLS space allocated by
>     rtld beyond the default of '128'. This extra static TLS space is used
>     for objects loaded with dlopen.
>
>     The value specified in the variable must be no less than the default
>     value and no greater than the maximum allowed value for size_t type.
>
>

I think that we want to have a maximum allowed value that is smaller than
SIZE_T_MAX, both to prevent chance of this being used in attacks and to
prevent integer overflow.

Perhaps something large but not incredibly large, like 1M?


>     If an invalid value is specified, rtld will ignore it and just use
>     the default value.
>
>     The rtld(1) man page is updated to document this new option.
>
>     Obtained from:  Juniper Networks, Inc.
>     Differential Revision:  https://reviews.freebsd.org/D42025
> ---
>  libexec/rtld-elf/aarch64/reloc.c   |  2 +-
>  libexec/rtld-elf/amd64/reloc.c     |  2 +-
>  libexec/rtld-elf/arm/reloc.c       |  3 ++-
>  libexec/rtld-elf/i386/reloc.c      |  2 +-
>  libexec/rtld-elf/powerpc/reloc.c   |  3 ++-
>  libexec/rtld-elf/powerpc64/reloc.c |  3 ++-
>  libexec/rtld-elf/riscv/reloc.c     |  2 +-
>  libexec/rtld-elf/rtld.1            |  8 +++++++-
>  libexec/rtld-elf/rtld.c            | 21 +++++++++++++++++----
>  libexec/rtld-elf/rtld.h            |  1 +
>  10 files changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/libexec/rtld-elf/aarch64/reloc.c
> b/libexec/rtld-elf/aarch64/reloc.c
> index c8f09d797215..907377f2619a 100644
> --- a/libexec/rtld-elf/aarch64/reloc.c
> +++ b/libexec/rtld-elf/aarch64/reloc.c
> @@ -521,7 +521,7 @@ allocate_initial_tls(Obj_Entry *objs)
>         * use.
>         */
>         tls_static_space = tls_last_offset + tls_last_size +
> -           RTLD_STATIC_TLS_EXTRA;
> +           ld_static_tls_extra;
>
>         _tcb_set(allocate_tls(objs, NULL, TLS_TCB_SIZE, TLS_TCB_ALIGN));
>  }
> diff --git a/libexec/rtld-elf/amd64/reloc.c
> b/libexec/rtld-elf/amd64/reloc.c
> index ce74c54cc5c3..9c5887def356 100644
> --- a/libexec/rtld-elf/amd64/reloc.c
> +++ b/libexec/rtld-elf/amd64/reloc.c
> @@ -527,7 +527,7 @@ allocate_initial_tls(Obj_Entry *objs)
>          * offset allocated so far and adding a bit for dynamic
>          * modules to use.
>          */
> -       tls_static_space = tls_last_offset + RTLD_STATIC_TLS_EXTRA;
> +       tls_static_space = tls_last_offset + ld_static_tls_extra;
>
>
This calculation could overflow, as I see it (likewise for the other archs).

-Ben