From patchwork Thu Aug 18 16:29:45 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Zack Weinberg X-Patchwork-Id: 14760 Received: (qmail 91250 invoked by alias); 18 Aug 2016 16:30:01 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 91232 invoked by uid 89); 18 Aug 2016 16:30:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL, BAYES_40, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=_not_, landing, stdout, alt X-HELO: mail-qk0-f193.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to; bh=Bl9qzwH910BL1QWshjUE2mAyzgysG4xfzs9Hy2cgn9Q=; b=dq+EgoLNL4sb4l/PQX2OknplilSaCzfTIl7dbP0DS4aQXZwHSdhVs4TW8dV2Oi/j8f b0uIaeONUPkrrx2Adg6j0SJlevD1jE/24tkb9zTiM1R2mQ0BXiXZxuOJAT4Kd+l2vJhk eWVqT/4YCBR0T6qWlwnZL5aNpF3PRTvAyolagysEVuIe3NSZrb5oIbFWI8fSGRXLr2QX iYFW9EL+sap1C+5Dn52hfVgeBvPIKrL4NtmGDGmGx3ZuMhMyZzoRrDJR9LzCk6gm/nvD UYUenu+id4JeLhou1n3TxICHvKLvROUWSspvXsMd2IGsxFZrpR+COIaIyl2+6IjlcXxd NPnA== X-Gm-Message-State: AEkoouvSKkN4HEG54+xkOzy0LgZJRdzvOAb+IYKfSHYa6WTDgC6tLuoZhA8JXJgrJnao+w== X-Received: by 10.55.43.130 with SMTP id r2mr3646741qkr.223.1471537787934; Thu, 18 Aug 2016 09:29:47 -0700 (PDT) Subject: Re: [PATCH 0/4] explicit_bzero, this time for sure To: libc-alpha@sourceware.org References: <1ea59b00-0ee9-4ba4-ee52-66104c6fc5c5@gmail.com> From: Zack Weinberg Message-ID: <8fe23bfb-1447-4f49-ff47-c197d86f4826@panix.com> Date: Thu, 18 Aug 2016 12:29:45 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.2.0 MIME-Version: 1.0 In-Reply-To: <1ea59b00-0ee9-4ba4-ee52-66104c6fc5c5@gmail.com> On 08/17/2016 06:20 PM, Manuel López-Ibáñez wrote: > On 17/08/16 18:19, Zack Weinberg wrote: >> anything I would like to hear about it. If anyone has any ideas for >> how to write a test that will start failing if a compiler ever learns >> to "see through" __glibc_read_memory, I would like to hear about that, >> too. (I can imagine a way to do it with gcc's scan-assembler tests, >> but we don't have those here.) > > Perhaps? https://plus.google.com/+MatthewDempsky/posts/KQHFBouxurX Ingenious! Done - patch 5/4 attached to this message, and also included in zack/explicit-bzero. I would fold this into patch 2 for landing, but it may be easier to review this way. (What is the preferred way to deal with user-namespace project branches that get rebased? The server won't let me push -f; I can delete the branch and recreate it but that seems ... not great.) zw From a4eb2c453d93e566f00a9f67f6775a51a2eef9fc Mon Sep 17 00:00:00 2001 From: Zack Weinberg Date: Thu, 18 Aug 2016 12:22:55 -0400 Subject: [PATCH] Add a test for explicit_bzero not getting optimized out. Conceptually based on a test written by Matthew Dempsky for the OpenBSD regression suite. * string/tst-xbzero-opt.c: New test. * string/Makefile: Run it. --- string/Makefile | 2 +- string/tst-xbzero-opt.c | 383 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 384 insertions(+), 1 deletion(-) create mode 100644 string/tst-xbzero-opt.c diff --git a/string/Makefile b/string/Makefile index 180ec86..be7154b 100644 --- a/string/Makefile +++ b/string/Makefile @@ -60,7 +60,7 @@ tests := tester inl-tester noinl-tester testcopy test-ffs \ tst-strtok tst-strxfrm bug-strcoll1 tst-strfry \ bug-strtok1 $(addprefix test-,$(strop-tests)) \ bug-envz1 tst-strxfrm2 tst-endian tst-svc2 \ - tst-strtok_r bug-strcoll2 + tst-strtok_r bug-strcoll2 tst-xbzero-opt xtests = tst-strcoll-overflow diff --git a/string/tst-xbzero-opt.c b/string/tst-xbzero-opt.c new file mode 100644 index 0000000..81eb5c1 --- /dev/null +++ b/string/tst-xbzero-opt.c @@ -0,0 +1,383 @@ +/* Test that explicit_bzero block clears are not optimized out. + Copyright (C) 2016 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 + . */ + +/* This test is conceptually based on a test designed by Matthew + Dempsky for the OpenBSD regression suite: + /src/regress/lib/libc/explicit_bzero/explicit_bzero.c. + The basic idea is, we have a function that contains a + block-clearing operation (not necessarily explicit_bzero), after + which the block is dead, in the compiler-jargon sense. Execute + that function from a signal handler running on an alternative + signal stack. Then we have another pointer to the memory region + affected by the block clear -- namely, the sigaltstack buffer -- + and can find out whether it actually happened. + + The OpenBSD test cautions that some operating systems (e.g. Solaris + and OSX) wipe the signal stack upon returning to the normal stack, + so the test has to happen while still executing on the signal + stack. This, of course, means that the buffer might be clobbered + by normal stack operations after the function with the block clear + returns (it has to return, so that the block is truly dead). The + most straightforward way to deal with this is to have a large block + containing several copies of a byte pattern that is unlikely to + occur by chance, and check whether _any_ of them survives. */ + +#define _GNU_SOURCE 1 + +#include +#include +#include +#include +#include +#include +#include + +/* The "byte pattern that is unlikely to occur by chance": the first + 16 prime numbers (OEIS A000040). */ +static const unsigned char test_pattern[16] = +{ + 2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53 +}; + +#define PATTERN_SIZE (sizeof test_pattern) +#define PATTERN_REPS 32 +#define TEST_BUFFER_SIZE (PATTERN_SIZE * PATTERN_REPS) + +static void +fill_with_test_pattern (unsigned char *buf) +{ + for (unsigned int i = 0; i < PATTERN_REPS; i++) + memcpy (buf + i*PATTERN_SIZE, test_pattern, PATTERN_SIZE); +} + +static unsigned int +count_test_patterns (unsigned char *buf, size_t bufsiz) +{ + unsigned char *first = memmem (buf, bufsiz, test_pattern, PATTERN_SIZE); + if (!first) + return 0; + unsigned int cnt = 0; + for (unsigned int i = 0; i < PATTERN_REPS; i++) + { + unsigned char *p = first + i*PATTERN_SIZE; + if (p + PATTERN_SIZE - buf > bufsiz) + break; + if (memcmp (p, test_pattern, PATTERN_SIZE) == 0) + cnt++; + } + return cnt; +} + +/* Global test state. */ +static int failed_subtests; +static bool this_subtest_failed; + +/* The signal stack is allocated with memalign. */ +static unsigned char *signal_stack_buffer; +#define SIGNAL_STACK_SIZE (SIGSTKSZ + TEST_BUFFER_SIZE) + +enum test_expectation { EXPECT_NONE, EXPECT_SOME, EXPECT_ALL }; + +static void +check_test_buffer (enum test_expectation expected, + const char *label, const char *stage) +{ + unsigned int cnt = count_test_patterns (signal_stack_buffer, + SIGNAL_STACK_SIZE); + switch (expected) + { + case EXPECT_NONE: + if (cnt == 0) + { + printf ("PASS: %s/%s: expected 0 got %d\n", label, stage, cnt); + fflush (stdout); + } + else + { + printf ("FAIL: %s/%s: expected 0 got %d\n", label, stage, cnt); + fflush (stdout); + this_subtest_failed = true; + failed_subtests++; + } + break; + + case EXPECT_SOME: + if (cnt > 0) + { + printf ("PASS: %s/%s: expected some got %d\n", label, stage, cnt); + fflush (stdout); + } + else + { + printf ("FAIL: %s/%s: expected some got 0\n", label, stage); + fflush (stdout); + this_subtest_failed = true; + failed_subtests++; + } + break; + + case EXPECT_ALL: + if (cnt == PATTERN_REPS) + { + printf ("PASS: %s/%s: expected %d got %d\n", label, stage, + PATTERN_REPS, cnt); + fflush (stdout); + } + else + { + printf ("FAIL: %s/%s: expected %d got %d\n", label, stage, + PATTERN_REPS, cnt); + fflush (stdout); + this_subtest_failed = true; + failed_subtests++; + } + break; + + default: + printf ("ERROR: %s/%s: invalid value for 'expected' = %d\n", + label, stage, (int)expected); + fflush (stdout); + this_subtest_failed = true; + failed_subtests++; + } +} + +/* Always check the test buffer immediately after filling it; this + makes externally visible side effects depend on the buffer existing + and having been filled in. */ +static void +prepare_test_buffer (unsigned char *buf, const char *label) +{ + fill_with_test_pattern (buf); + check_test_buffer (EXPECT_ALL, label, "prepare"); + + unsigned char *loc = memmem (signal_stack_buffer, SIGNAL_STACK_SIZE, + test_pattern, PATTERN_SIZE); + if (loc == buf) + { + printf ("PASS: %s/prepare: expected buffer location %p got %p\n", + label, buf, loc); + fflush (stdout); + } + else + { + printf ("FAIL: %s/prepare: expected buffer location %p got %p\n", + label, buf, loc); + fflush (stdout); + this_subtest_failed = true; + failed_subtests++; + } +} + +/* There are three subtests, two of which are sanity checks. + + In the "no_clear" case, we don't do anything to the test buffer + between preparing it and letting it go out of scope, and we expect + to find it. This confirms that the test buffer does get filled in + and we can find it from the stack buffer. In the "ordinary_clear" + case, we clear it using memset, and we expect to find it. This + confirms that the compiler can optimize out block clears in this + context; if it can't, the real test might be succeeding for the + wrong reason. Finally, the "explicit_clear" case uses + explicit_bzero and expects _not_ to find the test buffer, which is + the real test. */ + +static void +setup_no_clear (void) +{ + unsigned char buf[TEST_BUFFER_SIZE]; + prepare_test_buffer (buf, "no clear"); +} + +static void +setup_ordinary_clear (void) +{ + unsigned char buf[TEST_BUFFER_SIZE]; + prepare_test_buffer (buf, "ordinary clear"); + if (this_subtest_failed) + return; + memset (buf, 0, TEST_BUFFER_SIZE); +} + +static void +setup_explicit_clear (void) +{ + unsigned char buf[TEST_BUFFER_SIZE]; + prepare_test_buffer (buf, "explicit clear"); + if (this_subtest_failed) + return; + explicit_bzero (buf, TEST_BUFFER_SIZE); +} + +struct subtest +{ + void (*setup_subtest) (void); + const char *label; + enum test_expectation expected; +}; + +static const struct subtest subtests[] = +{ + { setup_no_clear, "no clear", EXPECT_SOME }, + { setup_ordinary_clear, "ordinary clear", EXPECT_SOME }, + { setup_explicit_clear, "explicit clear", EXPECT_NONE }, + { 0, 0, -1 } +}; +static const struct subtest *this_subtest; +static bool this_subtest_complete; + +/* This function is called as a signal handler. The signal is + triggered by a call to raise, therefore it is safe to do + non-async-signal-safe things within this function. + + The this_subtest_complete flag addresses a race. The outer loop + keeps SIGUSR1 blocked all the time, unblocking it only immediately + after setting up the appropriate conditions for a test and then + raising SIGUSR1 itself. SIGUSR1 is not a real-time signal, so if + another process sends this process SIGUSR1 _before_ it's unblocked + by the outer loop, this function will only be called once. + However, if another process sends this process SIGUSR1 _while this + handler is already running_, that signal will be pending upon + return from this function, and will fire before the outer loop has + a chance to re-block SIGUSR1. This is unavoidable; the workaround + is to arrange for this function not to do anything if it's called + several times in a row. */ +static void +do_subtest (int signo __attribute__ ((unused))) +{ + if (!this_subtest_complete) + { + this_subtest->setup_subtest (); + if (!this_subtest_failed) + check_test_buffer (this_subtest->expected, + this_subtest->label, + "test"); + this_subtest_complete = true; + } +} + +static int +do_test (void) +{ + /* test-skeleton.c unconditionally sets stdout to be unbuffered. + vfprintf allocates a great deal of memory on the stack if called + with an unbuffered FILE*, overflowing the alt-stack. This is + also why there is a call to fflush after every call to printf in + this file. */ + if (setvbuf (stdout, 0, _IOFBF, 0)) + { + printf ("ERROR: restoring stdout buffering: %s\n", strerror (errno)); + fflush (stdout); + return 2; + } + + size_t page_alignment = sysconf (_SC_PAGESIZE); + if (page_alignment < sizeof (void *)) + page_alignment = sizeof (void *); + + void *p; + int err = posix_memalign (&p, page_alignment, SIGNAL_STACK_SIZE); + if (err || !p) + { + printf ("ERROR: allocating alt stack: %s\n", strerror (err)); + fflush (stdout); + return 2; + } + signal_stack_buffer = p; + + /* This program will malfunction if it receives SIGUSR1 signals at any + time other than when it's just sent one to itself. Therefore, keep + it blocked most of the time. */ + sigset_t sigusr1_mask; + sigemptyset (&sigusr1_mask); + sigaddset (&sigusr1_mask, SIGUSR1); + if (sigprocmask (SIG_BLOCK, &sigusr1_mask, 0)) + { + printf ("ERROR: block(SIGUSR1): %s\n", strerror (errno)); + fflush (stdout); + return 2; + } + + stack_t ss; + ss.ss_sp = signal_stack_buffer; + ss.ss_flags = 0; + ss.ss_size = SIGNAL_STACK_SIZE; + if (sigaltstack (&ss, 0)) + { + printf ("ERROR: sigaltstack: %s\n", strerror (errno)); + fflush (stdout); + return 2; + } + + struct sigaction sa; + sigemptyset (&sa.sa_mask); + sigaddset (&sa.sa_mask, SIGUSR1); + sa.sa_handler = do_subtest; + sa.sa_flags = SA_RESTART | SA_ONSTACK; + if (sigaction (SIGUSR1, &sa, 0)) + { + printf ("ERROR: sigaction(SIGUSR1): %s\n", strerror (errno)); + fflush (stdout); + return 2; + } + + /* We use kill instead of raise, because raise screws with the + signal mask, which we don't want. */ + pid_t self = getpid (); + + this_subtest = subtests; + while (this_subtest->label) + { + this_subtest_complete = false; + this_subtest_failed = false; + + /* Completely clear the signal stack between tests, so that junk + from previous tests cannot interfere with the current one. */ + memset (signal_stack_buffer, 0, SIGNAL_STACK_SIZE); + + /* Raise SIGUSR1, then unblock it, then immediately block it + again. This will trigger do_subtest to run _once_ on the + alternative stack, at the point of calling sigprocmask to + unblock the signal. */ + if (kill (self, SIGUSR1)) + { + printf ("ERROR: raise(SIGUSR1): %s\n", strerror (errno)); + fflush (stdout); + return 2; + } + if (sigprocmask (SIG_UNBLOCK, &sigusr1_mask, 0)) + { + printf ("ERROR: unblock(SIGUSR1): %s\n", strerror (errno)); + fflush (stdout); + return 2; + } + if (sigprocmask (SIG_BLOCK, &sigusr1_mask, 0)) + { + printf ("ERROR: unblock(SIGUSR1): %s\n", strerror(errno)); + fflush (stdout); + return 2; + } + + this_subtest++; + } + + return failed_subtests ? 1 : 0; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" -- 2.9.3