elf: Add tests for working RELRO protection

Message ID 875zl6n7p2.fsf@oldenburg2.str.redhat.com
State Changes Requested, archived
Headers

Commit Message

Florian Weimer Oct. 2, 2019, 9:44 p.m. UTC
  (I need to rework the RELRO handling to address the rseq initialization
issue.  Tested on i686-linux-gnu, x86_64-linux-gnu,
powerpc64le-linux-gnu, aarch64-linux-gnu, s390x-linux-gnu.  (The test
does not exercise partial RELRO support, so there's no issue on s390x.)
Build-tested on i686-gnu.  If approved, I will commit the support bits
separately, for easier backporting.)

2019-10-02  Florian Weimer  <fweimer@redhat.com>

	elf: Add tests for working RELRO protection.
	* elf/Makefile [$(build-shared)] (tests): Add tst-relro-shared.
	[$(have-fpie) && $(build-shared)] (tests, tests-pie): Add
	tst-relro-shared-pie.
	(tst-relro-shared, tst-relro-shared-pie): Link with -ldl.
	(LDFLAGS-tst-relro-shared, LDFLAGS-tst-relro-shared-pie): Link
	with -z relro.
	(tst-relro-shared-no-pie): Set to disable PIE for
	tst-relro-shared.
	(CFLAGS-tst-relro-shared-pie.c): Build with -fpie.
	* elf/tst-relro-shared.c: New file.
	* elf/tst-relro-shared-pie.c: Likewise.
	* support/Makefile (libsupport-routines): Add
	support_disable_more_coredumps, support_check_fault_write.
	(tests): Add tst-support_check_fault.
	* support/support.h (support_disable_more_coredumps): Declare.
	* support/check_fault.h: New file.
	* support/support_check_fault_write.c: Likewise.
	* support/support_disable_more_coredumps.c: Likewise.
	* support/tst-support_check_fault.c: Likewise.
	* sysdeps/mach/hurd/support_disable_more_coredumps.c: Likewise.
  

Comments

Carlos O'Donell Oct. 7, 2019, 4:37 a.m. UTC | #1
On 10/2/19 5:44 PM, Florian Weimer wrote:
> (I need to rework the RELRO handling to address the rseq initialization
> issue.  Tested on i686-linux-gnu, x86_64-linux-gnu,
> powerpc64le-linux-gnu, aarch64-linux-gnu, s390x-linux-gnu.  (The test
> does not exercise partial RELRO support, so there's no issue on s390x.)
> Build-tested on i686-gnu.  If approved, I will commit the support bits
> separately, for easier backporting.)
> 
> 2019-10-02  Florian Weimer  <fweimer@redhat.com>
> 
> 	elf: Add tests for working RELRO protection.
> 	* elf/Makefile [$(build-shared)] (tests): Add tst-relro-shared.
> 	[$(have-fpie) && $(build-shared)] (tests, tests-pie): Add
> 	tst-relro-shared-pie.
> 	(tst-relro-shared, tst-relro-shared-pie): Link with -ldl.
> 	(LDFLAGS-tst-relro-shared, LDFLAGS-tst-relro-shared-pie): Link
> 	with -z relro.
> 	(tst-relro-shared-no-pie): Set to disable PIE for
> 	tst-relro-shared.
> 	(CFLAGS-tst-relro-shared-pie.c): Build with -fpie.
> 	* elf/tst-relro-shared.c: New file.
> 	* elf/tst-relro-shared-pie.c: Likewise.
> 	* support/Makefile (libsupport-routines): Add
> 	support_disable_more_coredumps, support_check_fault_write.
> 	(tests): Add tst-support_check_fault.
> 	* support/support.h (support_disable_more_coredumps): Declare.
> 	* support/check_fault.h: New file.
> 	* support/support_check_fault_write.c: Likewise.
> 	* support/support_disable_more_coredumps.c: Likewise.
> 	* support/tst-support_check_fault.c: Likewise.
> 	* sysdeps/mach/hurd/support_disable_more_coredumps.c: Likewise.

See comments below.

Looking forward to a v2!

> diff --git a/elf/Makefile b/elf/Makefile
> index 8f962991a3..9c696e42a8 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -192,7 +192,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
>  	 tst-latepthread tst-tls-manydynamic tst-nodelete-dlclose \
>  	 tst-debug1 tst-main1 tst-absolute-sym tst-absolute-zero tst-big-note \
>  	 tst-unwind-ctor tst-unwind-main tst-audit13 \
> -	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout
> +	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout \
> +	 tst-relro-shared

OK. New test.

>  #	 reldep9
>  tests-internal += loadtest unload unload2 circleload1 \
>  	 neededtest neededtest2 neededtest3 neededtest4 \
> @@ -308,8 +309,8 @@ test-xfail-tst-protected1b = yes
>  endif
>  ifeq (yesyes,$(have-fpie)$(build-shared))
>  modules-names += tst-piemod1
> -tests += tst-pie1 tst-pie2 tst-dlopen-pie
> -tests-pie += tst-pie1 tst-pie2
> +tests += tst-pie1 tst-pie2 tst-dlopen-pie tst-relro-shared-pie

OK.

> +tests-pie += tst-pie1 tst-pie2 tst-relro-shared-pie

OK.

>  ifeq (yes,$(have-protected-data))
>  tests += vismain
>  tests-pie += vismain
> @@ -1270,6 +1271,12 @@ $(objpfx)tst-thrlock: $(libdl) $(shared-thread-library)
>  $(objpfx)tst-dlopen-aout: $(libdl) $(shared-thread-library)
>  $(objpfx)tst-dlopen-aout-container: $(libdl) $(shared-thread-library)
>  LDFLAGS-tst-dlopen-aout-container += -Wl,-rpath,\$$ORIGIN
> +$(objpfx)tst-relro-shared: $(libdl)
> +LDFLAGS-tst-relro-shared += $(relro-LDFLAGS)
> +tst-relro-shared-no-pie = yes
> +CFLAGS-tst-relro-shared-pie.c += $(pie-ccflag)
> +$(objpfx)tst-relro-shared-pie: $(libdl)
> +LDFLAGS-tst-relro-shared-pie += $(relro-LDFLAGS)

OK.

>  
>  CFLAGS-ifuncmain1pic.c += $(pic-ccflag)
>  CFLAGS-ifuncmain1picstatic.c += $(pic-ccflag)
> diff --git a/elf/tst-relro-shared-pie.c b/elf/tst-relro-shared-pie.c
> new file mode 100644
> index 0000000000..64f3dfcf45
> --- /dev/null
> +++ b/elf/tst-relro-shared-pie.c
> @@ -0,0 +1,19 @@
> +/* Test that RELRO is applied to the program, ld.so, libc.so.  PIE version.
> +   Copyright (C) 2019 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include "tst-relro-shared.c"

OK.

> diff --git a/elf/tst-relro-shared.c b/elf/tst-relro-shared.c
> new file mode 100644
> index 0000000000..e906fd7fa5
> --- /dev/null
> +++ b/elf/tst-relro-shared.c
> @@ -0,0 +1,64 @@
> +/* Test that RELRO protection is applied to the program, ld.so, libc.so.

OK.

> +   Copyright (C) 2019 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <gnu/lib-names.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <support/check_fault.h>
> +#include <support/xdlfcn.h>
> +
> +static void (*const global_function_pointer) (void *) = free;

OK.

> +
> +static int
> +do_test (void)
> +{
> +  printf ("info: checking global_function_pointer (%p)\n",
> +          &global_function_pointer);
> +  support_check_fault_write (&global_function_pointer,
> +                             sizeof (global_function_pointer));

OK.

> +
> +  /* Use dlsym to avoid creating copy relocations.  */

Could you please expand this comment to explain why this is required?

> +
> +  /* This comes from ld.so.  */
> +  void *ptr = xdlsym (NULL, "_rtld_global_ro");
> +  printf ("info: checking _rtld_global_ro (%p)\n", ptr);
> +  /* First byte is _dl_debug_mask, so overwriting that should be
> +     relatively harmless (and not cause a crash on its own).  */
> +  support_check_fault_write (ptr, 1);

What if these symbols are moved?

Could you use dlinfo just to double check they came from the expected locations?

> +
> +  /* This comes from libc.so.  */
> +  ptr = xdlsym (NULL, "_nl_default_dirname");
> +  printf ("info: checking _nl_default_dirname (%p)\n", ptr);
> +  support_check_fault_write (ptr, 1);

Likewise.

> +
> +  /* Load libc.so into a new namespace and check that as well. */

OK. Nice check!

> +
> +  void *new_libc = xdlmopen (LM_ID_NEWLM, LIBC_SO, RTLD_NOW);
> +  ptr = xdlsym (new_libc, "_rtld_global_ro");
> +  printf ("info: checking _rtld_global_ro in namespace (%p)\n", ptr);
> +  support_check_fault_write (ptr, 1);

OK.

> +  ptr = xdlsym (new_libc, "_nl_default_dirname");
> +  printf ("info: checking _nl_default_dirname in namespace (%p)\n", ptr);
> +  support_check_fault_write (ptr, 1);

OK.

> +
> +  xdlclose (new_libc);

OK.

> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/support/Makefile b/support/Makefile
> index ca238ee8b4..58ea1b974d 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -43,10 +43,12 @@ libsupport-routines = \
>    support_can_chroot \
>    support_capture_subprocess \
>    support_capture_subprocess_check \
> +  support_check_fault_write \

OK.

>    support_chroot \
>    support_copy_file_range \
>    support_descriptor_supports_holes \
>    support_descriptors \
> +  support_disable_more_coredumps \

OK.

>    support_enter_mount_namespace \
>    support_enter_network_namespace \
>    support_format_address_family \
> @@ -218,6 +220,7 @@ tests = \
>    tst-support-namespace \
>    tst-support_blob_repeat \
>    tst-support_capture_subprocess \
> +  tst-support_check_fault \

OK.

>    tst-support_descriptors \
>    tst-support_format_dns_packet \
>    tst-support_quote_blob \
> diff --git a/support/check_fault.h b/support/check_fault.h
> new file mode 100644
> index 0000000000..03d5c7f41d
> --- /dev/null
> +++ b/support/check_fault.h
> @@ -0,0 +1,29 @@
> +/* Check for faults at specific addresses.

OK.

> +   Copyright (C) 2019 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef SUPPORT_CHECK_FAULT_H
> +#define SUPPORT_CHECK_FAULT_H
> +
> +#include <stddef.h>
> +
> +/* Check that write access to SIZE individual bytes starting at
> +   address results in SIGSEGV or SIGBUS signal.  Both signals must not
> +   be blocked and must have SIG_DFL disposition.  */
> +void support_check_fault_write (const void *address, size_t size);

OK.

> +
> +#endif /* SUPPORT_CHECK_FAULT_H */
> diff --git a/support/support.h b/support/support.h
> index a9df6e9a3c..5677a8e682 100644
> --- a/support/support.h
> +++ b/support/support.h
> @@ -80,6 +80,11 @@ char *support_quote_string (const char *);
>     regular file open for writing, and initially empty.  */
>  int support_descriptor_supports_holes (int fd);
>  
> +/* For performance reasons, some tests need to disable all coredumps
> +   (not just those which are written to the source directory, which
> +   are disabled by the test driver).  */
> +void support_disable_more_coredumps (void);

OK.

> +
>  /* Error-checking wrapper functions which terminate the process on
>     error.  */
>  
> diff --git a/support/support_check_fault_write.c b/support/support_check_fault_write.c
> new file mode 100644
> index 0000000000..e3497863b6
> --- /dev/null
> +++ b/support/support_check_fault_write.c
> @@ -0,0 +1,72 @@
> +/* Check for faults at specific addresses.

OK.

> +   Copyright (C) 2019 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/check_fault.h>
> +
> +#include <signal.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +
> +/* Probe once at ADDRESS.  Use START and INDEX in error reporting.  */

OK.

> +static bool
> +check_one_address (volatile char *address, const void *start, size_t index)
> +{
> +  pid_t pid = xfork ();
> +  if (pid == 0)
> +    {
> +      support_disable_more_coredumps ();
> +      *address = 0xcc;  /* Try to write an arbitrary value.  */
> +      _exit (0);
> +    }
> +  int status;
> +  xwaitpid (pid, &status, 0);
> +  if (WIFSIGNALED (status))
> +    {
> +      if (WTERMSIG (status) != SIGSEGV && WTERMSIG (status) != SIGBUS)
> +        {
> +          support_record_failure ();
> +          printf ("error: unexpected signal %d at %p (%p+%zu)\n",
> +                  WTERMSIG (status), address, start, index);
> +          return false;
> +        }
> +    }
> +  else
> +    {
> +      support_record_failure ();
> +      printf ("error: missing fault at %p (%p+%zu)\n",
> +              address, start, index);
> +      return false;
> +    }
> +  return true;
> +}
> +
> +void
> +support_check_fault_write (const void *address, size_t size)
> +{
> +  TEST_VERIFY (address != NULL);
> +  TEST_VERIFY (size > 0);
> +
> +  for (size_t i = 0; i < size; ++i)
> +    if (!check_one_address (((char *) address) + i, address, i))

OK. Robust design. This is one fork per byte :-)

> +      break;
> +}

OK.

> diff --git a/support/support_disable_more_coredumps.c b/support/support_disable_more_coredumps.c
> new file mode 100644
> index 0000000000..954f16027b
> --- /dev/null
> +++ b/support/support_disable_more_coredumps.c
> @@ -0,0 +1,28 @@
> +/* Disable additional coredumps.  Version using <sys/prctl.h>.

OK.

> +   Copyright (C) 2019 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <sys/prctl.h>
> +
> +void
> +support_disable_more_coredumps (void)
> +{
> +  if (prctl (PR_SET_DUMPABLE, 0, 0, 0, 0) != 0)
> +    FAIL_EXIT1 ("prctl (PR_SET_DUMPABLE) failure: %m");

OK, this is linux specific but you fix that below with a hurd version of this framework function.

> +}
> diff --git a/support/tst-support_check_fault.c b/support/tst-support_check_fault.c
> new file mode 100644
> index 0000000000..8a6b6d9825
> --- /dev/null
> +++ b/support/tst-support_check_fault.c
> @@ -0,0 +1,118 @@
> +/* Tests for support_fault_check_write.

OK.

> +   Copyright (C) 2019 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/check_fault.h>
> +
> +#include <array_length.h>
> +#include <stdlib.h>
> +#include <support/capture_subprocess.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <sys/mman.h>
> +
> +static void
> +check_fault_two_bytes (void *address)
> +{
> +  support_check_fault_write (address, 2);

OK.

> +}
> +
> +static int
> +do_test (void)
> +{
> +  size_t page_size = xsysconf (_SC_PAGESIZE);
> +
> +  /* Tests with PROT_NONE, PROT_READ and PROT_EXEC are expected to
> +     succeed.  */
> +  {
> +    static const int prots[] = { PROT_NONE, PROT_READ, PROT_EXEC };
> +    for (int i = 0; i < array_length (prots); ++i)
> +      {
> +        void *ptr = xmmap (NULL, 2, prots[i], MAP_PRIVATE | MAP_ANONYMOUS, -1);
> +        support_check_fault_write (ptr, 2);
> +        xmunmap (ptr, 2);
> +      }
> +  }

OK.

> +
> +  /* Do not call support_record_failure_reset below if the process has
> +     already failed.  */
> +  int is_failed = support_record_failure_is_failed ();
> +
> +  /* Check that the lack of a fault for regular memory is reported at
> +     the fist byte.  */
OK. Thank you for putting comments at the start of each of these test blocks.
It makes it really clear what's going on.

> +  {
> +    void *ptr = xmalloc (2);
> +    struct support_capture_subprocess proc
> +      = support_capture_subprocess (check_fault_two_bytes, ptr);
> +    support_capture_subprocess_check (&proc, "malloc", 0, sc_allow_stdout);
> +    if (!is_failed)
> +      support_record_failure_reset ();
> +    char *expected = xasprintf ("error: missing fault at %p (%p+0)\n",
> +                                ptr, ptr);
> +    TEST_COMPARE_STRING (proc.out.buffer, expected);
> +    free (expected);
> +    support_capture_subprocess_free (&proc);
> +    free (ptr);
> +  }

OK.

> +
> +  /* Check that a fault only at the first byte results in an error.
> +     The probe range crosses a page boundary.  */

Should this say: "Check that a *missing* fault..." ?

> +  is_failed = support_record_failure_is_failed ();
> +  {
> +    char *ptr = xmmap (NULL, 2 * page_size, PROT_READ | PROT_WRITE,
> +                       MAP_PRIVATE | MAP_ANONYMOUS, -1);
> +    xmprotect (ptr, page_size, PROT_READ);
> +    struct support_capture_subprocess proc
> +      = support_capture_subprocess (check_fault_two_bytes,
> +                                    ptr + page_size - 1);
> +    support_capture_subprocess_check (&proc, "R, RW", 0, sc_allow_stdout);
> +    if (!is_failed)
> +      support_record_failure_reset ();
> +    char *expected = xasprintf ("error: missing fault at %p (%p+1)\n",
> +                                ptr + page_size, ptr + page_size - 1);
> +    TEST_COMPARE_STRING (proc.out.buffer, expected);
> +    free (expected);
> +    support_capture_subprocess_free (&proc);
> +    xmunmap (ptr, 2 * page_size);
> +  }


OK.

> +
> +  /* Check that a fault only at the second byte results in an error.
> +     Again, the probe range crosses a page boundary.  */

Should this say "Check that a *missing* fault only at the ... " ?

> +  is_failed = support_record_failure_is_failed ();
> +  {
> +    char *ptr = xmmap (NULL, 2 * page_size, PROT_READ | PROT_WRITE,
> +                       MAP_PRIVATE | MAP_ANONYMOUS, -1);
> +    xmprotect (ptr + page_size, page_size, PROT_READ);
> +    struct support_capture_subprocess proc
> +      = support_capture_subprocess (check_fault_two_bytes,
> +                                    ptr + page_size - 1);
> +    support_capture_subprocess_check (&proc, "RW, R", 0, sc_allow_stdout);
> +    if (!is_failed)
> +      support_record_failure_reset ();
> +    char *expected = xasprintf ("error: missing fault at %p (%p+0)\n",
> +                                ptr + page_size - 1, ptr + page_size - 1);
> +    TEST_COMPARE_STRING (proc.out.buffer, expected);
> +    free (expected);
> +    support_capture_subprocess_free (&proc);
> +    xmunmap (ptr, 2 * page_size);
> +  }
> +

OK.

> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/mach/hurd/support_disable_more_coredumps.c b/sysdeps/mach/hurd/support_disable_more_coredumps.c
> new file mode 100644
> index 0000000000..fe0fdcc89a
> --- /dev/null
> +++ b/sysdeps/mach/hurd/support_disable_more_coredumps.c
> @@ -0,0 +1,25 @@
> +/* Disable additional coredumps.  Hurd version.
> +   Copyright (C) 2019 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/support.h>
> +
> +void
> +support_disable_more_coredumps (void)
> +{
> +  /* Hurd does not have any additional coredumps to disable.  */
> +}
> 

OK. This fixes the prctl issue. Looks like you ran bmg? ;-)

It is odd to have a file that is for support/* but is outside of support/*.

I wish we had a better answer for that. Like making a stub prctl for hurd, 
but that might cause configure checks to pass and cause problems in other ways.

Thanks for at least considering hurd, as a steward for the project I appreciate
that.
  
Florian Weimer Oct. 7, 2019, 11:29 a.m. UTC | #2
* Carlos O'Donell:

> Could you use dlinfo just to double check they came from the expected
> locations?

There's currently no way to obtain the soname of an object.  I'm going
to add an interface for that.

l_name varies depending on how the object has been loaded.

Thanks,
Florian
  

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 8f962991a3..9c696e42a8 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -192,7 +192,8 @@  tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-latepthread tst-tls-manydynamic tst-nodelete-dlclose \
 	 tst-debug1 tst-main1 tst-absolute-sym tst-absolute-zero tst-big-note \
 	 tst-unwind-ctor tst-unwind-main tst-audit13 \
-	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout
+	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout \
+	 tst-relro-shared
 #	 reldep9
 tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
@@ -308,8 +309,8 @@  test-xfail-tst-protected1b = yes
 endif
 ifeq (yesyes,$(have-fpie)$(build-shared))
 modules-names += tst-piemod1
-tests += tst-pie1 tst-pie2 tst-dlopen-pie
-tests-pie += tst-pie1 tst-pie2
+tests += tst-pie1 tst-pie2 tst-dlopen-pie tst-relro-shared-pie
+tests-pie += tst-pie1 tst-pie2 tst-relro-shared-pie
 ifeq (yes,$(have-protected-data))
 tests += vismain
 tests-pie += vismain
@@ -1270,6 +1271,12 @@  $(objpfx)tst-thrlock: $(libdl) $(shared-thread-library)
 $(objpfx)tst-dlopen-aout: $(libdl) $(shared-thread-library)
 $(objpfx)tst-dlopen-aout-container: $(libdl) $(shared-thread-library)
 LDFLAGS-tst-dlopen-aout-container += -Wl,-rpath,\$$ORIGIN
+$(objpfx)tst-relro-shared: $(libdl)
+LDFLAGS-tst-relro-shared += $(relro-LDFLAGS)
+tst-relro-shared-no-pie = yes
+CFLAGS-tst-relro-shared-pie.c += $(pie-ccflag)
+$(objpfx)tst-relro-shared-pie: $(libdl)
+LDFLAGS-tst-relro-shared-pie += $(relro-LDFLAGS)
 
 CFLAGS-ifuncmain1pic.c += $(pic-ccflag)
 CFLAGS-ifuncmain1picstatic.c += $(pic-ccflag)
diff --git a/elf/tst-relro-shared-pie.c b/elf/tst-relro-shared-pie.c
new file mode 100644
index 0000000000..64f3dfcf45
--- /dev/null
+++ b/elf/tst-relro-shared-pie.c
@@ -0,0 +1,19 @@ 
+/* Test that RELRO is applied to the program, ld.so, libc.so.  PIE version.
+   Copyright (C) 2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include "tst-relro-shared.c"
diff --git a/elf/tst-relro-shared.c b/elf/tst-relro-shared.c
new file mode 100644
index 0000000000..e906fd7fa5
--- /dev/null
+++ b/elf/tst-relro-shared.c
@@ -0,0 +1,64 @@ 
+/* Test that RELRO protection is applied to the program, ld.so, libc.so.
+   Copyright (C) 2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <gnu/lib-names.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <support/check_fault.h>
+#include <support/xdlfcn.h>
+
+static void (*const global_function_pointer) (void *) = free;
+
+static int
+do_test (void)
+{
+  printf ("info: checking global_function_pointer (%p)\n",
+          &global_function_pointer);
+  support_check_fault_write (&global_function_pointer,
+                             sizeof (global_function_pointer));
+
+  /* Use dlsym to avoid creating copy relocations.  */
+
+  /* This comes from ld.so.  */
+  void *ptr = xdlsym (NULL, "_rtld_global_ro");
+  printf ("info: checking _rtld_global_ro (%p)\n", ptr);
+  /* First byte is _dl_debug_mask, so overwriting that should be
+     relatively harmless (and not cause a crash on its own).  */
+  support_check_fault_write (ptr, 1);
+
+  /* This comes from libc.so.  */
+  ptr = xdlsym (NULL, "_nl_default_dirname");
+  printf ("info: checking _nl_default_dirname (%p)\n", ptr);
+  support_check_fault_write (ptr, 1);
+
+  /* Load libc.so into a new namespace and check that as well. */
+
+  void *new_libc = xdlmopen (LM_ID_NEWLM, LIBC_SO, RTLD_NOW);
+  ptr = xdlsym (new_libc, "_rtld_global_ro");
+  printf ("info: checking _rtld_global_ro in namespace (%p)\n", ptr);
+  support_check_fault_write (ptr, 1);
+  ptr = xdlsym (new_libc, "_nl_default_dirname");
+  printf ("info: checking _nl_default_dirname in namespace (%p)\n", ptr);
+  support_check_fault_write (ptr, 1);
+
+  xdlclose (new_libc);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/support/Makefile b/support/Makefile
index ca238ee8b4..58ea1b974d 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -43,10 +43,12 @@  libsupport-routines = \
   support_can_chroot \
   support_capture_subprocess \
   support_capture_subprocess_check \
+  support_check_fault_write \
   support_chroot \
   support_copy_file_range \
   support_descriptor_supports_holes \
   support_descriptors \
+  support_disable_more_coredumps \
   support_enter_mount_namespace \
   support_enter_network_namespace \
   support_format_address_family \
@@ -218,6 +220,7 @@  tests = \
   tst-support-namespace \
   tst-support_blob_repeat \
   tst-support_capture_subprocess \
+  tst-support_check_fault \
   tst-support_descriptors \
   tst-support_format_dns_packet \
   tst-support_quote_blob \
diff --git a/support/check_fault.h b/support/check_fault.h
new file mode 100644
index 0000000000..03d5c7f41d
--- /dev/null
+++ b/support/check_fault.h
@@ -0,0 +1,29 @@ 
+/* Check for faults at specific addresses.
+   Copyright (C) 2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef SUPPORT_CHECK_FAULT_H
+#define SUPPORT_CHECK_FAULT_H
+
+#include <stddef.h>
+
+/* Check that write access to SIZE individual bytes starting at
+   address results in SIGSEGV or SIGBUS signal.  Both signals must not
+   be blocked and must have SIG_DFL disposition.  */
+void support_check_fault_write (const void *address, size_t size);
+
+#endif /* SUPPORT_CHECK_FAULT_H */
diff --git a/support/support.h b/support/support.h
index a9df6e9a3c..5677a8e682 100644
--- a/support/support.h
+++ b/support/support.h
@@ -80,6 +80,11 @@  char *support_quote_string (const char *);
    regular file open for writing, and initially empty.  */
 int support_descriptor_supports_holes (int fd);
 
+/* For performance reasons, some tests need to disable all coredumps
+   (not just those which are written to the source directory, which
+   are disabled by the test driver).  */
+void support_disable_more_coredumps (void);
+
 /* Error-checking wrapper functions which terminate the process on
    error.  */
 
diff --git a/support/support_check_fault_write.c b/support/support_check_fault_write.c
new file mode 100644
index 0000000000..e3497863b6
--- /dev/null
+++ b/support/support_check_fault_write.c
@@ -0,0 +1,72 @@ 
+/* Check for faults at specific addresses.
+   Copyright (C) 2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/check_fault.h>
+
+#include <signal.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+/* Probe once at ADDRESS.  Use START and INDEX in error reporting.  */
+static bool
+check_one_address (volatile char *address, const void *start, size_t index)
+{
+  pid_t pid = xfork ();
+  if (pid == 0)
+    {
+      support_disable_more_coredumps ();
+      *address = 0xcc;  /* Try to write an arbitrary value.  */
+      _exit (0);
+    }
+  int status;
+  xwaitpid (pid, &status, 0);
+  if (WIFSIGNALED (status))
+    {
+      if (WTERMSIG (status) != SIGSEGV && WTERMSIG (status) != SIGBUS)
+        {
+          support_record_failure ();
+          printf ("error: unexpected signal %d at %p (%p+%zu)\n",
+                  WTERMSIG (status), address, start, index);
+          return false;
+        }
+    }
+  else
+    {
+      support_record_failure ();
+      printf ("error: missing fault at %p (%p+%zu)\n",
+              address, start, index);
+      return false;
+    }
+  return true;
+}
+
+void
+support_check_fault_write (const void *address, size_t size)
+{
+  TEST_VERIFY (address != NULL);
+  TEST_VERIFY (size > 0);
+
+  for (size_t i = 0; i < size; ++i)
+    if (!check_one_address (((char *) address) + i, address, i))
+      break;
+}
diff --git a/support/support_disable_more_coredumps.c b/support/support_disable_more_coredumps.c
new file mode 100644
index 0000000000..954f16027b
--- /dev/null
+++ b/support/support_disable_more_coredumps.c
@@ -0,0 +1,28 @@ 
+/* Disable additional coredumps.  Version using <sys/prctl.h>.
+   Copyright (C) 2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/check.h>
+#include <support/support.h>
+#include <sys/prctl.h>
+
+void
+support_disable_more_coredumps (void)
+{
+  if (prctl (PR_SET_DUMPABLE, 0, 0, 0, 0) != 0)
+    FAIL_EXIT1 ("prctl (PR_SET_DUMPABLE) failure: %m");
+}
diff --git a/support/tst-support_check_fault.c b/support/tst-support_check_fault.c
new file mode 100644
index 0000000000..8a6b6d9825
--- /dev/null
+++ b/support/tst-support_check_fault.c
@@ -0,0 +1,118 @@ 
+/* Tests for support_fault_check_write.
+   Copyright (C) 2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/check_fault.h>
+
+#include <array_length.h>
+#include <stdlib.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xunistd.h>
+#include <sys/mman.h>
+
+static void
+check_fault_two_bytes (void *address)
+{
+  support_check_fault_write (address, 2);
+}
+
+static int
+do_test (void)
+{
+  size_t page_size = xsysconf (_SC_PAGESIZE);
+
+  /* Tests with PROT_NONE, PROT_READ and PROT_EXEC are expected to
+     succeed.  */
+  {
+    static const int prots[] = { PROT_NONE, PROT_READ, PROT_EXEC };
+    for (int i = 0; i < array_length (prots); ++i)
+      {
+        void *ptr = xmmap (NULL, 2, prots[i], MAP_PRIVATE | MAP_ANONYMOUS, -1);
+        support_check_fault_write (ptr, 2);
+        xmunmap (ptr, 2);
+      }
+  }
+
+  /* Do not call support_record_failure_reset below if the process has
+     already failed.  */
+  int is_failed = support_record_failure_is_failed ();
+
+  /* Check that the lack of a fault for regular memory is reported at
+     the fist byte.  */
+  {
+    void *ptr = xmalloc (2);
+    struct support_capture_subprocess proc
+      = support_capture_subprocess (check_fault_two_bytes, ptr);
+    support_capture_subprocess_check (&proc, "malloc", 0, sc_allow_stdout);
+    if (!is_failed)
+      support_record_failure_reset ();
+    char *expected = xasprintf ("error: missing fault at %p (%p+0)\n",
+                                ptr, ptr);
+    TEST_COMPARE_STRING (proc.out.buffer, expected);
+    free (expected);
+    support_capture_subprocess_free (&proc);
+    free (ptr);
+  }
+
+  /* Check that a fault only at the first byte results in an error.
+     The probe range crosses a page boundary.  */
+  is_failed = support_record_failure_is_failed ();
+  {
+    char *ptr = xmmap (NULL, 2 * page_size, PROT_READ | PROT_WRITE,
+                       MAP_PRIVATE | MAP_ANONYMOUS, -1);
+    xmprotect (ptr, page_size, PROT_READ);
+    struct support_capture_subprocess proc
+      = support_capture_subprocess (check_fault_two_bytes,
+                                    ptr + page_size - 1);
+    support_capture_subprocess_check (&proc, "R, RW", 0, sc_allow_stdout);
+    if (!is_failed)
+      support_record_failure_reset ();
+    char *expected = xasprintf ("error: missing fault at %p (%p+1)\n",
+                                ptr + page_size, ptr + page_size - 1);
+    TEST_COMPARE_STRING (proc.out.buffer, expected);
+    free (expected);
+    support_capture_subprocess_free (&proc);
+    xmunmap (ptr, 2 * page_size);
+  }
+
+  /* Check that a fault only at the second byte results in an error.
+     Again, the probe range crosses a page boundary.  */
+  is_failed = support_record_failure_is_failed ();
+  {
+    char *ptr = xmmap (NULL, 2 * page_size, PROT_READ | PROT_WRITE,
+                       MAP_PRIVATE | MAP_ANONYMOUS, -1);
+    xmprotect (ptr + page_size, page_size, PROT_READ);
+    struct support_capture_subprocess proc
+      = support_capture_subprocess (check_fault_two_bytes,
+                                    ptr + page_size - 1);
+    support_capture_subprocess_check (&proc, "RW, R", 0, sc_allow_stdout);
+    if (!is_failed)
+      support_record_failure_reset ();
+    char *expected = xasprintf ("error: missing fault at %p (%p+0)\n",
+                                ptr + page_size - 1, ptr + page_size - 1);
+    TEST_COMPARE_STRING (proc.out.buffer, expected);
+    free (expected);
+    support_capture_subprocess_free (&proc);
+    xmunmap (ptr, 2 * page_size);
+  }
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/mach/hurd/support_disable_more_coredumps.c b/sysdeps/mach/hurd/support_disable_more_coredumps.c
new file mode 100644
index 0000000000..fe0fdcc89a
--- /dev/null
+++ b/sysdeps/mach/hurd/support_disable_more_coredumps.c
@@ -0,0 +1,25 @@ 
+/* Disable additional coredumps.  Hurd version.
+   Copyright (C) 2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/support.h>
+
+void
+support_disable_more_coredumps (void)
+{
+  /* Hurd does not have any additional coredumps to disable.  */
+}