This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [patch] Refactor wordexp-test
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Paul Pluzhnikov <ppluzhnikov at gmail dot com>, GLIBC Devel <libc-alpha at sourceware dot org>
- Date: Sun, 08 Mar 2015 15:03:14 -0400
- Subject: Re: [patch] Refactor wordexp-test
- Authentication-results: sourceware.org; auth=none
- References: <CALoOobPa9ZqLJ=YaG3PJ10py51G+juH_QED6xmcfDhMNKACkHw at mail dot gmail dot com>
On 03/06/2015 08:06 PM, Paul Pluzhnikov wrote:
> Greetings,
>
> This patch modifies wordexp-test.c such that words always ends at the
> edge of unreadable page.
>
> This makes it easy to catch overflows, such as BZ #18043 (and BZ #18042).
>
> Tested: reverted fix for BZ #18043 in wordexp.c and verified that the
> test for it fails with expected SIGSEGV.
>
>
> Thanks,
>
> 2015-03-06 Paul Pluzhnikov <ppluzhnikov@google.com>
>
> * posix/wordexp-test.c (test_case): Add test for BZ #18043
> (do_bz18043): Delete.
> (at_page_end): New.
> (testit): Refactor to have words at the edge of unreadable page.
Amazing.
OK to commit if you fix the nit e.g. use PTR_ALIGN_DOWN or ALIGN_DOWN
to avoid the bespoke alignment code.
> -- Paul Pluzhnikov
>
>
> glibc-20150306-bz18043-refactor.patch.txt
>
>
> diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
> index 137044e..b71352d 100644
> --- a/posix/wordexp-test.c
> +++ b/posix/wordexp-test.c
> @@ -233,6 +233,8 @@ struct test_case_struct
> { WRDE_CMDSUB, NULL, "$((1+`echo 1`))", WRDE_NOCMD, 0, { NULL, }, IFS },
> { WRDE_CMDSUB, NULL, "$((1+$((`echo 1`))))", WRDE_NOCMD, 0, { NULL, }, IFS },
>
> + { WRDE_SYNTAX, NULL, "${", 0, 0, { NULL, }, IFS }, /* BZ 18043 */
> +
OK.
> { -1, NULL, NULL, 0, 0, { NULL, }, IFS },
> };
>
> @@ -250,33 +252,6 @@ command_line_test (const char *words)
> printf ("we_wordv[%d] = \"%s\"\n", i, we.we_wordv[i]);
> }
>
> -static int
> -do_bz18043 (void)
> -{
> - const int pagesize = getpagesize ();
> - char *start = mmap (0, 2 * pagesize, PROT_READ|PROT_WRITE,
> - MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
> -
> - if (start == MAP_FAILED)
> - return 1;
> -
> - if (mprotect (start + pagesize, pagesize, PROT_NONE))
> - return 2;
> -
> - const char word[] = "${";
> - char *word_start = start + pagesize - sizeof (word);
> - memcpy (word_start, word, sizeof (word));
> -
> - wordexp_t w;
> - if (wordexp (word_start, &w, 0) != WRDE_SYNTAX)
> - return 3;
> -
> - if (munmap (start, 2 * pagesize) != 0)
> - return 4;
> -
> - return 0;
> -}
> -
> int
> main (int argc, char *argv[])
> {
> @@ -398,12 +373,32 @@ main (int argc, char *argv[])
>
> printf ("tests failed: %d\n", fail);
>
> - if (do_bz18043 ())
> - ++fail;
> -
> return fail != 0;
> }
>
> +static const char *
> +at_page_end (const char *words)
> +{
> + const int pagesize = getpagesize ();
> + char *start = mmap (0, 2 * pagesize, PROT_READ|PROT_WRITE,
> + MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
> +
> + if (start == MAP_FAILED)
> + return start;
> +
> + if (mprotect (start + pagesize, pagesize, PROT_NONE))
> + {
> + munmap (start, 2 * pagesize);
> + return MAP_FAILED;
> + }
> +
> + /* Includes terminating NUL. */
> + const size_t words_size = strlen (words) + 1;
> + char *words_start = start + pagesize - words_size;
> + memcpy (words_start, words, words_size);
> +
> + return words_start;
> +}
OK.
>
> static int
> testit (struct test_case_struct *tc)
> @@ -431,6 +426,8 @@ testit (struct test_case_struct *tc)
> we = sav_we;
>
> printf ("Test %d (%s): ", ++tests, tc->words);
> + fflush (NULL);
OK. Good idea to flush before we potentially crash.
> + const char *words = at_page_end (tc->words);
>
> if (tc->flags & WRDE_NOCMD)
> registered_forks = 0;
> @@ -444,7 +441,7 @@ testit (struct test_case_struct *tc)
> return 1;
> }
> }
> - retval = wordexp (tc->words, &we, tc->flags);
> + retval = wordexp (words, &we, tc->flags);
OK. Using a copy.
>
> if ((tc->flags & WRDE_NOCMD)
> && (registered_forks > 0))
> @@ -508,5 +505,11 @@ testit (struct test_case_struct *tc)
> if (retval == 0 || retval == WRDE_NOSPACE)
> wordfree (&we);
>
> + const int page_size = getpagesize ();
> + char *start = (char *) (((uintptr_t) words) & ~(page_size - 1));
#include <libc-internal>, use PTR_ALIGN_DOWN().
> + if (munmap (start, 2 * page_size) != 0)
> + return 1;
> +
> + fflush (NULL);
> return bzzzt;
> }
Cheers,
Carlos.