Message ID | CALoOobPa9ZqLJ=YaG3PJ10py51G+juH_QED6xmcfDhMNKACkHw@mail.gmail.com |
---|---|
State | New |
Headers | show |
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.
On Sun, Mar 8, 2015 at 12:03 PM, Carlos O'Donell <carlos@redhat.com> wrote: > OK to commit if you fix the nit e.g. use PTR_ALIGN_DOWN or ALIGN_DOWN > to avoid the bespoke alignment code. Thanks. Committed as 36103ba2f5db530bff24896dfc9076955fba3b5f.
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 */ + { -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; +} 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); + 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); 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)); + if (munmap (start, 2 * page_size) != 0) + return 1; + + fflush (NULL); return bzzzt; }