Date: Mon, 30 Oct 2023 10:56:52 -0700 From: Benjamin Kaduk <bjkfbsd@gmail.com> To: "Stephen J. Kiernan" <stevek@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 95335dd3c19e - main - rtld: introduce STATIC_TLS_EXTRA Message-ID: <CAJ5_RoCx1zbpU4U1ZE2N%2BQCM6oVA4wwrM=dyP9%2Bqe2FqvHpv4w@mail.gmail.com> In-Reply-To: <202310301742.39UHgSPp011580@gitrepo.freebsd.org> References: <202310301742.39UHgSPp011580@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--0000000000000065c70608f2c495 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Oct 30, 2023 at 10:42=E2=80=AFAM Stephen J. Kiernan <stevek@freebsd= .org> wrote: > The branch main has been updated by stevek: > > URL: > https://cgit.FreeBSD.org/src/commit/?id=3D95335dd3c19e0ade161bb4dc8462fc3= d045ce4f8 > > 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 =3D 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 =3D tls_last_offset + RTLD_STATIC_TLS_EXTRA; > + tls_static_space =3D tls_last_offset + ld_static_tls_extra; > > This calculation could overflow, as I see it (likewise for the other archs)= . -Ben --0000000000000065c70608f2c495 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr">On Mon, Oct 30, 2023 at 10:42=E2=80=AFAM = Stephen J. Kiernan <<a href=3D"mailto:stevek@freebsd.org">stevek@freebsd= .org</a>> wrote:<br></div><div class=3D"gmail_quote"><blockquote class= =3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rg= b(204,204,204);padding-left:1ex">The branch main has been updated by stevek= :<br> <br> URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D95335dd3c19e0ade1= 61bb4dc8462fc3d045ce4f8" rel=3D"noreferrer" target=3D"_blank">https://cgit.= FreeBSD.org/src/commit/?id=3D95335dd3c19e0ade161bb4dc8462fc3d045ce4f8</a><b= r> <br> commit 95335dd3c19e0ade161bb4dc8462fc3d045ce4f8<br> Author:=C2=A0 =C2=A0 =C2=A0Stephen J. Kiernan <stevek@FreeBSD.org><br= > AuthorDate: 2023-10-29 21:13:10 +0000<br> Commit:=C2=A0 =C2=A0 =C2=A0Stephen J. Kiernan <stevek@FreeBSD.org><br= > CommitDate: 2023-10-30 17:42:05 +0000<br> <br> =C2=A0 =C2=A0 rtld: introduce STATIC_TLS_EXTRA<br> <br> =C2=A0 =C2=A0 The new STATIC_TLS_EXTRA variable provides a means for applic= ations<br> =C2=A0 =C2=A0 to increases the size of the extra static TLS space allocated= by<br> =C2=A0 =C2=A0 rtld beyond the default of '128'. This extra static T= LS space is used<br> =C2=A0 =C2=A0 for objects loaded with dlopen.<br> <br> =C2=A0 =C2=A0 The value specified in the variable must be no less than the = default<br> =C2=A0 =C2=A0 value and no greater than the maximum allowed value for size_= t type.<br> <br></blockquote><div><br></div><div><br></div><div>I think that we want to= have a maximum allowed value that is smaller than SIZE_T_MAX, both to prev= ent chance of this being used in attacks and to prevent integer overflow.</= div><div><br></div><div>Perhaps something large but not incredibly large, l= ike 1M?</div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"ma= rgin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:= 1ex"> =C2=A0 =C2=A0 If an invalid value is specified, rtld will ignore it and jus= t use<br> =C2=A0 =C2=A0 the default value.<br> <br> =C2=A0 =C2=A0 The rtld(1) man page is updated to document this new option.<= br> <br> =C2=A0 =C2=A0 Obtained from:=C2=A0 Juniper Networks, Inc.<br> =C2=A0 =C2=A0 Differential Revision:=C2=A0 <a href=3D"https://reviews.freeb= sd.org/D42025" rel=3D"noreferrer" target=3D"_blank">https://reviews.freebsd= .org/D42025</a><br> ---<br> =C2=A0libexec/rtld-elf/aarch64/reloc.c=C2=A0 =C2=A0|=C2=A0 2 +-<br> =C2=A0libexec/rtld-elf/amd64/reloc.c=C2=A0 =C2=A0 =C2=A0|=C2=A0 2 +-<br> =C2=A0libexec/rtld-elf/arm/reloc.c=C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 3 ++-<= br> =C2=A0libexec/rtld-elf/i386/reloc.c=C2=A0 =C2=A0 =C2=A0 |=C2=A0 2 +-<br> =C2=A0libexec/rtld-elf/powerpc/reloc.c=C2=A0 =C2=A0|=C2=A0 3 ++-<br> =C2=A0libexec/rtld-elf/powerpc64/reloc.c |=C2=A0 3 ++-<br> =C2=A0libexec/rtld-elf/riscv/reloc.c=C2=A0 =C2=A0 =C2=A0|=C2=A0 2 +-<br> =C2=A0libexec/rtld-elf/rtld.1=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2= =A0 8 +++++++-<br> =C2=A0libexec/rtld-elf/rtld.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 21= +++++++++++++++++----<br> =C2=A0libexec/rtld-elf/rtld.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2= =A0 1 +<br> =C2=A010 files changed, 35 insertions(+), 12 deletions(-)<br> <br> diff --git a/libexec/rtld-elf/aarch64/reloc.c b/libexec/rtld-elf/aarch64/re= loc.c<br> index c8f09d797215..907377f2619a 100644<br> --- a/libexec/rtld-elf/aarch64/reloc.c<br> +++ b/libexec/rtld-elf/aarch64/reloc.c<br> @@ -521,7 +521,7 @@ allocate_initial_tls(Obj_Entry *objs)<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 * use.<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 */<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 tls_static_space =3D tls_last_offset + tls_last= _size +<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0RTLD_STATIC_TLS_EXTRA;<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ld_static_tls_extra;<br> <br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 _tcb_set(allocate_tls(objs, NULL, TLS_TCB_SIZE,= TLS_TCB_ALIGN));<br> =C2=A0}<br> diff --git a/libexec/rtld-elf/amd64/reloc.c b/libexec/rtld-elf/amd64/reloc.= c<br> index ce74c54cc5c3..9c5887def356 100644<br> --- a/libexec/rtld-elf/amd64/reloc.c<br> +++ b/libexec/rtld-elf/amd64/reloc.c<br> @@ -527,7 +527,7 @@ allocate_initial_tls(Obj_Entry *objs)<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* offset allocated so far and adding a bi= t for dynamic<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* modules to use.<br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/<br> -=C2=A0 =C2=A0 =C2=A0 =C2=A0tls_static_space =3D tls_last_offset + RTLD_STA= TIC_TLS_EXTRA;<br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0tls_static_space =3D tls_last_offset + ld_stati= c_tls_extra;<br> <br></blockquote><div><br></div><div>This calculation could overflow, as I = see it (likewise for the other archs).</div><div><br></div><div>-Ben</div><= div><br></div><div>=C2=A0</div></div></div> --0000000000000065c70608f2c495--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ5_RoCx1zbpU4U1ZE2N%2BQCM6oVA4wwrM=dyP9%2Bqe2FqvHpv4w>