From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zimbra.cs.ucla.edu (zimbra.cs.ucla.edu [131.179.128.68]) by sourceware.org (Postfix) with ESMTPS id 86E0F385782C for ; Mon, 21 Dec 2020 07:20:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 86E0F385782C 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 B47B51600DA; Sun, 20 Dec 2020 23:20:35 -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 ZvHv0FTMrXYY; Sun, 20 Dec 2020 23:20:34 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 1B74F160105; Sun, 20 Dec 2020 23:20:34 -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 vuCSF2CgEWXJ; Sun, 20 Dec 2020 23:20:33 -0800 (PST) Received: from [192.168.1.9] (cpe-23-243-218-95.socal.res.rr.com [23.243.218.95]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id E55B11600DA; Sun, 20 Dec 2020 23:20:33 -0800 (PST) Subject: Re: [PATCH] free: preserve errno [BZ#17924] To: Carlos O'Donell References: <20201220202556.3714-1-eggert@cs.ucla.edu> Cc: libc-alpha@sourceware.org From: Paul Eggert Organization: UCLA Computer Science Department Message-ID: Date: Sun, 20 Dec 2020 23:20:33 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------5867BEFE4AEEEA966370EBCA" Content-Language: en-US X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, NICE_REPLY_A, 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: , X-List-Received-Date: Mon, 21 Dec 2020 07:20:38 -0000 This is a multi-part message in MIME format. --------------5867BEFE4AEEEA966370EBCA Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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). --------------5867BEFE4AEEEA966370EBCA Content-Type: text/x-patch; charset=UTF-8; name="0001-free-preserve-errno-BZ-17924.patch" Content-Disposition: attachment; filename="0001-free-preserve-errno-BZ-17924.patch" Content-Transfer-Encoding: quoted-printable >From e4fec6f28270c9b0979f9fe8920dc52f8f7d70cd Mon Sep 17 00:00:00 2001 From: Paul Eggert 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 . 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 | 167 ++++++++++++++++++++++++++++++++++++++++ manual/memory.texi | 9 +++ 4 files changed, 186 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 :=3D mallocbug tst-malloc tst-valloc tst-calloc t= st-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..44020d1fd8 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -3124,6 +3124,8 @@ __libc_free (void *mem) if (mem =3D=3D 0) /* free(0) has no effec= t */ return; =20 + int err =3D errno; + p =3D mem2chunk (mem); =20 if (chunk_is_mmapped (p)) /* release mmapped mem= ory. */ @@ -3141,13 +3143,16 @@ __libc_free (void *mem) mp_.mmap_threshold, mp_.trim_threshold); } munmap_chunk (p); - return; } + else + { + MAYBE_INIT_TCACHE (); =20 - MAYBE_INIT_TCACHE (); + ar_ptr =3D arena_for_chunk (p); + _int_free (ar_ptr, p, 0); + } =20 - ar_ptr =3D arena_for_chunk (p); - _int_free (ar_ptr, p, 0); + __set_errno (err); } libc_hidden_def (__libc_free) =20 diff --git a/malloc/tst-free-errno.c b/malloc/tst-free-errno.c new file mode 100644 index 0000000000..960c419512 --- /dev/null +++ b/malloc/tst-free-errno.c @@ -0,0 +1,167 @@ +/* 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 + . */ + +#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 =3D string_literal; \ + int ret =3D write (2, s, strlen (s)); \ + (void) ret; \ + } + +/* The indirection through a volatile function pointer is necessary to p= revent + a GCC optimization. Without it, when optimizing, GCC would "know" th= at errno + is unchanged by calling free(ptr), when ptr was the result of a mallo= c(...) + call in the same function. */ +static int +get_errno (void) +{ + volatile int err =3D errno; + return err; +} + +static int (* volatile get_errno_func) (void) =3D get_errno; + +static int +do_test (void) +{ + /* Check that free() preserves errno. */ + { + errno =3D 1789; /* Libert=C3=A9, =C3=A9galit=C3=A9, fraternit=C3=A9.= */ + free (NULL); + ASSERT_NO_STDIO (get_errno_func () =3D=3D 1789); + } + { /* Large memory allocations. */ + #define N 2 + void * volatile ptrs[N]; + size_t i; + for (i =3D 0; i < N; i++) + ptrs[i] =3D malloc (5318153); + for (i =3D 0; i < N; i++) + { + errno =3D 1789; + free (ptrs[i]); + ASSERT_NO_STDIO (get_errno_func () =3D=3D 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: whe= n + - 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 >=3D 2.4.19: 65536 (DEFAULT_MAX_MAP_COUNT in linux/in= clude/linux/sched.h) + - for Linux >=3D 2.6.31: 65530 (DEFAULT_MAX_MAP_COUNT in linux/in= clude/linux/mm.h). + */ + #if defined __linux__ + if (open ("/proc/sys/vm/max_map_count", O_RDONLY) >=3D 0) + { + /* Preparations. */ + size_t pagesize =3D getpagesize (); + void *firstpage_backup =3D malloc (pagesize); + void *lastpage_backup =3D malloc (pagesize); + /* Allocate a large memory area, as a bumper, so that the MAP_FIXE= D + allocation later will not overwrite parts of the memory areas + allocated to ld.so or libc.so. */ + void *bumper_region =3D + mmap (NULL, 0x1000000, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -= 1, 0); + /* A file descriptor pointing to a regular file. */ + int fd =3D open ("/etc/hosts", O_RDONLY); + + if (firstpage_backup !=3D NULL && lastpage_backup !=3D NULL + && bumper_region !=3D (void *)(-1) + && fd >=3D 0) + { + /* Do a large memory allocation. */ + size_t big_size =3D 0x1000000; + void * volatile ptr =3D malloc (big_size - 0x100); + char *ptr_aligned =3D (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, pa= gesize); + if (mmap (ptr_aligned - pagesize, pagesize + big_size + pagesi= ze, + PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0) + !=3D (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 ca= se the + limit has been increased by the system administrator). = */ + size_t i; + for (i =3D 0; i < 65536; i++) + if (mmap (NULL, pagesize, PROT_READ, MAP_FILE | MAP_PRIV= ATE, fd, 0) + =3D=3D (void *)(-1)) + break; + /* Now the number of VMAs of this process has hopefully at= tained + its limit. */ + + errno =3D 1789; + /* This call to free() is supposed to call + munmap (ptr_aligned, big_size); + which increases the number of VMAs by 1, which is suppo= sed + to fail. */ + free (ptr); + ASSERT_NO_STDIO (get_errno_func () =3D=3D 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}. =20 +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 implic= itly). 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}. =20 The minimum set of functions which has to be provided by a custom @code{malloc} is given in the table below. --=20 2.29.2 --------------5867BEFE4AEEEA966370EBCA--