public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
To: Szabolcs Nagy <szabolcs.nagy@arm.com>, libc-alpha@sourceware.org
Subject: Re: [PATCH] elf: Add TLS modid reuse test for bug 29039
Date: Tue, 19 Dec 2023 16:28:33 -0300	[thread overview]
Message-ID: <8b33f157-5ff0-4a52-91c3-87996c063b5d@linaro.org> (raw)
In-Reply-To: <20231130081119.1252882-1-szabolcs.nagy@arm.com>



On 30/11/23 05:11, Szabolcs Nagy wrote:
> This is a minimal regression test for bug 29039 which only affects
> targets with TLSDESC and a reproducer requires that
> 
> 1) Have modid gaps (closed modules) with old generation.
> 2) Update a DTV to a newer generation (needs a newer dlopen).
> 3) But do not update the closed gap entry in that DTV.
> 4) Reuse the modid gap for a new module (another dlopen).
> 5) Use dynamic TLSDESC in that new module with old generation (bug).
> 6) Access TLS via this TLSDESC and the now outdated DTV.
> 
> However step (3) in practice rarely happens: during DTV update the
> entries for closed modids are initialized to "unallocated" and then
> dynamic TLSDESC calls __tls_get_addr independently of its generation.
> The only exception to this is DTV setup at thread creation (gaps are
> initialized to NULL instead of unallocated) or DTV resize where the
> gap entries are outside the previous DTV array (again NULL instead
> of unallocated, and this requires loading > DTV_SURPLUS modules).
> 
> So the bug can only cause NULL (+ offset) dereference, not use after
> free. And the easiest way to get (3) is via thread creation.
> 
> Note that step (5) requires that the newly loaded module has larger
> TLS than the remaining optional static TLS. And for (6) there cannot
> be other TLS access or dlopen in the thread that updates the DTV.
> 
> Tested on aarch64-linux-gnu.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/Makefile          | 15 +++++++
>  elf/tst-tlsgap-mod0.c |  2 +
>  elf/tst-tlsgap-mod1.c |  2 +
>  elf/tst-tlsgap-mod2.c |  2 +
>  elf/tst-tlsgap.c      | 92 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 113 insertions(+)
>  create mode 100644 elf/tst-tlsgap-mod0.c
>  create mode 100644 elf/tst-tlsgap-mod1.c
>  create mode 100644 elf/tst-tlsgap-mod2.c
>  create mode 100644 elf/tst-tlsgap.c
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index afec7be084..22902c18f1 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -469,6 +469,7 @@ tests += \
>    tst-tls21 \
>    tst-tlsalign \
>    tst-tlsalign-extern \
> +  tst-tlsgap \
>    tst-unique1 \
>    tst-unique2 \
>    tst-unwind-ctor \
> @@ -896,6 +897,9 @@ modules-names += \
>    tst-tls20mod-bad \
>    tst-tls21mod \
>    tst-tlsalign-lib \
> +  tst-tlsgap-mod0 \
> +  tst-tlsgap-mod1 \
> +  tst-tlsgap-mod2 \
>    tst-tlsmod1 \
>    tst-tlsmod10 \
>    tst-tlsmod11 \

Ok.

> @@ -3030,3 +3034,14 @@ $(objpfx)tst-nodeps2-mod.so: $(common-objpfx)libc.so \
>  	$(LINK.o) -Wl,--no-as-needed -nostartfiles -nostdlib -shared -o $@ $^
>  $(objpfx)tst-nodeps2.out: \
>    $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.so
> +
> +$(objpfx)tst-tlsgap: $(shared-thread-library)
> +$(objpfx)tst-tlsgap.out: \
> +  $(objpfx)tst-tlsgap-mod0.so \
> +  $(objpfx)tst-tlsgap-mod1.so \
> +  $(objpfx)tst-tlsgap-mod2.so
> +ifeq (yes,$(have-mtls-dialect-gnu2))
> +CFLAGS-tst-tlsgap-mod0.c += -mtls-dialect=gnu2
> +CFLAGS-tst-tlsgap-mod1.c += -mtls-dialect=gnu2
> +CFLAGS-tst-tlsgap-mod2.c += -mtls-dialect=gnu2
> +endif

Ok.

> diff --git a/elf/tst-tlsgap-mod0.c b/elf/tst-tlsgap-mod0.c
> new file mode 100644
> index 0000000000..1478b0beac
> --- /dev/null
> +++ b/elf/tst-tlsgap-mod0.c
> @@ -0,0 +1,2 @@
> +int __thread tls0;
> +int *f0(void) { return &tls0; }
> diff --git a/elf/tst-tlsgap-mod1.c b/elf/tst-tlsgap-mod1.c
> new file mode 100644
> index 0000000000..b10fc3702c
> --- /dev/null
> +++ b/elf/tst-tlsgap-mod1.c
> @@ -0,0 +1,2 @@
> +int __thread tls1[100]; /* Size > glibc.rtld.optional_static_tls / 2.  */
> +int *f1(void) { return tls1; }
> diff --git a/elf/tst-tlsgap-mod2.c b/elf/tst-tlsgap-mod2.c
> new file mode 100644
> index 0000000000..166c27d7f3
> --- /dev/null
> +++ b/elf/tst-tlsgap-mod2.c
> @@ -0,0 +1,2 @@
> +int __thread tls2;
> +int *f2(void) { return &tls2; }
> diff --git a/elf/tst-tlsgap.c b/elf/tst-tlsgap.c

Ok.

> new file mode 100644
> index 0000000000..4932885076
> --- /dev/null
> +++ b/elf/tst-tlsgap.c
> @@ -0,0 +1,92 @@
> +/* TLS modid gap reuse regression test for bug 29039.
> +   Copyright (C) 2023 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <dlfcn.h>
> +#include <pthread.h>
> +#include <support/xdlfcn.h>
> +#include <support/xthread.h>
> +#include <support/check.h>
> +
> +static void *mod[3];
> +#define MOD(i) "tst-tlsgap-mod" #i ".so"
> +static const char *modname[3] = { MOD(0), MOD(1), MOD(2) };
> +#undef MOD
> +
> +static void
> +open_mod (int i)
> +{
> +  mod[i] = xdlopen (modname[i], RTLD_LAZY);
> +  printf ("open %s\n", modname[i]);
> +}
> +
> +static void
> +close_mod (int i)
> +{
> +  xdlclose (mod[i]);
> +  mod[i] = NULL;
> +  printf ("close %s\n", modname[i]);
> +}
> +
> +static void
> +access_mod (int i, const char *sym)
> +{
> +  int *(*f) (void) = xdlsym (mod[i], sym);
> +  int *p = f ();
> +  printf ("access %s: %s() = %p\n", modname[i], sym, p);
> +  TEST_VERIFY_EXIT (p != NULL);
> +  ++*p;
> +}
> +
> +static void *
> +start (void *arg)
> +{
> +  /* The DTV generation is at the last dlopen of mod0 and the
> +     entry for mod1 is NULL.  */
> +
> +  open_mod (1); /* Reuse modid of mod1. Uses dynamic TLS.  */
> +
> +  /* DTV is unchanged: dlopen only updates the DTV to the latest
> +     generation if static TLS is allocated for a loaded module.
> +
> +     With bug 29039, the TLSDESC relocation in mod1 uses the old
> +     dlclose generation of mod1 instead of the new dlopen one so
> +     DTV is not updated on TLS access.  */
> +
> +  access_mod (1, "f1");
> +
> +  return arg;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  open_mod (0);
> +  open_mod (1);
> +  open_mod (2);
> +  close_mod (0);
> +  close_mod (1); /* Create modid gap at mod1.  */
> +  open_mod (0); /* Reuse modid of mod0, bump generation count.  */
> +
> +  /* Create a thread where DTV of mod1 is NULL.  */
> +  pthread_t t = xpthread_create (NULL, start, NULL);
> +  xpthread_join (t);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>


Ok.

      reply	other threads:[~2023-12-19 19:28 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-30  8:11 Szabolcs Nagy
2023-12-19 19:28 ` Adhemerval Zanella Netto [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b33f157-5ff0-4a52-91c3-87996c063b5d@linaro.org \
    --to=adhemerval.zanella@linaro.org \
    --cc=libc-alpha@sourceware.org \
    --cc=szabolcs.nagy@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).