[PATCH] Add tests for Safe-Linking

[Carlos O'Donell]

On 4/2/20 7:45 AM, Eyal Itkin via Libc-alpha wrote:
> Hello,
> 
> As promised, I converted my test cases and added a new test case
> called: tst-safe-linking.
> This test checks that Safe-Linking works, by testing the 3 main flows:
> 1. tcache protection
> 2. fastbin protection
> 3. malloc_consolidate() correctness
> 
> As there is a random chance of 1/16 that of the alignment will remain
> correct, the test checks each flow up to 10 times, using different
> random
> values for the pointer corruption. As a result, the chance for a false
> failure of a given tested flow is 2**(-40), thus highly unlikely.
> 
> The test was tested on an intel 32 bit and 64 bit setup, and it never
> failed even after dozens of executions on each setup.

Thank you for putting together these tests. They look good to me, and
while they might be a bit fragile they add critical coverage to these
checks.

make test t=malloc/tst-safe-linking passes on 32-bit and 64-bit.

No regressions.

OK to push with the cleanups added? (provided as attached diff for review)

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> From ef66bb07c2081075dd210368606fcf3060b39db7 Mon Sep 17 00:00:00 2001
> From: Eyal Itkin <eyalit@checkpoint.com>
> Date: Thu, 2 Apr 2020 07:26:35 -0400
> Subject: [PATCH] Add tests for Safe-Linking
> 
> Adding the test "tst-safe-linking" for testing that Safe-Linking works
> as expected. The test checks these 3 main flows:
>  * tcache protection
>  * fastbin protection
>  * malloc_consolidate() correctness
> 
> As there is a random chance of 1/16 that of the alignment will remain
> correct, the test checks each flow up to 10 times, using different random
> values for the pointer corruption. As a result, the chance for a false
> failure of a given tested flow is 2**(-40), thus highly unlikely.
> ---
>  malloc/Makefile           |   1 +
>  malloc/tst-safe-linking.c | 180 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 181 insertions(+)
>  create mode 100644 malloc/tst-safe-linking.c
> 
> diff --git a/malloc/Makefile b/malloc/Makefile
> index 984045b5b9..e22cbde22d 100644
> --- a/malloc/Makefile
> +++ b/malloc/Makefile
> @@ -39,6 +39,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
>  	 tst-malloc-too-large \
>  	 tst-malloc-stats-cancellation \
>  	 tst-tcfree1 tst-tcfree2 tst-tcfree3 \
> +	 tst-safe-linking \

OK. Normal test.

>  
>  tests-static := \
>  	 tst-interpose-static-nothread \
> diff --git a/malloc/tst-safe-linking.c b/malloc/tst-safe-linking.c
> new file mode 100644
> index 0000000000..324dd91454
> --- /dev/null
> +++ b/malloc/tst-safe-linking.c
> @@ -0,0 +1,180 @@
> +/* Test reporting of Safe-Linking caught errors.

OK. First line has description.

> +   Copyright (C) 2017-2020 Free Software Foundation, Inc.

Should say "2020" only since the test is new.

> +   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 <signal.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <memory.h>
> +#include <string.h>
> +#include <time.h>
> +#include <stdbool.h>
> +#include <support/capture_subprocess.h>
> +#include <support/check.h>
> +
> +/* Run CALLBACK and check that the data on standard error equals
> +   EXPECTED.  */
> +static void
> +check (const char *test, void (*callback) (void *),
> +       const char *expected)
> +{
> +  int i, rand_mask;
> +  bool success = false;
> +  /* There is a chance of 1/16 that a corrupted pointer will be aligned.
> +     Try multiple times so that statistical failure will be improbable.  */
> +  for (i=0; i<10 && !success; ++i)

GNU Style: for (i = 0; i < 10 && !success; ++i)
- Spaces between operators.

> +    {
> +      rand_mask = rand() & 0xFF;

GNU Style: rand_mask = rand () & 0xFF;
- Space after function.

> +      struct support_capture_subprocess result
> +	= support_capture_subprocess (callback, &rand_mask);
> +      /* Did not crash, could happen.  Try again.  */
> +      if (strlen (result.err.buffer) == 0)
> +	continue;
> +      /* Crashed, must be the expected result.  */
> +      if (strcmp (result.err.buffer, expected) != 0)
> +	{
> +	  support_record_failure ();
> +	  printf ("error: test %s unexpected standard error data\n"
> +	          "  expected: %s\n"
> +	          "  actual:   %s\n",
> +	          test, expected, result.err.buffer);
> +	}
> +      TEST_VERIFY (WIFSIGNALED (result.status));
> +      if (WIFSIGNALED (result.status))
> +	TEST_VERIFY (WTERMSIG (result.status) == SIGABRT);
> +      support_capture_subprocess_free (&result);
> +      success = true;
> +    }
> +  TEST_VERIFY (success);
> +}
> +

Suggest:
/* Implementation details must be kept in sync with malloc.  */

> +#define TCACHE_FILL_COUNT               7
> +#define TCACHE_ALLOC_SIZE               0x20
> +#define MALLOC_CONSOLIDATE_SIZE         256*1024
> +
> +/* Try corrupting the tcache list.  */
> +static void
> +test_tcache (void *closure)
> +{
> +  int mask = ((int*)closure)[0];

GNU style - space after type before pointer e.g. ((int *)closure)[0]

> +  size_t size = TCACHE_ALLOC_SIZE;
> +
> +  /* Populate the tcache list.  */
> +  void * volatile a = malloc (size);
> +  void * volatile b = malloc (size);
> +  void * volatile c = malloc (size);
> +  free (a);
> +  free (b);
> +  free (c);
> +
> +  /* Corrupt the pointer with a random value, and avoid optimizations.  */
> +  printf("Before: c=%p, c[0]=%p\n", c, ((void **)c)[0]);

printf (...

> +  memset (c, mask & 0xFF, size);
> +  printf("After: c=%p, c[0]=%p\n", c, ((void **)c)[0]);

printf (...

> +
> +  c = malloc (size);
> +  /* This line will trigger the Safe-Linking check.  */
> +  b = malloc (size);
> +  printf("b=%p\n", b);

printf (...

> +}
> +
> +/* Try corrupting the fastbin list.  */
> +static void
> +test_fastbin (void *closure)
> +{
> +  int i;
> +  int mask = ((int*)closure)[0];

(int *...

> +  size_t size = TCACHE_ALLOC_SIZE;
> +
> +  /* Take the tcache out of the game.  */
> +  for (i = 0; i<TCACHE_FILL_COUNT; ++i)

i < TCACHE_FILL_COUNT...

> +    {
> +      void * volatile p = calloc (1, size);
> +      free (p);
> +    }
> +
> +  /* Populate the fastbin list.  */
> +  void * volatile a = calloc (1, size);
> +  void * volatile b = calloc (1, size);
> +  void * volatile c = calloc (1, size);
> +  free (a);
> +  free (b);
> +  free (c);
> +
> +  /* Corrupt the pointer with a random value, and avoid optimizations.  */
> +  printf("Before: c=%p, c[0]=%p\n", c, ((void **)c)[0]);

printf (...

> +  memset (c, mask & 0xFF, size);
> +  printf("After: c=%p, c[0]=%p\n", c, ((void **)c)[0]);

printf (...

> +
> +  c = calloc (1, size);
> +  /* This line will trigger the Safe-Linking check.  */
> +  b = calloc (1, size);
> +  printf("b=%p\n", b);

printf (...

> +}
> +
> +/* Try corrupting the fastbin list and trigger a consolidate.  */
> +static void
> +test_fastbin_consolidate (void *closure)
> +{
> +  int i;
> +  int mask = ((int*)closure)[0];
> +  size_t size = TCACHE_ALLOC_SIZE;
> +
> +  /* Take the tcache out of the game.  */
> +  for (i = 0; i<TCACHE_FILL_COUNT; ++i)

i < TCACHE_FILL_COUNT...

> +    {
> +      void * volatile p = calloc (1, size);
> +      free (p);
> +    }
> +
> +  /* Populate the fastbin list.  */
> +  void * volatile a = calloc (1, size);
> +  void * volatile b = calloc (1, size);
> +  void * volatile c = calloc (1, size);
> +  free (a);
> +  free (b);
> +  free (c);
> +
> +  /* Corrupt the pointer with a random value, and avoid optimizations.  */
> +  printf("Before: c=%p, c[0]=%p\n", c, ((void **)c)[0]);

printf (...

> +  memset (c, mask & 0xFF, size);
> +  printf("After: c=%p, c[0]=%p\n", c, ((void **)c)[0]);

printf (...

> +
> +  /* This line will trigger the Safe-Linking check.  */
> +  b = malloc (MALLOC_CONSOLIDATE_SIZE);> +  printf("b=%p\n", b);

printf (...

> +}
> +
> +static int
> +do_test (void)
> +{
> +  /* Seed the random for the test.  */
> +  srand (time (NULL));
> +

No longer needed as of commit a289ea09ea843ced6e5277c2f2e63c357bc7f9a3
> +  TEST_VERIFY (setenv ("LIBC_FATAL_STDERR_", "1", 1) == 0);

Other test should get cleaned up also.

> +
> +  check ("test_tcache", test_tcache,
> +         "malloc(): unaligned tcache chunk detected\n");
> +  check ("test_fastbin", test_fastbin,
> +         "malloc(): unaligned fastbin chunk detected 2\n");
> +  check ("test_fastbin_consolidate", test_fastbin_consolidate,
> +         "malloc_consolidate(): unaligned fastbin chunk detected\n");
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> -- 
> 2.20.1
> 


-- 
Cheers,
Carlos.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cleanups.diff
Type: text/x-patch
Size: 4382 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20200402/106db055/attachment-0001.bin>