Message ID | 87h80uqsv4.fsf@oldenburg2.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | Remove incorrect alloc_size attribute from pvalloc [BZ #25401] | expand |
On 17/01/2020 07:28, Florian Weimer wrote: > pvalloc is guarantueed to round up the allocation size to the page > size, so applications can assume that the memory region is larger > than the passed-in argument. The alloc_size attribute cannot express > that. > > The test case is based on a suggestion from Jakub Jelinek. > > This fixes commit 9bf8e29ca136094f73f69f725f15c51facc97206 ("malloc: > make malloc fail with requests larger than PTRDIFF_MAX (BZ#23741)"). LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > ----- > malloc/Makefile | 2 +- > malloc/malloc.h | 3 +-- > malloc/tst-pvalloc-fortify.c | 48 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 50 insertions(+), 3 deletions(-) > > diff --git a/malloc/Makefile b/malloc/Makefile > index 734efe368d..984045b5b9 100644 > --- a/malloc/Makefile > +++ b/malloc/Makefile > @@ -27,7 +27,7 @@ headers := $(dist-headers) obstack.h mcheck.h > tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ > tst-mcheck tst-mallocfork tst-trim1 \ > tst-malloc-usable tst-realloc tst-reallocarray tst-posix_memalign \ > - tst-pvalloc tst-memalign tst-mallopt \ > + tst-pvalloc tst-pvalloc-fortify tst-memalign tst-mallopt \ > tst-malloc-backtrace tst-malloc-thread-exit \ > tst-malloc-thread-fail tst-malloc-fork-deadlock \ > tst-mallocfork2 \ Ok. > diff --git a/malloc/malloc.h b/malloc/malloc.h > index 0c76264421..a6903fdd54 100644 > --- a/malloc/malloc.h > +++ b/malloc/malloc.h > @@ -71,8 +71,7 @@ extern void *valloc (size_t __size) __THROW __attribute_malloc__ > > /* Equivalent to valloc(minimum-page-that-holds(n)), that is, round up > __size to nearest pagesize. */ > -extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ > - __attribute_alloc_size__ ((1)) __wur; > +extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ __wur; > > /* Underlying allocation function; successive calls should return > contiguous pieces of memory. */ Ok. > diff --git a/malloc/tst-pvalloc-fortify.c b/malloc/tst-pvalloc-fortify.c > new file mode 100644 > index 0000000000..391b7fa2f5 > --- /dev/null > +++ b/malloc/tst-pvalloc-fortify.c > @@ -0,0 +1,48 @@ > +/* Test fortify-source allocation size handling in pvalloc (bug 25401). > + Copyright (C) 2020 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; see the file COPYING.LIB. If > + not, see <https://www.gnu.org/licenses/>. */ > + > +#undef _FORTIFY_SOURCE > +#define _FORTIFY_SOURCE 2 > +#include <malloc.h> > +#include <string.h> > +#include <support/check.h> > +#include <support/xunistd.h> > +#include <unistd.h> > + > +static int > +do_test (void) > +{ > + /* The test below assumes that pvalloc rounds up the allocation size > + to at least 8. */ > + TEST_VERIFY (xsysconf (_SC_PAGESIZE) >= 8); > + > + void *p = pvalloc (5); > + TEST_VERIFY_EXIT (p != NULL); > + > + /* This is valid assuming the page size is at least 8 because > + pvalloc rounds up the allocation size to a multiple of the page > + size. Due to bug 25041, this used to trigger a compiler > + warning. */ > + strcpy (p, "abcdefg"); > + > + asm ("" : : "g" (p) : "memory"); /* Optimization barrier. */ > + TEST_VERIFY (malloc_usable_size (p) >= xsysconf (_SC_PAGESIZE)); > + return 0; > +} > + > +#include <support/test-driver.c> > Ok.
* Adhemerval Zanella: > On 17/01/2020 07:28, Florian Weimer wrote: >> pvalloc is guarantueed to round up the allocation size to the page >> size, so applications can assume that the memory region is larger >> than the passed-in argument. The alloc_size attribute cannot express >> that. >> >> The test case is based on a suggestion from Jakub Jelinek. >> >> This fixes commit 9bf8e29ca136094f73f69f725f15c51facc97206 ("malloc: >> make malloc fail with requests larger than PTRDIFF_MAX (BZ#23741)"). > > LGTM, thanks. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Thanks. Siddhesh, should we put this into the release? Florian
On 17/01/20 6:44 pm, Florian Weimer wrote: >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > Thanks. > > Siddhesh, should we put this into the release? > Yes please. Well contained bug fixes are generally OK during freeze; this one qualifies. Thanks, Siddhesh
diff --git a/malloc/Makefile b/malloc/Makefile index 734efe368d..984045b5b9 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -27,7 +27,7 @@ headers := $(dist-headers) obstack.h mcheck.h tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-mcheck tst-mallocfork tst-trim1 \ tst-malloc-usable tst-realloc tst-reallocarray tst-posix_memalign \ - tst-pvalloc tst-memalign tst-mallopt \ + tst-pvalloc tst-pvalloc-fortify tst-memalign tst-mallopt \ tst-malloc-backtrace tst-malloc-thread-exit \ tst-malloc-thread-fail tst-malloc-fork-deadlock \ tst-mallocfork2 \ diff --git a/malloc/malloc.h b/malloc/malloc.h index 0c76264421..a6903fdd54 100644 --- a/malloc/malloc.h +++ b/malloc/malloc.h @@ -71,8 +71,7 @@ extern void *valloc (size_t __size) __THROW __attribute_malloc__ /* Equivalent to valloc(minimum-page-that-holds(n)), that is, round up __size to nearest pagesize. */ -extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ - __attribute_alloc_size__ ((1)) __wur; +extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ __wur; /* Underlying allocation function; successive calls should return contiguous pieces of memory. */ diff --git a/malloc/tst-pvalloc-fortify.c b/malloc/tst-pvalloc-fortify.c new file mode 100644 index 0000000000..391b7fa2f5 --- /dev/null +++ b/malloc/tst-pvalloc-fortify.c @@ -0,0 +1,48 @@ +/* Test fortify-source allocation size handling in pvalloc (bug 25401). + Copyright (C) 2020 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; see the file COPYING.LIB. If + not, see <https://www.gnu.org/licenses/>. */ + +#undef _FORTIFY_SOURCE +#define _FORTIFY_SOURCE 2 +#include <malloc.h> +#include <string.h> +#include <support/check.h> +#include <support/xunistd.h> +#include <unistd.h> + +static int +do_test (void) +{ + /* The test below assumes that pvalloc rounds up the allocation size + to at least 8. */ + TEST_VERIFY (xsysconf (_SC_PAGESIZE) >= 8); + + void *p = pvalloc (5); + TEST_VERIFY_EXIT (p != NULL); + + /* This is valid assuming the page size is at least 8 because + pvalloc rounds up the allocation size to a multiple of the page + size. Due to bug 25041, this used to trigger a compiler + warning. */ + strcpy (p, "abcdefg"); + + asm ("" : : "g" (p) : "memory"); /* Optimization barrier. */ + TEST_VERIFY (malloc_usable_size (p) >= xsysconf (_SC_PAGESIZE)); + return 0; +} + +#include <support/test-driver.c>