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

On Mon, Oct 30, 2023 at 10:56:52AM -0700, Benjamin Kaduk wrote:
> 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?
What attacks?

> 
> 
> >     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).
So do not do it.