GNU bug report logs - #30481
26.0.91; infinite recursion + edebug = memory corruption

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 30481 in the body.
You can then email your comments to 30481 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.91; infinite recursion + edebug = memory corruption
Date: Thu, 15 Feb 2018 22:38:10 -0500

[Message part 1 (text/plain, inline)]

Tags: patch

Picking up on a side issue from Bug#30243:

>>> emacs: malloc.c:2427: sysmalloc: Assertion `(old_top == initial_top
>>> (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE &&
>>> prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1))
>>> == 0)' failed.
>>> Fatal error 6: Aborted

> The problem appears to be that we hit the limit in grow_specpdl(),
> and then call signal_error which does Ffuncall and then
> record_in_backtrace writes to specdl, this latter write is invalid
> since we failed to grow specdl before.  Thus memory corruption,
> undefined behaviour, etc.
>
> #0  0x000000000063999d in record_in_backtrace (function=XIL(0xd9ea380), args=0xffef5b188, nargs=2)
>     at ../../src/eval.c:2096
> #1  0x000000000063b8c9 in Ffuncall (nargs=3, args=0xffef5b180) at ../../src/eval.c:2746
> #2  0x000000000063b320 in call2 (fn=XIL(0xd9ea380), arg1=XIL(0x5250), arg2=XIL(0x1161fc03))
>     at ../../src/eval.c:2625
> #3  0x00000000006381db in signal_or_quit (error_symbol=XIL(0x5250), data=XIL(0x1161fc03), 
>     keyboard_quit=false) at ../../src/eval.c:1565
> #4  0x000000000063806d in Fsignal (error_symbol=XIL(0x5250), data=XIL(0x1161fc03))
>     at ../../src/eval.c:1514
> #5  0x000000000057939a in xsignal (error_symbol=XIL(0x5250), data=XIL(0x1161fc03))
>     at ../../src/lisp.h:3861
> #6  0x0000000000638704 in signal_error (s=0x75e388 "Variable binding depth exceeds max-specpdl-size", 
>     arg=XIL(0)) at ../../src/eval.c:1688
> #7  0x00000000006398cd in grow_specpdl () at ../../src/eval.c:2080
> (More stack frames follow...)

A simple reproducer from emacs -Q, C-u C-M-x on the following:

    (defun foo ()
      (let ((x 1))
        (foo)))

then evaluate (foo) and git 'g' to continue until the "Variable binding
depth exceeds max-specpdl-size" error.  At that point the memory
corruption has happened (verified with valgrind), although I found I had
to split window to actually trigger the malloc assertion.

The following patch solves the problem by not calling
signal-hook-function when the specpdl array is exhausted.  I think it
could be safe for emacs-26.

[v1-0001-Avoid-memory-corruption-with-lisp-stack-overflow-.patch (text/x-diff, inline)]

From c9a183b31dce87803dad3d5feccf561fe3f63c9b Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Thu, 15 Feb 2018 22:13:51 -0500
Subject: [PATCH v1] Avoid memory corruption with lisp stack overflow + edebug

If grow_specpdl fails due to outgrowing max_specpdl_size, it will
signal an error *before* growing the specpdl array.  Therefore, when
handling the signal, specpdl_ptr points past the end of the specpdl
array and any further use of of specpdl before unwinding (e.g., if
edebug binds signal-hook-function) will cause memory corruption.
* src/eval.c (signal_or_quit): Don't call `signal-hook-function' if
the specpdl_ptr is already past the end of the specpdl array.
---
 src/eval.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/eval.c b/src/eval.c
index e05a17f7b4..ca1eb84ff3 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1553,7 +1553,10 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool keyboard_quit)
 
   /* This hook is used by edebug.  */
   if (! NILP (Vsignal_hook_function)
-      && ! NILP (error_symbol))
+      && ! NILP (error_symbol)
+      /* Don't try to call a lisp function if we've already overflowed
+         the specpdl stack.  */
+      && specpdl_ptr < specpdl + specpdl_size)
     {
       /* Edebug takes care of restoring these variables when it exits.  */
       if (lisp_eval_depth + 20 > max_lisp_eval_depth)
-- 
2.11.0

Message #8 received at 30481 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 30481 <at> debbugs.gnu.org
Subject: Re: bug#30481: 26.0.91;
 infinite recursion + edebug = memory corruption
Date: Fri, 16 Feb 2018 10:39:08 +0200

> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Thu, 15 Feb 2018 22:38:10 -0500
> 
> The following patch solves the problem by not calling
> signal-hook-function when the specpdl array is exhausted.  I think it
> could be safe for emacs-26.

Please push to emacs-26, and thanks.

(Is it practical to have a test for this?)

Message #11 received at 30481 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 30481 <at> debbugs.gnu.org
Subject: Re: bug#30481: 26.0.91;
 infinite recursion + edebug = memory corruption
Date: Fri, 16 Feb 2018 22:30:33 -0500

tags 30481 fixed
close 30481 26.1
quit

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Noam Postavsky <npostavs <at> gmail.com>
>> Date: Thu, 15 Feb 2018 22:38:10 -0500
>> 
>> The following patch solves the problem by not calling
>> signal-hook-function when the specpdl array is exhausted.  I think it
>> could be safe for emacs-26.
>
> Please push to emacs-26, and thanks.

Pushed (with test) [1: c352434ab8].

> (Is it practical to have a test for this?)

Yes, actually.  I initially had some trouble reproducing without
instrumenting a function with edebug, but now I see that's just because
a function which let-binds only a single variable hits
max-lisp-eval-depth before max-specpdl-size (edebug's intrumentation
adds more bindings per call).  Let-binding two variables allows to
trigger the bug with just

    (defun foo ()
      (let ((x 1)
            (y 2))
        (foo)))

    (let ((signal-hook-function #'ignore))
      (foo))

[1: c352434ab8]: 2018-02-16 22:13:34 -0500
  Avoid memory corruption with specpdl overflow + edebug (Bug#30481)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=c352434ab89617b48c7c1f29342a22e5a5685504

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.