diff options
author | Paul Eggert <eggert@cs.ucla.edu> | 2020-12-29 00:45:49 -0800 |
---|---|---|
committer | Paul Eggert <eggert@cs.ucla.edu> | 2020-12-29 00:46:46 -0800 |
commit | 69fda43b8dd795c3658869633ca0708ed3134006 (patch) | |
tree | e86b216293d450bfbecaaf8230c3247444b668c4 | |
parent | 016c64236dee6e28f09c10ba38f274aad7205f95 (diff) | |
download | glibc-69fda43b8dd795c3658869633ca0708ed3134006.tar.gz glibc-69fda43b8dd795c3658869633ca0708ed3134006.tar.xz glibc-69fda43b8dd795c3658869633ca0708ed3134006.zip |
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. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
-rw-r--r-- | malloc/Makefile | 1 | ||||
-rw-r--r-- | malloc/malloc.c | 13 | ||||
-rw-r--r-- | malloc/tst-free-errno.c | 131 | ||||
-rw-r--r-- | manual/memory.texi | 9 |
4 files changed, 150 insertions, 4 deletions
diff --git a/malloc/Makefile b/malloc/Makefile index 37173b29ea..39ffaa7161 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..7781c62bff --- /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] = xmalloc (5318153); + 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 (xopen ("/proc/sys/vm/max_map_count", O_RDONLY, 0) >= 0) + { + /* 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> 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. |