Message ID | 20201220202556.3714-1-eggert@cs.ucla.edu |
---|---|
State | New |
Headers | show |
Series | free: preserve errno [BZ#17924] | expand |
On 12/21/20 1:55 AM, Paul Eggert wrote: > * malloc/Makefile (tests): Add tst-free-errno. > * malloc/malloc.c (tcache_init): Preserve errno when initializing, > since 'free' might be calling us. > (__libc_free): Preserve errno when calling munmap. > * malloc/tst-free-errno.c: New file, almost all from Bruno Haible. > * manual/memory.texi (Freeing after Malloc, Replacing malloc): > Document that free preserves errno. > --- > malloc/Makefile | 1 + > malloc/malloc.c | 6 ++ > malloc/tst-free-errno.c | 169 ++++++++++++++++++++++++++++++++++++++++ > manual/memory.texi | 9 +++ > 4 files changed, 185 insertions(+) > create mode 100644 malloc/tst-free-errno.c > > diff --git a/malloc/Makefile b/malloc/Makefile > index ab64dcfd73..4b3975f90d 100644 > --- a/malloc/Makefile > +++ b/malloc/Makefile > @@ -34,6 +34,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ > tst-interpose-nothread \ > tst-interpose-thread \ > tst-alloc_buffer \ > + tst-free-errno \ > tst-malloc-tcache-leak \ > tst-malloc_info tst-mallinfo2 \ > tst-malloc-too-large \ > diff --git a/malloc/malloc.c b/malloc/malloc.c > index 326075e704..14bc55f96d 100644 > --- a/malloc/malloc.c > +++ b/malloc/malloc.c > @@ -3003,6 +3003,8 @@ tcache_init(void) > if (tcache_shutting_down) > return; > > + int err = errno; > + > arena_get (ar_ptr, bytes); > victim = _int_malloc (ar_ptr, bytes); > if (!victim && ar_ptr != NULL) > @@ -3015,6 +3017,8 @@ tcache_init(void) > if (ar_ptr != NULL) > __libc_lock_unlock (ar_ptr->mutex); > > + __set_errno (err); > + > /* In a low memory situation, we may not be able to allocate memory > - in which case, we just keep trying later. However, we > typically do this very early, so either there is sufficient > @@ -3140,7 +3144,9 @@ __libc_free (void *mem) > LIBC_PROBE (memory_mallopt_free_dyn_thresholds, 2, > mp_.mmap_threshold, mp_.trim_threshold); > } > + int err = errno; > munmap_chunk (p); > + __set_errno (err); > return; > } The _int_free call after this may trigger a trim, which could result in an mmap, madvise or brk, all of which set errno. In practice I think only mmap may do this but it's reason enough to perhaps just save errno at the top and restore it on exit. The mmap is called when /proc/sys/vm/overcommit_memory has 2. > > diff --git a/malloc/tst-free-errno.c b/malloc/tst-free-errno.c > new file mode 100644 > index 0000000000..6243cb6e0b > --- /dev/null > +++ b/malloc/tst-free-errno.c > @@ -0,0 +1,169 @@ > +/* Test that free preserves errno. > + 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; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +/* Written by Bruno Haible <bruno@clisp.org>, 2020. */ We don't write these anymore. > + > +#include <errno.h> > +#include <stdlib.h> > +#include <string.h> > +#include <unistd.h> > +#if defined __linux__ > +# include <fcntl.h> > +# include <stdint.h> > +# include <string.h> > +# include <sys/mman.h> > +#endif > + > +#define ASSERT_NO_STDIO(expr) \ > + do \ > + { \ > + if (!(expr)) \ > + { \ > + WRITE_TO_STDERR (__FILE__); \ > + WRITE_TO_STDERR (":"); \ > + WRITE_MACROEXPANDED_INTEGER_TO_STDERR (__LINE__); \ > + WRITE_TO_STDERR (": assertion '"); \ > + WRITE_TO_STDERR (#expr); \ > + WRITE_TO_STDERR ("' failed\n"); \ > + abort (); \ > + } \ > + } \ > + while (0) > +#define WRITE_MACROEXPANDED_INTEGER_TO_STDERR(integer) \ > + WRITE_INTEGER_TO_STDERR(integer) > +#define WRITE_INTEGER_TO_STDERR(integer) \ > + WRITE_TO_STDERR (#integer) > +#define WRITE_TO_STDERR(string_literal) \ > + { \ > + const char *s = string_literal; \ > + int ret = write (2, s, strlen (s)); \ > + (void) ret; \ > + } > + > +/* The indirection through a volatile function pointer is necessary to prevent > + a GCC optimization. Without it, when optimizing, GCC would "know" that errno > + is unchanged by calling free(ptr), when ptr was the result of a malloc(...) > + call in the same function. */ > +static int > +get_errno (void) > +{ > + volatile int err = errno; > + return err; > +} > + > +static int (* volatile get_errno_func) (void) = get_errno; > + > +static int > +do_test (void) > +{ > + /* Check that free() preserves errno. */ > + { > + errno = 1789; /* Liberté, égalité, fraternité. */ > + free (NULL); > + ASSERT_NO_STDIO (get_errno_func () == 1789); > + } > + { /* Large memory allocations. */ > + #define N 2 > + void * volatile ptrs[N]; > + size_t i; > + for (i = 0; i < N; i++) > + ptrs[i] = malloc (5318153); > + for (i = 0; i < N; i++) > + { > + errno = 1789; > + free (ptrs[i]); > + ASSERT_NO_STDIO (get_errno_func () == 1789); > + } > + #undef N > + } > + > + /* Test a less common code path. > + When malloc() is based on mmap(), free() can sometimes call munmap(). > + munmap() usually succeeds, but fails in a particular situation: when > + - it has to unmap the middle part of a VMA, and > + - the number of VMAs of a process is limited and the limit is > + already reached. > + The latter condition is fulfilled on Linux, when the file > + /proc/sys/vm/max_map_count exists. This file contains the limit > + - for Linux >= 2.4.19: 65536 (DEFAULT_MAX_MAP_COUNT in linux/include/linux/sched.h) > + - for Linux >= 2.6.31: 65530 (DEFAULT_MAX_MAP_COUNT in linux/include/linux/mm.h). > + */ > + #if defined __linux__ > + if (open ("/proc/sys/vm/max_map_count", O_RDONLY) >= 0) > + { > + /* Preparations. */ > + size_t pagesize = getpagesize (); > + void *firstpage_backup = malloc (pagesize); > + void *lastpage_backup = malloc (pagesize); > + /* Allocate a large memory area, as a bumper, so that the MAP_FIXED > + allocation later will not overwrite parts of the memory areas > + allocated to ld.so or libc.so. */ > + void *bumper_region = > + mmap (NULL, 0x1000000, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); > + /* A file descriptor pointing to a regular file. */ > + int fd = open ("/etc/hosts", O_RDONLY); > + > + if (firstpage_backup != NULL && lastpage_backup != NULL > + && bumper_region != (void *)(-1) > + && fd >= 0) > + { > + /* Do a large memory allocation. */ > + size_t big_size = 0x1000000; > + void * volatile ptr = malloc (big_size - 0x100); > + char *ptr_aligned = (char *) ((uintptr_t) ptr & ~(pagesize - 1)); > + /* This large memory allocation allocated a memory area > + from ptr_aligned to ptr_aligned + big_size. > + Enlarge this memory area by adding a page before and a page > + after it. */ > + memcpy (firstpage_backup, ptr_aligned, pagesize); > + memcpy (lastpage_backup, ptr_aligned + big_size - pagesize, pagesize); > + if (mmap (ptr_aligned - pagesize, pagesize + big_size + pagesize, > + PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0) > + != (void *)(-1)) > + { > + memcpy (ptr_aligned, firstpage_backup, pagesize); > + memcpy (ptr_aligned + big_size - pagesize, lastpage_backup, pagesize); > + > + /* Now add as many mappings as we can. > + Stop at 65536, in order not to crash the machine (in case the > + limit has been increased by the system administrator). */ > + size_t i; > + for (i = 0; i < 65536; i++) > + if (mmap (NULL, pagesize, PROT_READ, MAP_FILE | MAP_PRIVATE, fd, 0) > + == (void *)(-1)) > + break; > + /* Now the number of VMAs of this process has hopefully attained > + its limit. */ > + > + errno = 1789; > + /* This call to free() is supposed to call > + munmap (ptr_aligned, big_size); > + which increases the number of VMAs by 1, which is supposed > + to fail. */ > + free (ptr); > + ASSERT_NO_STDIO (get_errno_func () == 1789); > + } > + } > + } > + #endif > + > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/manual/memory.texi b/manual/memory.texi > index c132261084..b2cc65228a 100644 > --- a/manual/memory.texi > +++ b/manual/memory.texi > @@ -738,6 +738,12 @@ later call to @code{malloc} to reuse the space. In the meantime, the > space remains in your program as part of a free-list used internally by > @code{malloc}. > > +The @code{free} function preserves the value of @code{errno}, so that > +cleanup code need not worry about saving and restoring @code{errno} > +around a call to @code{free}. Although neither @w{ISO C} nor > +POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future > +version of POSIX is planned to require it. > + > There is no point in freeing blocks at the end of a program, because all > of the program's space is given back to the system when the process > terminates. > @@ -1935,6 +1941,9 @@ linking against @code{libc.a} (explicitly or implicitly). > functions (that is, all the functions used by the application, > @theglibc{}, and other linked-in libraries) can lead to static linking > failures, and, at run time, to heap corruption and application crashes. > +Replacement functions should implement the behavior documented for > +their counterparts in @theglibc{}; for example, the replacement > +@code{free} should also preserve @code{errno}. > > The minimum set of functions which has to be provided by a custom > @code{malloc} is given in the table below. >
On 12/20/20 3:25 PM, Paul Eggert wrote: > * malloc/Makefile (tests): Add tst-free-errno. > * malloc/malloc.c (tcache_init): Preserve errno when initializing, > since 'free' might be calling us. > (__libc_free): Preserve errno when calling munmap. > * malloc/tst-free-errno.c: New file, almost all from Bruno Haible. > * manual/memory.texi (Freeing after Malloc, Replacing malloc): > Document that free preserves errno. Please provide a reasonable commit message.
On 12/20/20 8:27 PM, Carlos O'Donell wrote:
> Please provide a reasonable commit message.
Thanks, unfortunately I slipped up and used the old-style commits still
common in other projects. Revised patch attached. This revision also
addresses Siddhesh's comments (he found another way errno could be munged).
On 12/21/20 12:50 PM, Paul Eggert wrote: > From e4fec6f28270c9b0979f9fe8920dc52f8f7d70cd Mon Sep 17 00:00:00 2001 > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Sat, 19 Dec 2020 12:52:09 -0800 > Subject: [PATCH] free: preserve errno [BZ#17924] > > In the next release of POSIX, free must preserve errno > <https://www.austingroupbugs.net/view.php?id=385>. > Modify __libc_free to save and restore errno, so that > any internal munmap etc. syscalls do not disturb the caller's errno. > Add a test malloc/tst-free-errno.c (almost all by Bruno Haible), > and document that free preserves errno. Thanks, this version looks good to me. Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
* Paul Eggert: > +/* The indirection through a volatile function pointer is necessary to prevent > + a GCC optimization. Without it, when optimizing, GCC would "know" that errno > + is unchanged by calling free(ptr), when ptr was the result of a malloc(...) > + call in the same function. */ > +static int > +get_errno (void) Long line (80 characters). I think you need to use this: int __attribute__ ((weak)) get_errno (void) { return errno; } With that volatile construct, the read of errno is not actually behind a compiler barrier. Thanks, Florian
On 12/21/20 3:03 PM, Florian Weimer via Libc-alpha wrote: > * Paul Eggert: > >> +/* The indirection through a volatile function pointer is necessary to prevent >> + a GCC optimization. Without it, when optimizing, GCC would "know" that errno >> + is unchanged by calling free(ptr), when ptr was the result of a malloc(...) >> + call in the same function. */ >> +static int >> +get_errno (void) > > Long line (80 characters). > > I think you need to use this: > > int __attribute__ ((weak)) > get_errno (void) > { > return errno; > } > > With that volatile construct, the read of errno is not actually behind a > compiler barrier. Wouldn't it be a bug in the compiler if it delays load of the TLS var beyond a function call boundary? Siddhesh
On 12/21/20 3:33 PM, Siddhesh Poyarekar wrote: > Wouldn't it be a bug in the compiler if it delays load of the TLS var > beyond a function call boundary? Sorry scratch that, I wasn't thinking straight. Siddhesh
Thanks for the comment about the test case's compiler barrier, and about the test case's too-long lines. I fixed those, and also fixed the BZ# in the Subject line. Revised patch attached.
On 23/12/2020 02:30, Paul Eggert wrote: > Thanks for the comment about the test case's compiler barrier, and about the test case's too-long lines. I fixed those, and also fixed the BZ# in the Subject line. Revised patch attached. > From a994c467ce04abfe102809812156cb30810eaa95 Mon Sep 17 00:00:00 2001 > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Tue, 22 Dec 2020 21:17:20 -0800 > Subject: [PATCH] free: preserve errno [BZ#17924] > > In the next release of POSIX, free must preserve errno > <https://www.austingroupbugs.net/view.php?id=385>. > Modify __libc_free to save and restore errno, so that > any internal munmap etc. syscalls do not disturb the caller's errno. > Add a test malloc/tst-free-errno.c (almost all by Bruno Haible), > and document that free preserves errno. > --- > malloc/Makefile | 1 + > malloc/malloc.c | 13 +++- > malloc/tst-free-errno.c | 168 ++++++++++++++++++++++++++++++++++++++++ > manual/memory.texi | 9 +++ > 4 files changed, 187 insertions(+), 4 deletions(-) > create mode 100644 malloc/tst-free-errno.c > > diff --git a/malloc/Makefile b/malloc/Makefile > index ab64dcfd73..4b3975f90d 100644 > --- a/malloc/Makefile > +++ b/malloc/Makefile > @@ -34,6 +34,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ > tst-interpose-nothread \ > tst-interpose-thread \ > tst-alloc_buffer \ > + tst-free-errno \ > tst-malloc-tcache-leak \ > tst-malloc_info tst-mallinfo2 \ > tst-malloc-too-large \ > diff --git a/malloc/malloc.c b/malloc/malloc.c > index a3e914fa8a..3b151f44f7 100644 > --- a/malloc/malloc.c > +++ b/malloc/malloc.c > @@ -3278,6 +3278,8 @@ __libc_free (void *mem) > *(volatile char *)mem; > #endif > > + int err = errno; > + > p = mem2chunk (mem); > > /* Mark the chunk as belonging to the library again. */ > @@ -3298,13 +3300,16 @@ __libc_free (void *mem) > mp_.mmap_threshold, mp_.trim_threshold); > } > munmap_chunk (p); > - return; > } > + else > + { > + MAYBE_INIT_TCACHE (); > > - MAYBE_INIT_TCACHE (); > + ar_ptr = arena_for_chunk (p); > + _int_free (ar_ptr, p, 0); > + } > > - ar_ptr = arena_for_chunk (p); > - _int_free (ar_ptr, p, 0); > + __set_errno (err); > } > libc_hidden_def (__libc_free) > I am not very found on adding a errno set/restore on *every* free, specially because it is extra overhead on small allocations that won't touch mmap/munmap (specially for tcache case). Fixing in a more fine grained would require a lot of more work to check if the shared routines that calls mmap, madvise or brk won't interfere with other symbols; so maybe it should be ok to use this large hammer for now. > diff --git a/malloc/tst-free-errno.c b/malloc/tst-free-errno.c > new file mode 100644 > index 0000000000..a612bbb3bf > --- /dev/null > +++ b/malloc/tst-free-errno.c > @@ -0,0 +1,168 @@ > +/* Test that free preserves errno. > + 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; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <stdlib.h> > +#include <string.h> > +#include <unistd.h> > +#if defined __linux__ > +# include <fcntl.h> > +# include <stdint.h> > +# include <string.h> > +# include <sys/mman.h> > +#endif Why do you need to include this solely for Linux? I think all of the headers should be provided by glibc on all supported systems. > + > +#define ASSERT_NO_STDIO(expr) \ > + do \ > + { \ > + if (!(expr)) \ > + { \ > + WRITE_TO_STDERR (__FILE__); \ > + WRITE_TO_STDERR (":"); \ > + WRITE_MACROEXPANDED_INTEGER_TO_STDERR (__LINE__); \ > + WRITE_TO_STDERR (": assertion '"); \ > + WRITE_TO_STDERR (#expr); \ > + WRITE_TO_STDERR ("' failed\n"); \ > + abort (); \ > + } \ > + } \ > + while (0) > +#define WRITE_MACROEXPANDED_INTEGER_TO_STDERR(integer) \ > + WRITE_INTEGER_TO_STDERR(integer) > +#define WRITE_INTEGER_TO_STDERR(integer) \ > + WRITE_TO_STDERR (#integer) > +#define WRITE_TO_STDERR(string_literal) \ > + { \ > + const char *s = string_literal; \ > + int ret = write (2, s, strlen (s)); \ > + (void) ret; \ > + } Why can't you use support/check.h instead? > + > +/* The __attribute__ ((weak)) prevents a GCC optimization. Without > + it, GCC would "know" that errno is unchanged by calling free (ptr), > + when ptr was the result of a malloc call in the same function. */ > +int __attribute__ ((weak)) > +get_errno (void) > +{ > + return errno; > +} > + > +static int > +do_test (void) > +{ > + /* Check that free() preserves errno. */ > + { > + errno = 1789; /* Liberté, égalité, fraternité. */ > + free (NULL); > + ASSERT_NO_STDIO (get_errno () == 1789); > + } > + { /* Large memory allocations. */ Maybe add a comment that it is forcing a mmap with the large allocation. > + #define N 2 > + void * volatile ptrs[N]; > + size_t i; > + for (i = 0; i < N; i++) > + ptrs[i] = malloc (5318153); > + for (i = 0; i < N; i++) > + { > + errno = 1789; > + free (ptrs[i]); > + ASSERT_NO_STDIO (get_errno () == 1789); > + } > + #undef N > + } No need to set/unset N here. > + > + /* Test a less common code path. > + When malloc() is based on mmap(), free() can sometimes call munmap(). > + munmap() usually succeeds, but fails in a particular situation: when > + - it has to unmap the middle part of a VMA, and > + - the number of VMAs of a process is limited and the limit is > + already reached. > + The latter condition is fulfilled on Linux, when the file > + /proc/sys/vm/max_map_count exists. This file contains the limit > + - for Linux >= 2.4.19: 65536 > + (DEFAULT_MAX_MAP_COUNT in linux/include/linux/sched.h) > + - for Linux >= 2.6.31: 65530 > + (DEFAULT_MAX_MAP_COUNT in linux/include/linux/mm.h). I think there is no need to reference old and unsupported kernel such as 2.4. > + */ > + #if defined __linux__ > + if (open ("/proc/sys/vm/max_map_count", O_RDONLY) >= 0) > + { > + /* Preparations. */ > + size_t pagesize = getpagesize (); > + void *firstpage_backup = malloc (pagesize); > + void *lastpage_backup = malloc (pagesize); Use xmalloc here, so there is no need to check if the allocation has succeded (they are small enough we can assume a failure means something wrong). > + /* Allocate a large memory area, as a bumper, so that the MAP_FIXED > + allocation later will not overwrite parts of the memory areas > + allocated to ld.so or libc.so. */ > + void *bumper_region = > + mmap (NULL, 0x1000000, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); > + /* A file descriptor pointing to a regular file. */ Use xmmap. > + int fd = open ("/etc/hosts", O_RDONLY); > + I think it is better to use a temporary file from support/create_temp_file.h. > + if (firstpage_backup != NULL && lastpage_backup != NULL > + && bumper_region != (void *)(-1) > + && fd >= 0) > + { > + /* Do a large memory allocation. */ > + size_t big_size = 0x1000000; > + void * volatile ptr = malloc (big_size - 0x100); User xmalloc. > + char *ptr_aligned = (char *) ((uintptr_t) ptr & ~(pagesize - 1)); > + /* This large memory allocation allocated a memory area > + from ptr_aligned to ptr_aligned + big_size. > + Enlarge this memory area by adding a page before and a page > + after it. */ > + memcpy (firstpage_backup, ptr_aligned, pagesize); > + memcpy (lastpage_backup, ptr_aligned + big_size - pagesize, > + pagesize); > + if (mmap (ptr_aligned - pagesize, pagesize + big_size + pagesize, > + PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0) > + != (void *)(-1)) Use xmmap. > + { > + memcpy (ptr_aligned, firstpage_backup, pagesize); > + memcpy (ptr_aligned + big_size - pagesize, lastpage_backup, > + pagesize); > + > + /* Now add as many mappings as we can. > + Stop at 65536, in order not to crash the machine (in case the > + limit has been increased by the system administrator). */ > + size_t i; > + for (i = 0; i < 65536; i++) > + if (mmap (NULL, pagesize, PROT_READ, > + MAP_FILE | MAP_PRIVATE, fd, 0) > + == (void *)(-1)) > + break; > + /* Now the number of VMAs of this process has hopefully attained > + its limit. */ > + > + errno = 1789; > + /* This call to free() is supposed to call > + munmap (ptr_aligned, big_size); > + which increases the number of VMAs by 1, which is supposed > + to fail. */ > + free (ptr); > + ASSERT_NO_STDIO (get_errno () == 1789); > + } > + } > + } > + #endif > + > + return 0; > +} > + > +#include <support/test-driver.c> Ok. > diff --git a/manual/memory.texi b/manual/memory.texi > index c132261084..b2cc65228a 100644 > --- a/manual/memory.texi > +++ b/manual/memory.texi > @@ -738,6 +738,12 @@ later call to @code{malloc} to reuse the space. In the meantime, the > space remains in your program as part of a free-list used internally by > @code{malloc}. > > +The @code{free} function preserves the value of @code{errno}, so that > +cleanup code need not worry about saving and restoring @code{errno} > +around a call to @code{free}. Although neither @w{ISO C} nor > +POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future > +version of POSIX is planned to require it. > + > There is no point in freeing blocks at the end of a program, because all > of the program's space is given back to the system when the process > terminates. Not sure if this is worth to add, since we will need to update the manual once the POSIX does require it. > @@ -1935,6 +1941,9 @@ linking against @code{libc.a} (explicitly or implicitly). > functions (that is, all the functions used by the application, > @theglibc{}, and other linked-in libraries) can lead to static linking > failures, and, at run time, to heap corruption and application crashes. > +Replacement functions should implement the behavior documented for > +their counterparts in @theglibc{}; for example, the replacement > +@code{free} should also preserve @code{errno}. > > The minimum set of functions which has to be provided by a custom > @code{malloc} is given in the table below. > -- > 2.29.2
Thanks for the comments about the patch's test case. I modified the test case to reflect nearly all the comments, resulting in the attached revised patch. I'm replying below only to the comments that didn't result in a change to the patch. On 12/23/20 11:19 AM, Adhemerval Zanella wrote: > Fixing in a more fine grained would require a lot of more work to check if > the shared routines that calls mmap, madvise or brk won't interfere with other > symbols; so maybe it should be ok to use this large hammer for now. Yes, that was my thought as well. >> +The @code{free} function preserves the value of @code{errno}, so that >> +cleanup code need not worry about saving and restoring @code{errno} >> +around a call to @code{free}. Although neither @w{ISO C} nor >> +POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future >> +version of POSIX is planned to require it. ... > Not sure if this is worth to add, since we will need to update the manual > once the POSIX does require it. I'll volunteer to update the manual. :-) It's worth mentioning that preserving errno is not something that portable C or POSIX code should assume for 'free'. If there's a better way for the manual to warn its readers about this, that'd be fine of course. I did consider changing "Although neither @w{ISO C} nor POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future version of POSIX is planned to require it" to "Neither @w{ISO C} nor POSIX requires @code{free} to preserve @code{errno}", but that wording would be less informative and would still need updating once POSIX does require 'free' to preserve errno.
On 23/12/2020 22:03, Paul Eggert wrote: > Thanks for the comments about the patch's test case. I modified the test case to reflect nearly all the comments, resulting in the attached revised patch. I'm replying below only to the comments that didn't result in a change to the patch. > > On 12/23/20 11:19 AM, Adhemerval Zanella wrote: > >> Fixing in a more fine grained would require a lot of more work to check if >> the shared routines that calls mmap, madvise or brk won't interfere with other >> symbols; so maybe it should be ok to use this large hammer for now. > > Yes, that was my thought as well. I will try to revise this for 2.33. > >>> +The @code{free} function preserves the value of @code{errno}, so that >>> +cleanup code need not worry about saving and restoring @code{errno} >>> +around a call to @code{free}. Although neither @w{ISO C} nor >>> +POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future >>> +version of POSIX is planned to require it. > ... >> Not sure if this is worth to add, since we will need to update the manual >> once the POSIX does require it. > > I'll volunteer to update the manual. :-) > > It's worth mentioning that preserving errno is not something that portable C or POSIX code should assume for 'free'. If there's a better way for the manual to warn its readers about this, that'd be fine of course. > > I did consider changing "Although neither @w{ISO C} nor POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future version of POSIX is planned to require it" to "Neither @w{ISO C} nor POSIX requires @code{free} to preserve @code{errno}", but that wording would be less informative and would still need updating once POSIX does require 'free' to preserve errno. Thanks. > From afbf4ff042cf3a5c8f983d5aa3bd0de3fb696dd3 Mon Sep 17 00:00:00 2001 > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Wed, 23 Dec 2020 11:27:25 -0800 > Subject: [PATCH] free: preserve errno [BZ#17924] > > In the next release of POSIX, free must preserve errno > <https://www.austingroupbugs.net/view.php?id=385>. > Modify __libc_free to save and restore errno, so that > any internal munmap etc. syscalls do not disturb the caller's errno. > Add a test malloc/tst-free-errno.c (almost all by Bruno Haible), > and document that free preserves errno. LGTM with the small test change below. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > malloc/Makefile | 1 + > malloc/malloc.c | 13 ++-- > malloc/tst-free-errno.c | 131 ++++++++++++++++++++++++++++++++++++++++ > manual/memory.texi | 9 +++ > 4 files changed, 150 insertions(+), 4 deletions(-) > create mode 100644 malloc/tst-free-errno.c > > diff --git a/malloc/Makefile b/malloc/Makefile > index ab64dcfd73..4b3975f90d 100644 > --- a/malloc/Makefile > +++ b/malloc/Makefile > @@ -34,6 +34,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ > tst-interpose-nothread \ > tst-interpose-thread \ > tst-alloc_buffer \ > + tst-free-errno \ > tst-malloc-tcache-leak \ > tst-malloc_info tst-mallinfo2 \ > tst-malloc-too-large \ > diff --git a/malloc/malloc.c b/malloc/malloc.c > index a3e914fa8a..3b151f44f7 100644 > --- a/malloc/malloc.c > +++ b/malloc/malloc.c > @@ -3278,6 +3278,8 @@ __libc_free (void *mem) > *(volatile char *)mem; > #endif > > + int err = errno; > + > p = mem2chunk (mem); > > /* Mark the chunk as belonging to the library again. */ > @@ -3298,13 +3300,16 @@ __libc_free (void *mem) > mp_.mmap_threshold, mp_.trim_threshold); > } > munmap_chunk (p); > - return; > } > + else > + { > + MAYBE_INIT_TCACHE (); > > - MAYBE_INIT_TCACHE (); > + ar_ptr = arena_for_chunk (p); > + _int_free (ar_ptr, p, 0); > + } > > - ar_ptr = arena_for_chunk (p); > - _int_free (ar_ptr, p, 0); > + __set_errno (err); > } > libc_hidden_def (__libc_free) > > diff --git a/malloc/tst-free-errno.c b/malloc/tst-free-errno.c > new file mode 100644 > index 0000000000..89629751e8 > --- /dev/null > +++ b/malloc/tst-free-errno.c > @@ -0,0 +1,131 @@ > +/* Test that free preserves errno. > + 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; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <stdlib.h> > +#include <string.h> > +#include <unistd.h> > +#include <fcntl.h> > +#include <stdint.h> > +#include <string.h> > +#include <sys/mman.h> > +#include <support/check.h> > +#include <support/support.h> > +#include <support/temp_file.h> > +#include <support/xunistd.h> > + > +/* The __attribute__ ((weak)) prevents a GCC optimization. Without > + it, GCC would "know" that errno is unchanged by calling free (ptr), > + when ptr was the result of a malloc call in the same function. */ > +int __attribute__ ((weak)) > +get_errno (void) > +{ > + return errno; > +} > + > +static int > +do_test (void) > +{ > + /* Check that free() preserves errno. */ > + { > + errno = 1789; /* Liberté, égalité, fraternité. */ > + free (NULL); > + TEST_VERIFY (get_errno () == 1789); > + } > + { /* Large memory allocations, to force mmap. */ > + enum { N = 2 }; > + void * volatile ptrs[N]; > + size_t i; > + for (i = 0; i < N; i++) > + ptrs[i] = malloc (5318153); Use xmalloc here. > + for (i = 0; i < N; i++) > + { > + errno = 1789; > + free (ptrs[i]); > + TEST_VERIFY (get_errno () == 1789); > + } > + } > + > + /* Test a less common code path. > + When malloc() is based on mmap(), free() can sometimes call munmap(). > + munmap() usually succeeds, but fails in a particular situation: when > + - it has to unmap the middle part of a VMA, and > + - the number of VMAs of a process is limited and the limit is > + already reached. > + The latter condition is fulfilled on Linux, when the file > + /proc/sys/vm/max_map_count exists. For all known Linux versions > + the default limit is at most 65536. > + */ > + #if defined __linux__ > + if (open ("/proc/sys/vm/max_map_count", O_RDONLY) >= 0) I think we can assume for tests /proc should be mounted, otherwise this only partially test this interface. So I think we can use xopen here. > + { > + /* Preparations. */ > + size_t pagesize = getpagesize (); > + void *firstpage_backup = xmalloc (pagesize); > + void *lastpage_backup = xmalloc (pagesize); > + /* Allocate a large memory area, as a bumper, so that the MAP_FIXED > + allocation later will not overwrite parts of the memory areas > + allocated to ld.so or libc.so. */ > + xmmap (NULL, 0x1000000, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1); > + /* A file descriptor pointing to a regular file. */ > + int fd = create_temp_file ("tst-free-errno", NULL); > + if (fd < 0) > + FAIL_EXIT1 ("cannot create temporary file"); > + > + /* Do a large memory allocation. */ > + size_t big_size = 0x1000000; > + void * volatile ptr = xmalloc (big_size - 0x100); > + char *ptr_aligned = (char *) ((uintptr_t) ptr & ~(pagesize - 1)); > + /* This large memory allocation allocated a memory area > + from ptr_aligned to ptr_aligned + big_size. > + Enlarge this memory area by adding a page before and a page > + after it. */ > + memcpy (firstpage_backup, ptr_aligned, pagesize); > + memcpy (lastpage_backup, ptr_aligned + big_size - pagesize, > + pagesize); > + xmmap (ptr_aligned - pagesize, pagesize + big_size + pagesize, > + PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1); > + memcpy (ptr_aligned, firstpage_backup, pagesize); > + memcpy (ptr_aligned + big_size - pagesize, lastpage_backup, > + pagesize); > + > + /* Now add as many mappings as we can. > + Stop at 65536, in order not to crash the machine (in case the > + limit has been increased by the system administrator). */ > + for (int i = 0; i < 65536; i++) > + if (mmap (NULL, pagesize, PROT_READ, MAP_FILE | MAP_PRIVATE, fd, 0) > + == MAP_FAILED) > + break; > + /* Now the number of VMAs of this process has hopefully attained > + its limit. */ > + > + errno = 1789; > + /* This call to free() is supposed to call > + munmap (ptr_aligned, big_size); > + which increases the number of VMAs by 1, which is supposed > + to fail. */ > + free (ptr); > + TEST_VERIFY (get_errno () == 1789); > + } > + #endif > + > + return 0; > +} > + > +#include <support/test-driver.c> Ok. > diff --git a/manual/memory.texi b/manual/memory.texi > index c132261084..b2cc65228a 100644 > --- a/manual/memory.texi > +++ b/manual/memory.texi > @@ -738,6 +738,12 @@ later call to @code{malloc} to reuse the space. In the meantime, the > space remains in your program as part of a free-list used internally by > @code{malloc}. > > +The @code{free} function preserves the value of @code{errno}, so that > +cleanup code need not worry about saving and restoring @code{errno} > +around a call to @code{free}. Although neither @w{ISO C} nor > +POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future > +version of POSIX is planned to require it. > + > There is no point in freeing blocks at the end of a program, because all > of the program's space is given back to the system when the process > terminates. Ok. > @@ -1935,6 +1941,9 @@ linking against @code{libc.a} (explicitly or implicitly). > functions (that is, all the functions used by the application, > @theglibc{}, and other linked-in libraries) can lead to static linking > failures, and, at run time, to heap corruption and application crashes. > +Replacement functions should implement the behavior documented for > +their counterparts in @theglibc{}; for example, the replacement > +@code{free} should also preserve @code{errno}. > > The minimum set of functions which has to be provided by a custom > @code{malloc} is given in the table below. > -- > 2.29.2 Ok.
On Mon, Dec 28, 2020 at 11:24 AM Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote: > > > > On 23/12/2020 22:03, Paul Eggert wrote: > > Thanks for the comments about the patch's test case. I modified the test case to reflect nearly all the comments, resulting in the attached revised patch. I'm replying below only to the comments that didn't result in a change to the patch. > > > > On 12/23/20 11:19 AM, Adhemerval Zanella wrote: > > > >> Fixing in a more fine grained would require a lot of more work to check if > >> the shared routines that calls mmap, madvise or brk won't interfere with other > >> symbols; so maybe it should be ok to use this large hammer for now. > > > > Yes, that was my thought as well. > > I will try to revise this for 2.33. > > > > >>> +The @code{free} function preserves the value of @code{errno}, so that > >>> +cleanup code need not worry about saving and restoring @code{errno} > >>> +around a call to @code{free}. Although neither @w{ISO C} nor > >>> +POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future > >>> +version of POSIX is planned to require it. > > ... > >> Not sure if this is worth to add, since we will need to update the manual > >> once the POSIX does require it. > > > > I'll volunteer to update the manual. :-) > > > > It's worth mentioning that preserving errno is not something that portable C or POSIX code should assume for 'free'. If there's a better way for the manual to warn its readers about this, that'd be fine of course. > > > > I did consider changing "Although neither @w{ISO C} nor POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future version of POSIX is planned to require it" to "Neither @w{ISO C} nor POSIX requires @code{free} to preserve @code{errno}", but that wording would be less informative and would still need updating once POSIX does require 'free' to preserve errno. > > Thanks. > > > From afbf4ff042cf3a5c8f983d5aa3bd0de3fb696dd3 Mon Sep 17 00:00:00 2001 > > From: Paul Eggert <eggert@cs.ucla.edu> > > Date: Wed, 23 Dec 2020 11:27:25 -0800 > > Subject: [PATCH] free: preserve errno [BZ#17924] > > > > In the next release of POSIX, free must preserve errno > > <https://www.austingroupbugs.net/view.php?id=385>. > > Modify __libc_free to save and restore errno, so that > > any internal munmap etc. syscalls do not disturb the caller's errno. > > Add a test malloc/tst-free-errno.c (almost all by Bruno Haible), > > and document that free preserves errno. > > LGTM with the small test change below. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > On a machine with 192 GB RAM, I got [hjl@gnu-clx-1 build-x86_64-linux]$ cat malloc/tst-free-errno-mcheck.out error: tst-free-errno.c:124: not true: get_errno () == 1789 error: 1 test failures [hjl@gnu-clx-1 build-x86_64-linux]$
On 29/12/2020 10:38, H.J. Lu wrote: > On Mon, Dec 28, 2020 at 11:24 AM Adhemerval Zanella via Libc-alpha > <libc-alpha@sourceware.org> wrote: >> >> >> >> On 23/12/2020 22:03, Paul Eggert wrote: >>> Thanks for the comments about the patch's test case. I modified the test case to reflect nearly all the comments, resulting in the attached revised patch. I'm replying below only to the comments that didn't result in a change to the patch. >>> >>> On 12/23/20 11:19 AM, Adhemerval Zanella wrote: >>> >>>> Fixing in a more fine grained would require a lot of more work to check if >>>> the shared routines that calls mmap, madvise or brk won't interfere with other >>>> symbols; so maybe it should be ok to use this large hammer for now. >>> >>> Yes, that was my thought as well. >> >> I will try to revise this for 2.33. >> >>> >>>>> +The @code{free} function preserves the value of @code{errno}, so that >>>>> +cleanup code need not worry about saving and restoring @code{errno} >>>>> +around a call to @code{free}. Although neither @w{ISO C} nor >>>>> +POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future >>>>> +version of POSIX is planned to require it. >>> ... >>>> Not sure if this is worth to add, since we will need to update the manual >>>> once the POSIX does require it. >>> >>> I'll volunteer to update the manual. :-) >>> >>> It's worth mentioning that preserving errno is not something that portable C or POSIX code should assume for 'free'. If there's a better way for the manual to warn its readers about this, that'd be fine of course. >>> >>> I did consider changing "Although neither @w{ISO C} nor POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future version of POSIX is planned to require it" to "Neither @w{ISO C} nor POSIX requires @code{free} to preserve @code{errno}", but that wording would be less informative and would still need updating once POSIX does require 'free' to preserve errno. >> >> Thanks. >> >>> From afbf4ff042cf3a5c8f983d5aa3bd0de3fb696dd3 Mon Sep 17 00:00:00 2001 >>> From: Paul Eggert <eggert@cs.ucla.edu> >>> Date: Wed, 23 Dec 2020 11:27:25 -0800 >>> Subject: [PATCH] free: preserve errno [BZ#17924] >>> >>> In the next release of POSIX, free must preserve errno >>> <https://www.austingroupbugs.net/view.php?id=385>. >>> Modify __libc_free to save and restore errno, so that >>> any internal munmap etc. syscalls do not disturb the caller's errno. >>> Add a test malloc/tst-free-errno.c (almost all by Bruno Haible), >>> and document that free preserves errno. >> >> LGTM with the small test change below. >> >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> > > On a machine with 192 GB RAM, I got > > [hjl@gnu-clx-1 build-x86_64-linux]$ cat malloc/tst-free-errno-mcheck.out > error: tst-free-errno.c:124: not true: get_errno () == 1789 > error: 1 test failures > [hjl@gnu-clx-1 build-x86_64-linux]$ > We need to fix it for the malloc check hooks as well: diff --git a/malloc/hooks.c b/malloc/hooks.c index 6474ba8b38..336ff497e9 100644 --- a/malloc/hooks.c +++ b/malloc/hooks.c @@ -260,6 +260,8 @@ free_check (void *mem, const void *caller) if (!mem) return; + int err = errno; + #ifdef USE_MTAG /* Quickly check that the freed pointer matches the tag for the memory. This gives a useful double-free detection. */ @@ -274,12 +276,16 @@ free_check (void *mem, const void *caller) { __libc_lock_unlock (main_arena.mutex); munmap_chunk (p); - return; } - /* Mark the chunk as belonging to the library again. */ - (void)TAG_REGION (chunk2rawmem (p), CHUNK_AVAILABLE_SIZE (p) - CHUNK_HDR_SZ); - _int_free (&main_arena, p, 1); - __libc_lock_unlock (main_arena.mutex); + else + { + /* Mark the chunk as belonging to the library again. */ + (void)TAG_REGION (chunk2rawmem (p), CHUNK_AVAILABLE_SIZE (p) + - CHUNK_HDR_SZ); + _int_free (&main_arena, p, 1); + __libc_lock_unlock (main_arena.mutex); + } + __set_errno (err); } static void *
diff --git a/malloc/Makefile b/malloc/Makefile index ab64dcfd73..4b3975f90d 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -34,6 +34,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-interpose-nothread \ tst-interpose-thread \ tst-alloc_buffer \ + tst-free-errno \ tst-malloc-tcache-leak \ tst-malloc_info tst-mallinfo2 \ tst-malloc-too-large \ diff --git a/malloc/malloc.c b/malloc/malloc.c index 326075e704..14bc55f96d 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -3003,6 +3003,8 @@ tcache_init(void) if (tcache_shutting_down) return; + int err = errno; + arena_get (ar_ptr, bytes); victim = _int_malloc (ar_ptr, bytes); if (!victim && ar_ptr != NULL) @@ -3015,6 +3017,8 @@ tcache_init(void) if (ar_ptr != NULL) __libc_lock_unlock (ar_ptr->mutex); + __set_errno (err); + /* In a low memory situation, we may not be able to allocate memory - in which case, we just keep trying later. However, we typically do this very early, so either there is sufficient @@ -3140,7 +3144,9 @@ __libc_free (void *mem) LIBC_PROBE (memory_mallopt_free_dyn_thresholds, 2, mp_.mmap_threshold, mp_.trim_threshold); } + int err = errno; munmap_chunk (p); + __set_errno (err); return; } diff --git a/malloc/tst-free-errno.c b/malloc/tst-free-errno.c new file mode 100644 index 0000000000..6243cb6e0b --- /dev/null +++ b/malloc/tst-free-errno.c @@ -0,0 +1,169 @@ +/* Test that free preserves errno. + 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; if not, see + <https://www.gnu.org/licenses/>. */ + +/* Written by Bruno Haible <bruno@clisp.org>, 2020. */ + +#include <errno.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#if defined __linux__ +# include <fcntl.h> +# include <stdint.h> +# include <string.h> +# include <sys/mman.h> +#endif + +#define ASSERT_NO_STDIO(expr) \ + do \ + { \ + if (!(expr)) \ + { \ + WRITE_TO_STDERR (__FILE__); \ + WRITE_TO_STDERR (":"); \ + WRITE_MACROEXPANDED_INTEGER_TO_STDERR (__LINE__); \ + WRITE_TO_STDERR (": assertion '"); \ + WRITE_TO_STDERR (#expr); \ + WRITE_TO_STDERR ("' failed\n"); \ + abort (); \ + } \ + } \ + while (0) +#define WRITE_MACROEXPANDED_INTEGER_TO_STDERR(integer) \ + WRITE_INTEGER_TO_STDERR(integer) +#define WRITE_INTEGER_TO_STDERR(integer) \ + WRITE_TO_STDERR (#integer) +#define WRITE_TO_STDERR(string_literal) \ + { \ + const char *s = string_literal; \ + int ret = write (2, s, strlen (s)); \ + (void) ret; \ + } + +/* The indirection through a volatile function pointer is necessary to prevent + a GCC optimization. Without it, when optimizing, GCC would "know" that errno + is unchanged by calling free(ptr), when ptr was the result of a malloc(...) + call in the same function. */ +static int +get_errno (void) +{ + volatile int err = errno; + return err; +} + +static int (* volatile get_errno_func) (void) = get_errno; + +static int +do_test (void) +{ + /* Check that free() preserves errno. */ + { + errno = 1789; /* Liberté, égalité, fraternité. */ + free (NULL); + ASSERT_NO_STDIO (get_errno_func () == 1789); + } + { /* Large memory allocations. */ + #define N 2 + void * volatile ptrs[N]; + size_t i; + for (i = 0; i < N; i++) + ptrs[i] = malloc (5318153); + for (i = 0; i < N; i++) + { + errno = 1789; + free (ptrs[i]); + ASSERT_NO_STDIO (get_errno_func () == 1789); + } + #undef N + } + + /* Test a less common code path. + When malloc() is based on mmap(), free() can sometimes call munmap(). + munmap() usually succeeds, but fails in a particular situation: when + - it has to unmap the middle part of a VMA, and + - the number of VMAs of a process is limited and the limit is + already reached. + The latter condition is fulfilled on Linux, when the file + /proc/sys/vm/max_map_count exists. This file contains the limit + - for Linux >= 2.4.19: 65536 (DEFAULT_MAX_MAP_COUNT in linux/include/linux/sched.h) + - for Linux >= 2.6.31: 65530 (DEFAULT_MAX_MAP_COUNT in linux/include/linux/mm.h). + */ + #if defined __linux__ + if (open ("/proc/sys/vm/max_map_count", O_RDONLY) >= 0) + { + /* Preparations. */ + size_t pagesize = getpagesize (); + void *firstpage_backup = malloc (pagesize); + void *lastpage_backup = malloc (pagesize); + /* Allocate a large memory area, as a bumper, so that the MAP_FIXED + allocation later will not overwrite parts of the memory areas + allocated to ld.so or libc.so. */ + void *bumper_region = + mmap (NULL, 0x1000000, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + /* A file descriptor pointing to a regular file. */ + int fd = open ("/etc/hosts", O_RDONLY); + + if (firstpage_backup != NULL && lastpage_backup != NULL + && bumper_region != (void *)(-1) + && fd >= 0) + { + /* Do a large memory allocation. */ + size_t big_size = 0x1000000; + void * volatile ptr = malloc (big_size - 0x100); + char *ptr_aligned = (char *) ((uintptr_t) ptr & ~(pagesize - 1)); + /* This large memory allocation allocated a memory area + from ptr_aligned to ptr_aligned + big_size. + Enlarge this memory area by adding a page before and a page + after it. */ + memcpy (firstpage_backup, ptr_aligned, pagesize); + memcpy (lastpage_backup, ptr_aligned + big_size - pagesize, pagesize); + if (mmap (ptr_aligned - pagesize, pagesize + big_size + pagesize, + PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0) + != (void *)(-1)) + { + memcpy (ptr_aligned, firstpage_backup, pagesize); + memcpy (ptr_aligned + big_size - pagesize, lastpage_backup, pagesize); + + /* Now add as many mappings as we can. + Stop at 65536, in order not to crash the machine (in case the + limit has been increased by the system administrator). */ + size_t i; + for (i = 0; i < 65536; i++) + if (mmap (NULL, pagesize, PROT_READ, MAP_FILE | MAP_PRIVATE, fd, 0) + == (void *)(-1)) + break; + /* Now the number of VMAs of this process has hopefully attained + its limit. */ + + errno = 1789; + /* This call to free() is supposed to call + munmap (ptr_aligned, big_size); + which increases the number of VMAs by 1, which is supposed + to fail. */ + free (ptr); + ASSERT_NO_STDIO (get_errno_func () == 1789); + } + } + } + #endif + + return 0; +} + +#include <support/test-driver.c> diff --git a/manual/memory.texi b/manual/memory.texi index c132261084..b2cc65228a 100644 --- a/manual/memory.texi +++ b/manual/memory.texi @@ -738,6 +738,12 @@ later call to @code{malloc} to reuse the space. In the meantime, the space remains in your program as part of a free-list used internally by @code{malloc}. +The @code{free} function preserves the value of @code{errno}, so that +cleanup code need not worry about saving and restoring @code{errno} +around a call to @code{free}. Although neither @w{ISO C} nor +POSIX.1-2017 requires @code{free} to preserve @code{errno}, a future +version of POSIX is planned to require it. + There is no point in freeing blocks at the end of a program, because all of the program's space is given back to the system when the process terminates. @@ -1935,6 +1941,9 @@ linking against @code{libc.a} (explicitly or implicitly). functions (that is, all the functions used by the application, @theglibc{}, and other linked-in libraries) can lead to static linking failures, and, at run time, to heap corruption and application crashes. +Replacement functions should implement the behavior documented for +their counterparts in @theglibc{}; for example, the replacement +@code{free} should also preserve @code{errno}. The minimum set of functions which has to be provided by a custom @code{malloc} is given in the table below.