From nobody Mon Oct 30 17:56:52 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4SK1Dx6BrLz4yDkV; Mon, 30 Oct 2023 17:57:05 +0000 (UTC) (envelope-from bjkfbsd@gmail.com) Received: from mail-vs1-xe2e.google.com (mail-vs1-xe2e.google.com [IPv6:2607:f8b0:4864:20::e2e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4SK1Dx4YF8z3ZG3; Mon, 30 Oct 2023 17:57:05 +0000 (UTC) (envelope-from bjkfbsd@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-vs1-xe2e.google.com with SMTP id ada2fe7eead31-457c4e4a392so1324487137.0; Mon, 30 Oct 2023 10:57:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1698688624; x=1699293424; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=vHKO8upAXUtqtPYOuBW0WsUIDBw/XgSsq4J7wxIRHHg=; b=WOflVQ6ZNtaH1odxlFVonb4RZXSm6pkBRspTbxlu/5AQkYcni/KI6XxhzScyLf6rEq Sc462T4mLPVSdsuWL3ofgqGS2IBEOPjCYIG5J4D5ONprQQTVizi02I81EuSsfRRA+l9r nDSTBtUzrptI5lWAxQo8pKkk2YY92GmhY3Z48Tq2ZL9vvxGtT9p0WeHG++Fkmpv9+mVY h+vRdvCEnxp+YudrkIarNckChqWirzTXVozB5ClpSF3+m7aOd75E4+Ix/XeLOmsk679U 7LWxGOi6jZTndXQ26XuMIRNqORbfq+qB+EJ4JKUH/wJItkWUPZQMhzAlT8N1sVKVOW45 iw+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698688624; x=1699293424; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=vHKO8upAXUtqtPYOuBW0WsUIDBw/XgSsq4J7wxIRHHg=; b=E3WIJzFkxfHRiBZuZxRohjFZQ1Uu7ZQh67onhc6sEL2gOEoZYh30jACXjZuOcQjAku ustTOJw7Sv+KdBN3JtflCas8tWqpBV5haDtqwlUx8YLYXmtuEMdhFkvZQvYiR4c4B7on p5mZRcpk56nepYssGoSd+lKirRDjGmaRksfsqt7MJ9OIIyJeSoMvwvNR6op25gDLEkQS I35t5dOEcRazZYeIFkKDpXHRsAlFJMMWPYHP+K3Xm7+rIhjRCpob37xhkMxaoQflTj2O G0/IcZnrw57GcUW/4BNhiUlD8LSh+TK5w0eUTdbrFPrJLzhM1paD9Be/F01OxgrHxUMk Zqpg== X-Gm-Message-State: AOJu0YwxorjS71YOBX83SlY/LltDiLF26Go3KT5vHFkIQ8JfgY7g72QX 1p613VHp4QCkT42v6O09UTEKCjP4bkf/Y3RGAS4uwY/1 X-Google-Smtp-Source: AGHT+IEwUDOxkRgu1Oa6zZS6KYHFKmRoHoUYukw99oL7bzwLKwgIcP1GFot2lfyJk6Sc9aQjP1+f4Ox4Bac6MGskcgI= X-Received: by 2002:a67:c00b:0:b0:457:e3bb:867 with SMTP id v11-20020a67c00b000000b00457e3bb0867mr6835190vsi.6.1698688623729; Mon, 30 Oct 2023 10:57:03 -0700 (PDT) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 References: <202310301742.39UHgSPp011580@gitrepo.freebsd.org> In-Reply-To: <202310301742.39UHgSPp011580@gitrepo.freebsd.org> From: Benjamin Kaduk Date: Mon, 30 Oct 2023 10:56:52 -0700 Message-ID: Subject: Re: git: 95335dd3c19e - main - rtld: introduce STATIC_TLS_EXTRA To: "Stephen J. Kiernan" Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="0000000000000065c70608f2c495" X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] X-Rspamd-Queue-Id: 4SK1Dx4YF8z3ZG3 --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 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 > AuthorDate: 2023-10-29 21:13:10 +0000 > Commit: Stephen J. Kiernan > 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
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=3D95335dd3c19e0ade161bb4dc8462fc3d045ce4f8
commit 95335dd3c19e0ade161bb4dc8462fc3d045ce4f8
Author:=C2=A0 =C2=A0 =C2=A0Stephen J. Kiernan <stevek@FreeBSD.org> AuthorDate: 2023-10-29 21:13:10 +0000
Commit:=C2=A0 =C2=A0 =C2=A0Stephen J. Kiernan <stevek@FreeBSD.org> CommitDate: 2023-10-30 17:42:05 +0000

=C2=A0 =C2=A0 rtld: introduce STATIC_TLS_EXTRA

=C2=A0 =C2=A0 The new STATIC_TLS_EXTRA variable provides a means for applic= ations
=C2=A0 =C2=A0 to increases the size of the extra static TLS space allocated= by
=C2=A0 =C2=A0 rtld beyond the default of '128'. This extra static T= LS space is used
=C2=A0 =C2=A0 for objects loaded with dlopen.

=C2=A0 =C2=A0 The value specified in the variable must be no less than the = default
=C2=A0 =C2=A0 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 prev= ent chance of this being used in attacks and to prevent integer overflow.

Perhaps something large but not incredibly large, l= ike 1M?
=C2=A0
=C2=A0 =C2=A0 If an invalid value is specified, rtld will ignore it and jus= t use
=C2=A0 =C2=A0 the default value.

=C2=A0 =C2=A0 The rtld(1) man page is updated to document this new option.<= br>
=C2=A0 =C2=A0 Obtained from:=C2=A0 Juniper Networks, Inc.
=C2=A0 =C2=A0 Differential Revision:=C2=A0 https://reviews.freebsd= .org/D42025
---
=C2=A0libexec/rtld-elf/aarch64/reloc.c=C2=A0 =C2=A0|=C2=A0 2 +-
=C2=A0libexec/rtld-elf/amd64/reloc.c=C2=A0 =C2=A0 =C2=A0|=C2=A0 2 +-
=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 +-
=C2=A0libexec/rtld-elf/powerpc/reloc.c=C2=A0 =C2=A0|=C2=A0 3 ++-
=C2=A0libexec/rtld-elf/powerpc64/reloc.c |=C2=A0 3 ++-
=C2=A0libexec/rtld-elf/riscv/reloc.c=C2=A0 =C2=A0 =C2=A0|=C2=A0 2 +-
=C2=A0libexec/rtld-elf/rtld.1=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2= =A0 8 +++++++-
=C2=A0libexec/rtld-elf/rtld.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 21= +++++++++++++++++----
=C2=A0libexec/rtld-elf/rtld.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2= =A0 1 +
=C2=A010 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/libexec/rtld-elf/aarch64/reloc.c b/libexec/rtld-elf/aarch64/re= loc.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)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 * use.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 */
=C2=A0 =C2=A0 =C2=A0 =C2=A0 tls_static_space =3D tls_last_offset + tls_last= _size +
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0RTLD_STATIC_TLS_EXTRA;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ld_static_tls_extra;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 _tcb_set(allocate_tls(objs, NULL, TLS_TCB_SIZE,= TLS_TCB_ALIGN));
=C2=A0}
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)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* offset allocated so far and adding a bi= t for dynamic
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* modules to use.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
-=C2=A0 =C2=A0 =C2=A0 =C2=A0tls_static_space =3D tls_last_offset + RTLD_STA= TIC_TLS_EXTRA;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0tls_static_space =3D tls_last_offset + ld_stati= c_tls_extra;


This calculation could overflow, as I = see it (likewise for the other archs).

-Ben
<= div>
=C2=A0
--0000000000000065c70608f2c495--