From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 30127 invoked from network); 21 Jan 2021 15:50:23 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 21 Jan 2021 15:50:23 -0000 Received: (qmail 17579 invoked by uid 550); 21 Jan 2021 15:50:20 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 17557 invoked from network); 21 Jan 2021 15:50:20 -0000 Date: Thu, 21 Jan 2021 16:50:00 +0100 From: Natanael Copa To: "Alex Xu (Hello71)" Cc: musl@lists.openwall.com Message-ID: <20210121165000.61205767@ncopa-desktop.lan> In-Reply-To: <20210121140240.83405-1-alex_y_xu@yahoo.ca> References: <20210121140240.83405-1-alex_y_xu.ref@yahoo.ca> <20210121140240.83405-1-alex_y_xu@yahoo.ca> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-alpine-linux-musl) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [musl] [PATCH] don't set errno in free On Thu, 21 Jan 2021 09:02:40 -0500 "Alex Xu (Hello71)" wrote: > busybox echo fails if free sets errno, which madvise does on old > kernels. > --- > src/malloc/mallocng/free.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/src/malloc/mallocng/free.c b/src/malloc/mallocng/free.c > index 40745f97..82836815 100644 > --- a/src/malloc/mallocng/free.c > +++ b/src/malloc/mallocng/free.c > @@ -119,7 +119,13 @@ void free(void *p) > if (((uintptr_t)(start-1) ^ (uintptr_t)end) >= 2*PGSZ && g->last_idx) { > unsigned char *base = start + (-(uintptr_t)start & (PGSZ-1)); > size_t len = (end-base) & -PGSZ; > - if (len) madvise(base, len, MADV_FREE); > + if (len) { > + // madvise(..., MADV_FREE) returns -EINVAL on old kernels > + // POSIX.1-202x requires free() to not modify errno on success > + int e = errno; > + madvise(base, len, MADV_FREE); > + errno = e; > + } > } I think we should save the errno early and make sure its restored on exit of the function. you should also include . I suggest something like: diff --git a/src/malloc/mallocng/free.c b/src/malloc/mallocng/free.c index 40745f97..77bed88b 100644 --- a/src/malloc/mallocng/free.c +++ b/src/malloc/mallocng/free.c @@ -1,6 +1,7 @@ #define _BSD_SOURCE #include #include +#include #include "meta.h" @@ -102,6 +103,7 @@ void free(void *p) { if (!p) return; + int orig_errno = errno; struct meta *g = get_meta(p); int idx = get_slot_index(p); size_t stride = get_stride(g); @@ -133,11 +135,13 @@ void free(void *p) g->freed_mask = freed+self; else if (a_cas(&g->freed_mask, freed, freed+self)!=freed) continue; - return; + goto out; } wrlock(); struct mapinfo mi = nontrivial_free(g, idx); unlock(); if (mi.len) munmap(mi.base, mi.len); +out: + errno = orig_errno; } (looks like there are used names like errno_save, and old_errno in the code as well) > > // atomic free without locking if this is neither first or last slot > @@ -139,5 +145,9 @@ void free(void *p) > wrlock(); > struct mapinfo mi = nontrivial_free(g, idx); > unlock(); > - if (mi.len) munmap(mi.base, mi.len); > + // POSIX.1-202x requires free() to not modify errno on success > + // munmap should succeed but no harm checking it again > + if (mi.len) > + if (munmap(mi.base, mi.len)) > + a_crash(); > } This should go into separate commit. -nc