From patchwork Sun Dec 20 20:25:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Eggert X-Patchwork-Id: 41488 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0361E3858021; Sun, 20 Dec 2020 20:26:47 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from zimbra.cs.ucla.edu (zimbra.cs.ucla.edu [131.179.128.68]) by sourceware.org (Postfix) with ESMTPS id 211713857C52 for ; Sun, 20 Dec 2020 20:26:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 211713857C52 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=cs.ucla.edu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=eggert@cs.ucla.edu Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 535CD160105 for ; Sun, 20 Dec 2020 12:26:42 -0800 (PST) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id auQLALvYLuZE; Sun, 20 Dec 2020 12:26:41 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id E74001600FF; Sun, 20 Dec 2020 12:26:40 -0800 (PST) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id oM3cUyn1jtui; Sun, 20 Dec 2020 12:26:40 -0800 (PST) Received: from penguin.CS.UCLA.EDU (Penguin.CS.UCLA.EDU [131.179.64.200]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id BCB4A1600C3; Sun, 20 Dec 2020 12:26:40 -0800 (PST) From: Paul Eggert To: libc-alpha@sourceware.org Subject: [PATCH] free: preserve errno [BZ#17924] Date: Sun, 20 Dec 2020 12:25:56 -0800 Message-Id: <20201220202556.3714-1-eggert@cs.ucla.edu> X-Mailer: git-send-email 2.29.2 MIME-Version: 1.0 X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" * 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. Reviewed-by: Siddhesh Poyarekar Reviewed-by: Adhemerval Zanella --- 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; } 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 + . */ + +/* Written by Bruno Haible , 2020. */ + +#include +#include +#include +#include +#if defined __linux__ +# include +# include +# include +# include +#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 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.