unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* 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 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: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: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 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 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: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 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 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-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

* 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

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 public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).