* Dumper issue, revisited; invalid realloc/free @ 2015-02-04 17:57 Rich Felker 2015-02-04 19:08 ` Eli Zaretskii 2015-02-04 19:21 ` Eli Zaretskii 0 siblings, 2 replies; 23+ messages in thread From: Rich Felker @ 2015-02-04 17:57 UTC (permalink / raw) To: emacs-devel Last summer I started a thread about the ever-recurring dumper portability problem and how it was blocking use of emacs on systems based on musl libc. Recently I've been working with several people interested in getting emacs working on Alpine Linux and musl-based Gentoo, and I made some progress working around the issue: http://www.openwall.com/lists/musl/2015/02/03/1 (Note: there are a couple known bugs in the attached code.) However, on further examination, the workaround I did is insufficient. From what I can tell, emacs is making an additional assumption on malloc: not only that malloc results will be contiguous with data/bss/brk so they can be dumped, but also that calling free() or realloc() on these objects in the new process after dumping is valid. IMO this is utter nonsense, even with glibc or other widely-used systems. It imposes an assumption that the heap structures in the malloc version used at dump time match the heap structures in the malloc version used at runtime, and that the runtime malloc is not doing any sanity checks to catch and abort when a pointer into .data is passed to realloc/free. The simplest solution I can find is to make the affected free functions (xrealloc, xfree, lisp_free, and lisp_align_free) check whether their argument is in the range of data_start...initial_brk and act as a nop (or in the case of xrealloc, allocate a new object without freeing the old one) when this is the case. The check is easily accomplished by saving initial_brk=sbrk(0) at startup (to be lazy I did this with attribute((constructor)) but other approaches might be preferred for an upstream fix. For xrealloc, since the old size is not known, I simply estimate it as initial_brk-block. Copying up to the min or this value and the new size should be safe, anyway. Does this sound acceptable for upstream? Rich ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 17:57 Dumper issue, revisited; invalid realloc/free Rich Felker @ 2015-02-04 19:08 ` Eli Zaretskii 2015-02-04 19:13 ` Rich Felker 2015-02-04 19:21 ` Eli Zaretskii 1 sibling, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2015-02-04 19:08 UTC (permalink / raw) To: Rich Felker; +Cc: emacs-devel > Date: Wed, 4 Feb 2015 12:57:09 -0500 > From: Rich Felker <dalias@libc.org> > > The simplest solution I can find is to make the affected free > functions (xrealloc, xfree, lisp_free, and lisp_align_free) check > whether their argument is in the range of data_start...initial_brk and > act as a nop (or in the case of xrealloc, allocate a new object > without freeing the old one) when this is the case. The check is > easily accomplished by saving initial_brk=sbrk(0) at startup (to be > lazy I did this with attribute((constructor)) but other approaches > might be preferred for an upstream fix. For xrealloc, since the old > size is not known, I simply estimate it as initial_brk-block. Copying > up to the min or this value and the new size should be safe, anyway. > > Does this sound acceptable for upstream? Yes. Several platforms (Cygwin and MinGW on MS-Windows) already do exactly that. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 19:08 ` Eli Zaretskii @ 2015-02-04 19:13 ` Rich Felker 2015-02-04 19:22 ` Eli Zaretskii 2015-02-04 19:26 ` Eli Zaretskii 0 siblings, 2 replies; 23+ messages in thread From: Rich Felker @ 2015-02-04 19:13 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel On Wed, Feb 04, 2015 at 09:08:00PM +0200, Eli Zaretskii wrote: > > Date: Wed, 4 Feb 2015 12:57:09 -0500 > > From: Rich Felker <dalias@libc.org> > > > > The simplest solution I can find is to make the affected free > > functions (xrealloc, xfree, lisp_free, and lisp_align_free) check > > whether their argument is in the range of data_start...initial_brk and > > act as a nop (or in the case of xrealloc, allocate a new object > > without freeing the old one) when this is the case. The check is > > easily accomplished by saving initial_brk=sbrk(0) at startup (to be > > lazy I did this with attribute((constructor)) but other approaches > > might be preferred for an upstream fix. For xrealloc, since the old > > size is not known, I simply estimate it as initial_brk-block. Copying > > up to the min or this value and the new size should be safe, anyway. > > > > Does this sound acceptable for upstream? > > Yes. Several platforms (Cygwin and MinGW on MS-Windows) already do > exactly that. Where is the code that does this? I don't see it in alloc.c. Is it only used when system_malloc=no? The case where it's really needed is for system_malloc=yes... Rich ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 19:13 ` Rich Felker @ 2015-02-04 19:22 ` Eli Zaretskii 2015-02-04 19:26 ` Eli Zaretskii 1 sibling, 0 replies; 23+ messages in thread From: Eli Zaretskii @ 2015-02-04 19:22 UTC (permalink / raw) To: Rich Felker; +Cc: emacs-devel > Date: Wed, 4 Feb 2015 14:13:05 -0500 > From: Rich Felker <dalias@libc.org> > Cc: emacs-devel@gnu.org > > > Yes. Several platforms (Cygwin and MinGW on MS-Windows) already do > > exactly that. > > Where is the code that does this? I don't see it in alloc.c. The Cygwin code is in sheap.c, the MinGW code is in w32heap.c. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 19:13 ` Rich Felker 2015-02-04 19:22 ` Eli Zaretskii @ 2015-02-04 19:26 ` Eli Zaretskii 2015-02-04 20:34 ` Ken Brown 1 sibling, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2015-02-04 19:26 UTC (permalink / raw) To: Rich Felker; +Cc: emacs-devel > Date: Wed, 4 Feb 2015 14:13:05 -0500 > From: Rich Felker <dalias@libc.org> > Cc: emacs-devel@gnu.org > > > Yes. Several platforms (Cygwin and MinGW on MS-Windows) already do > > exactly that. > > Where is the code that does this? I don't see it in alloc.c. The Cygwin code is in sheap.c, the MinGW code is in w32heap.c. > Is it only used when system_malloc=no? The case where it's really > needed is for system_malloc=yes... MinGW uses that with system malloc. Not sure about Cygwin. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 19:26 ` Eli Zaretskii @ 2015-02-04 20:34 ` Ken Brown 2015-02-05 1:31 ` Rich Felker 0 siblings, 1 reply; 23+ messages in thread From: Ken Brown @ 2015-02-04 20:34 UTC (permalink / raw) To: Eli Zaretskii, Rich Felker; +Cc: emacs-devel On 2/4/2015 2:26 PM, Eli Zaretskii wrote: >> Date: Wed, 4 Feb 2015 14:13:05 -0500 >> From: Rich Felker <dalias@libc.org> >> Cc: emacs-devel@gnu.org >> >>> Yes. Several platforms (Cygwin and MinGW on MS-Windows) already do >>> exactly that. >> >> Where is the code that does this? I don't see it in alloc.c. > > The Cygwin code is in sheap.c, the MinGW code is in w32heap.c. > >> Is it only used when system_malloc=no? The case where it's really >> needed is for system_malloc=yes... > > MinGW uses that with system malloc. Not sure about Cygwin. Cygwin (in the master branch) uses gmalloc.c before dumping and the system malloc after dumping. See the code in gmalloc.c involving HYBRID_MALLOC. Ken ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 20:34 ` Ken Brown @ 2015-02-05 1:31 ` Rich Felker 2015-02-05 3:25 ` Rich Felker 0 siblings, 1 reply; 23+ messages in thread From: Rich Felker @ 2015-02-05 1:31 UTC (permalink / raw) To: Ken Brown; +Cc: Eli Zaretskii, emacs-devel On Wed, Feb 04, 2015 at 03:34:40PM -0500, Ken Brown wrote: > On 2/4/2015 2:26 PM, Eli Zaretskii wrote: > >>Date: Wed, 4 Feb 2015 14:13:05 -0500 > >>From: Rich Felker <dalias@libc.org> > >>Cc: emacs-devel@gnu.org > >> > >>>Yes. Several platforms (Cygwin and MinGW on MS-Windows) already do > >>>exactly that. > >> > >>Where is the code that does this? I don't see it in alloc.c. > > > >The Cygwin code is in sheap.c, the MinGW code is in w32heap.c. > > > >>Is it only used when system_malloc=no? The case where it's really > >>needed is for system_malloc=yes... > > > >MinGW uses that with system malloc. Not sure about Cygwin. > > Cygwin (in the master branch) uses gmalloc.c before dumping and the > system malloc after dumping. See the code in gmalloc.c involving > HYBRID_MALLOC. Interesting. This is new code I hadn't seen yet (seemingly based on the conversations last summer) and looks very promising. But I'm curious -- how do you prevent pointers obtained from the before-dumping gmalloc from getting passed to the system malloc's free? If we can solve this I think we have a clean, robust way to get GNU Emacs building correctly on musl without hacks. Rich ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-05 1:31 ` Rich Felker @ 2015-02-05 3:25 ` Rich Felker 2015-02-05 3:48 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Rich Felker @ 2015-02-05 3:25 UTC (permalink / raw) To: Ken Brown; +Cc: Eli Zaretskii, emacs-devel On Wed, Feb 04, 2015 at 08:31:47PM -0500, Rich Felker wrote: > On Wed, Feb 04, 2015 at 03:34:40PM -0500, Ken Brown wrote: > > On 2/4/2015 2:26 PM, Eli Zaretskii wrote: > > >>Date: Wed, 4 Feb 2015 14:13:05 -0500 > > >>From: Rich Felker <dalias@libc.org> > > >>Cc: emacs-devel@gnu.org > > >> > > >>>Yes. Several platforms (Cygwin and MinGW on MS-Windows) already do > > >>>exactly that. > > >> > > >>Where is the code that does this? I don't see it in alloc.c. > > > > > >The Cygwin code is in sheap.c, the MinGW code is in w32heap.c. > > > > > >>Is it only used when system_malloc=no? The case where it's really > > >>needed is for system_malloc=yes... > > > > > >MinGW uses that with system malloc. Not sure about Cygwin. > > > > Cygwin (in the master branch) uses gmalloc.c before dumping and the > > system malloc after dumping. See the code in gmalloc.c involving > > HYBRID_MALLOC. > > Interesting. This is new code I hadn't seen yet (seemingly based on > the conversations last summer) and looks very promising. But I'm > curious -- how do you prevent pointers obtained from the > before-dumping gmalloc from getting passed to the system malloc's > free? If we can solve this I think we have a clean, robust way to get > GNU Emacs building correctly on musl without hacks. After reading the source, I see -- there's a simple range checking macro. Simply removing the #ifdef CYGWIN around all the relevant code seems to make it work with no further hackery, which is very promising, but with current git master I'm hitting lots of lisp failures later in the build process, seemingly due to failure of autoloads to work, although some of the failures don't even seem to be mentioned in autoload.el. Is this kind of failure expected working with bleeding edge (if so I'll seek support somewhere more appropriate), or does it sound like something went wrong in dumping? Rich ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-05 3:25 ` Rich Felker @ 2015-02-05 3:48 ` Eli Zaretskii 2015-02-05 4:33 ` Rich Felker 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2015-02-05 3:48 UTC (permalink / raw) To: Rich Felker; +Cc: kbrown, emacs-devel > Date: Wed, 4 Feb 2015 22:25:50 -0500 > From: Rich Felker <dalias@libc.org> > Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org > > with current git master I'm hitting lots of lisp failures later in > the build process, seemingly due to failure of autoloads to work, > although some of the failures don't even seem to be mentioned in > autoload.el. Is this kind of failure expected working with bleeding > edge (if so I'll seek support somewhere more appropriate), or does > it sound like something went wrong in dumping? It's not expected, so you should debug it. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-05 3:48 ` Eli Zaretskii @ 2015-02-05 4:33 ` Rich Felker 2015-02-05 16:14 ` Wolfgang Jenkner 0 siblings, 1 reply; 23+ messages in thread From: Rich Felker @ 2015-02-05 4:33 UTC (permalink / raw) To: Eli Zaretskii; +Cc: kbrown, emacs-devel On Thu, Feb 05, 2015 at 05:48:30AM +0200, Eli Zaretskii wrote: > > Date: Wed, 4 Feb 2015 22:25:50 -0500 > > From: Rich Felker <dalias@libc.org> > > Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org > > > > with current git master I'm hitting lots of lisp failures later in > > the build process, seemingly due to failure of autoloads to work, > > although some of the failures don't even seem to be mentioned in > > autoload.el. Is this kind of failure expected working with bleeding > > edge (if so I'll seek support somewhere more appropriate), or does > > it sound like something went wrong in dumping? > > It's not expected, so you should debug it. Thanks for steering me in the right direction. The problem was a mix of two things: 1. I was having to incrementally increase the static sbrk size and still never getting enough because I didn't realize unexelf.c lacked code to set DUMPED to 1. 2. The failed bootstrap in lisp/ tree was leaving around stale/broken elc files that needed to be cleaned with "make bootstrap-clean". I now have a working build with no ugly hackery. I'll share my patches (mostly uninteresting) with the people doing dist packaging and see if they can make something suitable for upstream. Rich ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-05 4:33 ` Rich Felker @ 2015-02-05 16:14 ` Wolfgang Jenkner 2015-02-05 18:43 ` Rich Felker 0 siblings, 1 reply; 23+ messages in thread From: Wolfgang Jenkner @ 2015-02-05 16:14 UTC (permalink / raw) To: Rich Felker; +Cc: Eli Zaretskii, kbrown, emacs-devel On Wed, Feb 04 2015, Rich Felker wrote: > I now have a working build with no ugly hackery. Does it use any special properties of musl malloc or should the hybrid approach now work with any reasonable malloc implementation? Wolfgang ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-05 16:14 ` Wolfgang Jenkner @ 2015-02-05 18:43 ` Rich Felker 0 siblings, 0 replies; 23+ messages in thread From: Rich Felker @ 2015-02-05 18:43 UTC (permalink / raw) To: Eli Zaretskii, kbrown, emacs-devel On Thu, Feb 05, 2015 at 05:14:54PM +0100, Wolfgang Jenkner wrote: > On Wed, Feb 04 2015, Rich Felker wrote: > > > I now have a working build with no ugly hackery. > > Does it use any special properties of musl malloc or should the hybrid > approach now work with any reasonable malloc implementation? No special properties at all; that's what I meant by no ugly hackery. In fact it seems like it _couldn't_ depend on anything about the malloc implementation, because the system malloc implementation is not used prior to dumping and the pre-dump objects are never passed to it after dumping. So, based on both the concept and the practical success with it (everything worked basically first try with no difficulty on a new target), I believe hybrid malloc should be the default for platforms where there's not a reason to prefer something else. Rich ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 17:57 Dumper issue, revisited; invalid realloc/free Rich Felker 2015-02-04 19:08 ` Eli Zaretskii @ 2015-02-04 19:21 ` Eli Zaretskii 2015-02-04 19:37 ` Rich Felker 1 sibling, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2015-02-04 19:21 UTC (permalink / raw) To: Rich Felker; +Cc: emacs-devel > Date: Wed, 4 Feb 2015 12:57:09 -0500 > From: Rich Felker <dalias@libc.org> > > Last summer I started a thread about the ever-recurring dumper > portability problem and how it was blocking use of emacs on systems > based on musl libc. Recently I've been working with several people > interested in getting emacs working on Alpine Linux and musl-based > Gentoo, and I made some progress working around the issue: > > http://www.openwall.com/lists/musl/2015/02/03/1 I suggest that you take a look at src/w32heap.c on Emacs's master branch. There' you will see a simple solution of a very similar (if not identical) problem we have on MS-Windows. It even includes a simple handling of large allocations. The only disadvantage of this scheme is that it wastes space in the final binary, because the space reserved for the build-time allocations needs to be large enough to support the building of bootstrap-emacs, which is built before the Lisp files are compiled, and thus needs roughly twice the space -- which is then wasted in the next binary the build produces. It would be nice to solve this at some point, but it's not a catastrophe. > However, on further examination, the workaround I did is insufficient. > >From what I can tell, emacs is making an additional assumption on > malloc: not only that malloc results will be contiguous with > data/bss/brk so they can be dumped, but also that calling free() or > realloc() on these objects in the new process after dumping is valid. Either that, or realloc/free are never called on the objects allocated before dumping. On some platforms, the second assumption actually holds. > IMO this is utter nonsense, even with glibc or other widely-used > systems. It imposes an assumption that the heap structures in the > malloc version used at dump time match the heap structures in the > malloc version used at runtime, and that the runtime malloc is not > doing any sanity checks to catch and abort when a pointer into .data > is passed to realloc/free. Or that the libc memory allocation routines can gracefully handle these situations. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 19:21 ` Eli Zaretskii @ 2015-02-04 19:37 ` Rich Felker 2015-02-04 19:44 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Rich Felker @ 2015-02-04 19:37 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 3003 bytes --] On Wed, Feb 04, 2015 at 09:21:03PM +0200, Eli Zaretskii wrote: > > Date: Wed, 4 Feb 2015 12:57:09 -0500 > > From: Rich Felker <dalias@libc.org> > > > > Last summer I started a thread about the ever-recurring dumper > > portability problem and how it was blocking use of emacs on systems > > based on musl libc. Recently I've been working with several people > > interested in getting emacs working on Alpine Linux and musl-based > > Gentoo, and I made some progress working around the issue: > > > > http://www.openwall.com/lists/musl/2015/02/03/1 > > I suggest that you take a look at src/w32heap.c on Emacs's master > branch. There' you will see a simple solution of a very similar (if > not identical) problem we have on MS-Windows. It even includes a > simple handling of large allocations. As I suspected, this code is used only if you use gmalloc.c. It's not used with system_malloc=yes, which is the case I'm concerned about. > > > However, on further examination, the workaround I did is insufficient. > > >From what I can tell, emacs is making an additional assumption on > > malloc: not only that malloc results will be contiguous with > > data/bss/brk so they can be dumped, but also that calling free() or > > realloc() on these objects in the new process after dumping is valid. > > Either that, or realloc/free are never called on the objects allocated > before dumping. On some platforms, the second assumption actually > holds. Actually that seemed to be true on 32-bit x86 for me (which is why I originally thought my preload library was sufficient) but failed on 64-bit. All I can guess is that the larger pointer size perturbs the behavior. > > IMO this is utter nonsense, even with glibc or other widely-used > > systems. It imposes an assumption that the heap structures in the > > malloc version used at dump time match the heap structures in the > > malloc version used at runtime, and that the runtime malloc is not > > doing any sanity checks to catch and abort when a pointer into .data > > is passed to realloc/free. > > Or that the libc memory allocation routines can gracefully handle > these situations. I would not consider that "graceful". If they detect that the pointer passed to realloc or free is invalid, the only reasonable behavior is to abort. If they don't detect this case specially, then you're relying on an assumption that they'll be compatible with the runtime heap structures which could be invalidated by a libc upgrade. IMO the only reasonable solution is for emacs to make sure it never calls free or realloc with pointers that weren't obtained by malloc during the same program invocation (i.e. as opposed to pre-dump malloc). I'm attaching the patch. Apologies that it's not against latest emacs; 24.3 was the version I had lying around when I got started on this. At this point it's just for comments/discussion. I'll regenerate it against current emacs and clean it up if there's a possibility of it actually getting upstreamed. Rich [-- Attachment #2: emacs_alloc_invalid_frees.diff --] [-- Type: text/plain, Size: 1258 bytes --] --- emacs-24.3.orig/src/alloc.c +++ emacs-24.3/src/alloc.c @@ -47,6 +47,13 @@ #include <verify.h> +static void *initial_brk; +__attribute__((__constructor__)) +static void init() +{ + initial_brk = sbrk(0); +} + /* GC_CHECK_MARKED_OBJECTS means do sanity checks on allocated objects. Doable only if GC_MARK_STACK. */ #if ! GC_MARK_STACK @@ -699,6 +706,14 @@ { void *val; + if (block && block < initial_brk) { + size_t len = (char *)initial_brk - (char *)block; + if (len > size) len = size; + void *p = xmalloc(size); + memcpy(p, block, len); + return p; + } + MALLOC_BLOCK_INPUT; /* We must call malloc explicitly when BLOCK is 0, since some reallocs don't do this. */ @@ -720,6 +735,7 @@ void xfree (void *block) { + if (block < initial_brk) return; if (!block) return; MALLOC_BLOCK_INPUT; @@ -910,6 +926,7 @@ static void lisp_free (void *block) { + if (block < initial_brk) return; MALLOC_BLOCK_INPUT; free (block); #if GC_MARK_STACK && !defined GC_MALLOC_CHECK @@ -1117,6 +1134,8 @@ { struct ablock *ablock = block; struct ablocks *abase = ABLOCK_ABASE (ablock); + + if (block < initial_brk) return; MALLOC_BLOCK_INPUT; #if GC_MARK_STACK && !defined GC_MALLOC_CHECK ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 19:37 ` Rich Felker @ 2015-02-04 19:44 ` Eli Zaretskii 2015-02-04 19:49 ` Rich Felker 2015-02-04 20:08 ` Rich Felker 0 siblings, 2 replies; 23+ messages in thread From: Eli Zaretskii @ 2015-02-04 19:44 UTC (permalink / raw) To: Rich Felker; +Cc: emacs-devel > Date: Wed, 4 Feb 2015 14:37:32 -0500 > From: Rich Felker <dalias@libc.org> > Cc: emacs-devel@gnu.org > > > > http://www.openwall.com/lists/musl/2015/02/03/1 > > > > I suggest that you take a look at src/w32heap.c on Emacs's master > > branch. There' you will see a simple solution of a very similar (if > > not identical) problem we have on MS-Windows. It even includes a > > simple handling of large allocations. > > As I suspected, this code is used only if you use gmalloc.c. It's not > used with system_malloc=yes, which is the case I'm concerned about. No, you are mistaken. The 'master' version of Emacs uses the system malloc on MS-Windows. Perhaps you are looking at the 'emacs-24' branch, where indeed we use gmalloc.c and ralloc.c, with sbrk emulation in w32heap.c. But that's not what I had in mind. > > Or that the libc memory allocation routines can gracefully handle > > these situations. > > I would not consider that "graceful". If they detect that the pointer > passed to realloc or free is invalid, the only reasonable behavior is > to abort. They could do exactly what you planned to do: ignore the 'free' part and only allocate a new block. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 19:44 ` Eli Zaretskii @ 2015-02-04 19:49 ` Rich Felker 2015-02-04 19:54 ` Eli Zaretskii 2015-02-04 20:08 ` Rich Felker 1 sibling, 1 reply; 23+ messages in thread From: Rich Felker @ 2015-02-04 19:49 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel On Wed, Feb 04, 2015 at 09:44:17PM +0200, Eli Zaretskii wrote: > > Date: Wed, 4 Feb 2015 14:37:32 -0500 > > From: Rich Felker <dalias@libc.org> > > Cc: emacs-devel@gnu.org > > > > > > http://www.openwall.com/lists/musl/2015/02/03/1 > > > > > > I suggest that you take a look at src/w32heap.c on Emacs's master > > > branch. There' you will see a simple solution of a very similar (if > > > not identical) problem we have on MS-Windows. It even includes a > > > simple handling of large allocations. > > > > As I suspected, this code is used only if you use gmalloc.c. It's not > > used with system_malloc=yes, which is the case I'm concerned about. > > No, you are mistaken. The 'master' version of Emacs uses the system > malloc on MS-Windows. Perhaps you are looking at the 'emacs-24' > branch, where indeed we use gmalloc.c and ralloc.c, with sbrk > emulation in w32heap.c. But that's not what I had in mind. Ah, I wasn't aware there was significant new development in this area! I'll take a look at master. > > > Or that the libc memory allocation routines can gracefully handle > > > these situations. > > > > I would not consider that "graceful". If they detect that the pointer > > passed to realloc or free is invalid, the only reasonable behavior is > > to abort. > > They could do exactly what you planned to do: ignore the 'free' part > and only allocate a new block. This behavior does not make sense in system malloc; it's only logical if you know the erroneous call is a result of emacs' dumper. The usual case when a data/bss pointer is passed to realloc or free is a serious programming error or memory corruption and the responsible thing to do when this is seen (if you bother to detect it) is to abort the program before something worse happens. Rich ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 19:49 ` Rich Felker @ 2015-02-04 19:54 ` Eli Zaretskii 2015-02-04 20:02 ` Rich Felker 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2015-02-04 19:54 UTC (permalink / raw) To: Rich Felker; +Cc: emacs-devel > Date: Wed, 4 Feb 2015 14:49:10 -0500 > From: Rich Felker <dalias@libc.org> > Cc: emacs-devel@gnu.org > > > > > Or that the libc memory allocation routines can gracefully handle > > > > these situations. > > > > > > I would not consider that "graceful". If they detect that the pointer > > > passed to realloc or free is invalid, the only reasonable behavior is > > > to abort. > > > > They could do exactly what you planned to do: ignore the 'free' part > > and only allocate a new block. > > This behavior does not make sense in system malloc Of course, it does: to support Emacs. glibc is still a GNU project, isn't it? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 19:54 ` Eli Zaretskii @ 2015-02-04 20:02 ` Rich Felker 2015-02-04 20:36 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Rich Felker @ 2015-02-04 20:02 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel On Wed, Feb 04, 2015 at 09:54:38PM +0200, Eli Zaretskii wrote: > > Date: Wed, 4 Feb 2015 14:49:10 -0500 > > From: Rich Felker <dalias@libc.org> > > Cc: emacs-devel@gnu.org > > > > > > > Or that the libc memory allocation routines can gracefully handle > > > > > these situations. > > > > > > > > I would not consider that "graceful". If they detect that the pointer > > > > passed to realloc or free is invalid, the only reasonable behavior is > > > > to abort. > > > > > > They could do exactly what you planned to do: ignore the 'free' part > > > and only allocate a new block. > > > > This behavior does not make sense in system malloc > > Of course, it does: to support Emacs. glibc is still a GNU project, > isn't it? glibc is maintained by a consensus-based community these days, and I don't think sacrificing the ability to detect serious memory corruption that likely indicates an exploit attempt for the sake of satisfying emacs' invalid assumptions about malloc would be popular. The same checks can be made on the emacs side before calling the underlying realloc/free, and this way they don't compromise efforts to harden other applications. Rich ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 20:02 ` Rich Felker @ 2015-02-04 20:36 ` Eli Zaretskii 0 siblings, 0 replies; 23+ messages in thread From: Eli Zaretskii @ 2015-02-04 20:36 UTC (permalink / raw) To: Rich Felker; +Cc: emacs-devel > Date: Wed, 4 Feb 2015 15:02:15 -0500 > From: Rich Felker <dalias@libc.org> > Cc: emacs-devel@gnu.org > > glibc is maintained by a consensus-based community these days, and I > don't think sacrificing the ability to detect serious memory > corruption that likely indicates an exploit attempt for the sake of > satisfying emacs' invalid assumptions about malloc would be popular. It can be done without sacrificing anything. > The same checks can be made on the emacs side before calling the > underlying realloc/free Yes, they can. Both alternative require some knowledge of the other side, so they are both equivalent to some degree. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 19:44 ` Eli Zaretskii 2015-02-04 19:49 ` Rich Felker @ 2015-02-04 20:08 ` Rich Felker 2015-02-04 20:40 ` Eli Zaretskii 1 sibling, 1 reply; 23+ messages in thread From: Rich Felker @ 2015-02-04 20:08 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel On Wed, Feb 04, 2015 at 09:44:17PM +0200, Eli Zaretskii wrote: > > Date: Wed, 4 Feb 2015 14:37:32 -0500 > > From: Rich Felker <dalias@libc.org> > > Cc: emacs-devel@gnu.org > > > > > > http://www.openwall.com/lists/musl/2015/02/03/1 > > > > > > I suggest that you take a look at src/w32heap.c on Emacs's master > > > branch. There' you will see a simple solution of a very similar (if > > > not identical) problem we have on MS-Windows. It even includes a > > > simple handling of large allocations. > > > > As I suspected, this code is used only if you use gmalloc.c. It's not > > used with system_malloc=yes, which is the case I'm concerned about. > > No, you are mistaken. The 'master' version of Emacs uses the system > malloc on MS-Windows. Perhaps you are looking at the 'emacs-24' > branch, where indeed we use gmalloc.c and ralloc.c, with sbrk > emulation in w32heap.c. But that's not what I had in mind. Upon checking master, w32heap.c is not using the system malloc. It's its own implementation of the malloc API written on top of the Win32 HeapAlloc API. Rich ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 20:08 ` Rich Felker @ 2015-02-04 20:40 ` Eli Zaretskii 2015-02-04 22:17 ` Rich Felker 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2015-02-04 20:40 UTC (permalink / raw) To: Rich Felker; +Cc: emacs-devel > Date: Wed, 4 Feb 2015 15:08:42 -0500 > From: Rich Felker <dalias@libc.org> > Cc: emacs-devel@gnu.org > > Upon checking master, w32heap.c is not using the system malloc. It's > its own implementation of the malloc API written on top of the Win32 > HeapAlloc API. System malloc on Windows is a thin wrapper around HeapAlloc, so we are actually using system malloc. There are good reasons why we call HeapAlloc directly, but they are immaterial for the purposes of this discussion. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 20:40 ` Eli Zaretskii @ 2015-02-04 22:17 ` Rich Felker 2015-02-05 3:42 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Rich Felker @ 2015-02-04 22:17 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel On Wed, Feb 04, 2015 at 10:40:15PM +0200, Eli Zaretskii wrote: > > Date: Wed, 4 Feb 2015 15:08:42 -0500 > > From: Rich Felker <dalias@libc.org> > > Cc: emacs-devel@gnu.org > > > > Upon checking master, w32heap.c is not using the system malloc. It's > > its own implementation of the malloc API written on top of the Win32 > > HeapAlloc API. > > System malloc on Windows is a thin wrapper around HeapAlloc, so we are > actually using system malloc. There are good reasons why we call > HeapAlloc directly, but they are immaterial for the purposes of this > discussion. Yes, but from my standpoint it's conceptually very different -- as written, it's depending on interposing its own definition of malloc and would not work, for example, on a windows runtime that uses a malloc not based on HeapAlloc. (This is actually a practical consideration, as we have someone working on a development environment that brings musl to Windows with the intent of replacing both cygwin and mingw with something that satisfies the major usage cases for both and offers various advantages.) Rich ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Dumper issue, revisited; invalid realloc/free 2015-02-04 22:17 ` Rich Felker @ 2015-02-05 3:42 ` Eli Zaretskii 0 siblings, 0 replies; 23+ messages in thread From: Eli Zaretskii @ 2015-02-05 3:42 UTC (permalink / raw) To: Rich Felker; +Cc: emacs-devel > Date: Wed, 4 Feb 2015 17:17:04 -0500 > From: Rich Felker <dalias@libc.org> > Cc: emacs-devel@gnu.org > > On Wed, Feb 04, 2015 at 10:40:15PM +0200, Eli Zaretskii wrote: > > > Date: Wed, 4 Feb 2015 15:08:42 -0500 > > > From: Rich Felker <dalias@libc.org> > > > Cc: emacs-devel@gnu.org > > > > > > Upon checking master, w32heap.c is not using the system malloc. It's > > > its own implementation of the malloc API written on top of the Win32 > > > HeapAlloc API. > > > > System malloc on Windows is a thin wrapper around HeapAlloc, so we are > > actually using system malloc. There are good reasons why we call > > HeapAlloc directly, but they are immaterial for the purposes of this > > discussion. > > Yes, but from my standpoint it's conceptually very different -- as > written, it's depending on interposing its own definition of malloc > and would not work, for example, on a windows runtime that uses a > malloc not based on HeapAlloc. How is it different? You could make malloc a function pointer, and point it to one implementation at dump time, and to another after dumping. The main point here, and the one I thought was important for you, is that gmalloc is not used, i.e. the code does not depend on the internals of the malloc implementation. This condition is satisfied by using HeapAlloc. ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2015-02-05 18:43 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-02-04 17:57 Dumper issue, revisited; invalid realloc/free Rich Felker 2015-02-04 19:08 ` Eli Zaretskii 2015-02-04 19:13 ` Rich Felker 2015-02-04 19:22 ` Eli Zaretskii 2015-02-04 19:26 ` Eli Zaretskii 2015-02-04 20:34 ` Ken Brown 2015-02-05 1:31 ` Rich Felker 2015-02-05 3:25 ` Rich Felker 2015-02-05 3:48 ` Eli Zaretskii 2015-02-05 4:33 ` Rich Felker 2015-02-05 16:14 ` Wolfgang Jenkner 2015-02-05 18:43 ` Rich Felker 2015-02-04 19:21 ` Eli Zaretskii 2015-02-04 19:37 ` Rich Felker 2015-02-04 19:44 ` Eli Zaretskii 2015-02-04 19:49 ` Rich Felker 2015-02-04 19:54 ` Eli Zaretskii 2015-02-04 20:02 ` Rich Felker 2015-02-04 20:36 ` Eli Zaretskii 2015-02-04 20:08 ` Rich Felker 2015-02-04 20:40 ` Eli Zaretskii 2015-02-04 22:17 ` Rich Felker 2015-02-05 3:42 ` Eli Zaretskii
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.