GNU bug report logs - #18140
[PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes wrong_type_argument for Qnil instead of return 0

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 18140 in the body.
You can then email your comments to 18140 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: Jan Chaloupka <jchaloup <at> redhat.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes wrong_type_argument
 for Qnil instead of return 0
Date: Tue, 29 Jul 2014 07:01:58 +0200

In function Fstart_kbd_macro (macros.c), Vlast_kbd_macro of current_kboard is Qnil for the first invocation.
If NILP (append) is false, current_kboard->kbd_macro_ptr has random value (in our case 0x5353535353535353),
which after CHECK_VECTOR_OR_STRING failure (invocation of wrong_type_argument) results in garbage collecting.
During gc, marking of objects is processed and mark_kboards (keyboard.c) is invoked. Following for loop is fired:

for (p = kb->kbd_macro_buffer; p < kb->kbd_macro_ptr; p++)
                  mark_object (*p);

Since kb->kbd_macro_ptr is set to 0x5353535353535353, mark_object (*p) is trying to mark object on address
out of memory space (or memory that cannot be accessed). Thus resulting in SIGSEGV signal.

Solution is to check for Qnil before calling CHECK_VECTOR_OR_STRING and set len to 0 if Qnil occurs.

https://bugzilla.redhat.com/show_bug.cgi?id=1104012

Signed-off-by: Jan Chaloupka <jchaloup <at> redhat.com>
---
 src/macros.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/macros.c b/src/macros.c
index 4730a8b..219eb39 100644
--- a/src/macros.c
+++ b/src/macros.c
@@ -85,7 +85,23 @@ macro before appending to it.  */)
       bool cvt;
 
       /* Check the type of last-kbd-macro in case Lisp code changed it.  */
-      len = CHECK_VECTOR_OR_STRING (KVAR (current_kboard, Vlast_kbd_macro));
+      /* If Vlast_kbd_macro is Qnil, skip the check and set len to 0.
+       * Flength returns 0 for Qnil, CHECK_VECTOR_OR_STRING has to do the same.
+       * Otherwise CHECK_VECTOR_OR_STRING fails and results in garbage collecting,
+       * which results in (keyboard.c, mark_kboards(void))
+       *
+       * for (p = kb->kbd_macro_buffer; p < kb->kbd_macro_ptr; p++)
+       * 	  mark_object (*p);
+       *
+       * Here, kb->kbd_macro_ptr is not initialized and can contain address
+       * 0x5353535353535353, which results in SIGSEGV trying to access the address.
+       *
+       * https://bugzilla.redhat.com/show_bug.cgi?id=1104012
+       */
+      if (!NILP (KVAR (current_kboard, Vlast_kbd_macro) ))
+        len = CHECK_VECTOR_OR_STRING (KVAR (current_kboard, Vlast_kbd_macro));
+      else
+        len = 0;
 
       /* Copy last-kbd-macro into the buffer, in case the Lisp code
 	 has put another macro there.  */

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

From: Jan Chaloupka <jchaloup <at> redhat.com>
To: 18140 <at> debbugs.gnu.org
Subject: [PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes wrong_type_argument
 for Qnil instead of return 0
Date: Tue, 29 Jul 2014 07:35:03 +0200

Changelog:
	line wrapping to 80 characters

In function Fstart_kbd_macro (macros.c), Vlast_kbd_macro of current_kboard is
Qnil for the first invocation. If NILP (append) is false,
current_kboard->kbd_macro_ptr has random value (in our case
0x5353535353535353), which after CHECK_VECTOR_OR_STRING failure (invocation
of wrong_type_argument) results in garbage collecting.
During gc, marking of objects is processed and mark_kboards (keyboard.c) is
invoked. Following for loop is fired:

for (p = kb->kbd_macro_buffer; p < kb->kbd_macro_ptr; p++)
                  mark_object (*p);

Since kb->kbd_macro_ptr is set to 0x5353535353535353, mark_object (*p) is
trying to mark object on address out of memory space (or memory that
cannot be accessed). Thus resulting in SIGSEGV signal.

Solution is to check for Qnil before calling CHECK_VECTOR_OR_STRING and set
len to 0 if Qnil occurs.

https://bugzilla.redhat.com/show_bug.cgi?id=1104012

Signed-off-by: Jan Chaloupka <jchaloup <at> redhat.com>
---
 src/macros.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/macros.c b/src/macros.c
index 4730a8b..4fd6cb1 100644
--- a/src/macros.c
+++ b/src/macros.c
@@ -85,7 +85,25 @@ macro before appending to it.  */)
       bool cvt;
 
       /* Check the type of last-kbd-macro in case Lisp code changed it.  */
-      len = CHECK_VECTOR_OR_STRING (KVAR (current_kboard, Vlast_kbd_macro));
+      /* If Vlast_kbd_macro is Qnil, skip the check and set len to 0.
+       * Flength returns 0 for Qnil, CHECK_VECTOR_OR_STRING has to do the same.
+       * Otherwise CHECK_VECTOR_OR_STRING fails and results in garbage
+       * collecting, which results in (keyboard.c, mark_kboards(void)).
+       * Among others, mark_kboards it executes:
+       *
+       * for (p = kb->kbd_macro_buffer; p < kb->kbd_macro_ptr; p++)
+       * 	  mark_object (*p);
+       *
+       * Here, kb->kbd_macro_ptr is not initialized and can contain address
+       * 0x5353535353535353, which results in SIGSEGV trying to access
+       * the address.
+       *
+       * https://bugzilla.redhat.com/show_bug.cgi?id=1104012
+       */
+      if (!NILP (KVAR (current_kboard, Vlast_kbd_macro) ))
+        len = CHECK_VECTOR_OR_STRING (KVAR (current_kboard, Vlast_kbd_macro));
+      else
+        len = 0;
 
       /* Copy last-kbd-macro into the buffer, in case the Lisp code
 	 has put another macro there.  */

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

From: Andreas Schwab <schwab <at> suse.de>
To: Jan Chaloupka <jchaloup <at> redhat.com>
Cc: 18140 <at> debbugs.gnu.org
Subject: Re: bug#18140: [PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes
 wrong_type_argument for Qnil instead of return 0
Date: Tue, 29 Jul 2014 09:52:34 +0200

Jan Chaloupka <jchaloup <at> redhat.com> writes:

> Changelog:
> 	line wrapping to 80 characters
>
> In function Fstart_kbd_macro (macros.c), Vlast_kbd_macro of current_kboard is
> Qnil for the first invocation. If NILP (append) is false,
> current_kboard->kbd_macro_ptr has random value (in our case
> 0x5353535353535353), which after CHECK_VECTOR_OR_STRING failure (invocation
> of wrong_type_argument) results in garbage collecting.
> During gc, marking of objects is processed and mark_kboards (keyboard.c) is
> invoked. Following for loop is fired:
>
> for (p = kb->kbd_macro_buffer; p < kb->kbd_macro_ptr; p++)
>                   mark_object (*p);
>
> Since kb->kbd_macro_ptr is set to 0x5353535353535353, mark_object (*p) is
> trying to mark object on address out of memory space (or memory that
> cannot be accessed). Thus resulting in SIGSEGV signal.

So the correct solution is to initialize kbd_macro_ptr together with
kbd_macro_buffer.  Otherwise the same situation can still happen any
time garbage collection is called.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Message #16 received at 18140-done <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> suse.de>
To: Jan Chaloupka <jchaloup <at> redhat.com>
Cc: 18140-done <at> debbugs.gnu.org
Subject: Re: bug#18140: [PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes
 wrong_type_argument for Qnil instead of return 0
Date: Tue, 29 Jul 2014 10:10:11 +0200

Fixed for emacs 24.4.  Thanks for the report.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

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

From: Jan Chaloupka <jchaloup <at> redhat.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#18140: [PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes
 wrong_type_argument for Qnil instead of return 0
Date: Tue, 29 Jul 2014 10:23:52 +0200

On 07/29/2014 09:52 AM, Andreas Schwab wrote:
> Jan Chaloupka <jchaloup <at> redhat.com> writes:
>
>> Changelog:
>> 	line wrapping to 80 characters
>>
>> In function Fstart_kbd_macro (macros.c), Vlast_kbd_macro of current_kboard is
>> Qnil for the first invocation. If NILP (append) is false,
>> current_kboard->kbd_macro_ptr has random value (in our case
>> 0x5353535353535353), which after CHECK_VECTOR_OR_STRING failure (invocation
>> of wrong_type_argument) results in garbage collecting.
>> During gc, marking of objects is processed and mark_kboards (keyboard.c) is
>> invoked. Following for loop is fired:
>>
>> for (p = kb->kbd_macro_buffer; p < kb->kbd_macro_ptr; p++)
>>                    mark_object (*p);
>>
>> Since kb->kbd_macro_ptr is set to 0x5353535353535353, mark_object (*p) is
>> trying to mark object on address out of memory space (or memory that
>> cannot be accessed). Thus resulting in SIGSEGV signal.
> So the correct solution is to initialize kbd_macro_ptr together with
> kbd_macro_buffer.  Otherwise the same situation can still happen any
> time garbage collection is called.
Yes, for garbage collector. However, Vlast_kbd_macro will continue being 
Qnil.
The patch is still valid (just without comment about random value of
kbd_macro_ptr and garbage collection):

In function Fstart_kbd_macro (macros.c), Vlast_kbd_macro of 
current_kboard is
Qnil for the first invocation. If NILP (append) is false
CHECK_VECTOR_OR_STRING fails (invocation
of wrong_type_argument resulting in emacs_abort). However, it has to pass.

> Andreas.
>

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

From: Andreas Schwab <schwab <at> suse.de>
To: Jan Chaloupka <jchaloup <at> redhat.com>
Cc: 18140 <at> debbugs.gnu.org
Subject: Re: bug#18140: [PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes
 wrong_type_argument for Qnil instead of return 0
Date: Tue, 29 Jul 2014 10:37:42 +0200

Jan Chaloupka <jchaloup <at> redhat.com> writes:

> Yes, for garbage collector. However, Vlast_kbd_macro will continue being
> Qnil.

Why is that a problem?  That is the default value.

> In function Fstart_kbd_macro (macros.c), Vlast_kbd_macro of current_kboard
> is
> Qnil for the first invocation. If NILP (append) is false
> CHECK_VECTOR_OR_STRING fails (invocation
> of wrong_type_argument resulting in emacs_abort).

Where does it call emacs_abort?

> However, it has to pass.

Why?

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

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

From: Jan Chaloupka <jchaloup <at> redhat.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#18140: [PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes
 wrong_type_argument for Qnil instead of return 0
Date: Tue, 29 Jul 2014 10:55:20 +0200

On 07/29/2014 10:37 AM, Andreas Schwab wrote:
> Jan Chaloupka <jchaloup <at> redhat.com> writes:
>
>> Yes, for garbage collector. However, Vlast_kbd_macro will continue being
>> Qnil.
> Why is that a problem?  That is the default value.
So is it correct if append and Vlast_kbd_macro are both Qnil resulting 
in CHECK_VECTOR_OR_STRING fail?
The bug from BZ is use-case where emacs crashes at startup, loading 
.emacs.desktop file.
>> In function Fstart_kbd_macro (macros.c), Vlast_kbd_macro of current_kboard
>> is
>> Qnil for the first invocation. If NILP (append) is false
>> CHECK_VECTOR_OR_STRING fails (invocation
>> of wrong_type_argument resulting in emacs_abort).
> Where does it call emacs_abort?
Because Vlast_kbd_macro is not VECTOR nor STRING
>
>> However, it has to pass.
> Why?
My first question in this response.
>
> Andreas.
>

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

From: Andreas Schwab <schwab <at> suse.de>
To: Jan Chaloupka <jchaloup <at> redhat.com>
Cc: 18140 <at> debbugs.gnu.org
Subject: Re: bug#18140: [PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes
 wrong_type_argument for Qnil instead of return 0
Date: Tue, 29 Jul 2014 11:04:51 +0200

Jan Chaloupka <jchaloup <at> redhat.com> writes:

> So is it correct if append and Vlast_kbd_macro are both Qnil resulting in
> CHECK_VECTOR_OR_STRING fail?

Sure, that's the point of the check.  last-kbd-macro is a lisp-level
variable, so it must be checked.

> Because Vlast_kbd_macro is not VECTOR nor STRING

That just calls error, but not emacs_abort.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

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

From: Jan Chaloupka <jchaloup <at> redhat.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#18140: [PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes
 wrong_type_argument for Qnil instead of return 0
Date: Tue, 29 Jul 2014 11:59:38 +0200

On 07/29/2014 11:04 AM, Andreas Schwab wrote:
> Jan Chaloupka <jchaloup <at> redhat.com> writes:
>
>> So is it correct if append and Vlast_kbd_macro are both Qnil resulting in
>> CHECK_VECTOR_OR_STRING fail?
> Sure, that's the point of the check.  last-kbd-macro is a lisp-level
> variable, so it must be checked.

Yes, I agree it has to be check. Having .emacs.desktop file with series
of the folkowing kbd macro definitions:

(desktop-create-buffer 206
    ...
    '(defining-kbd-macro global-auto-revert-mode)
    ...
)

append argument of start-kbd-macro is false. But because there is no 
last kbd macro,
check fails. I guess then .emacs.desktop is incorectly written. Thus 
resulting in
proper check fail.

>> Because Vlast_kbd_macro is not VECTOR nor STRING
> That just calls error, but not emacs_abort.
CHECK_VECTOR_OR_STRING -> wrong_type_argument -> xsignal2 -> xsignal

void
xsignal (Lisp_Object error_symbol, Lisp_Object data)
{
  Fsignal (error_symbol, data);
  emacs_abort ();
}

emacs_abort then has to be called after Fsignal finished. Or is there a 
back jmp back to main loop or somewhere else?
> Andreas.
>

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

From: Andreas Schwab <schwab <at> suse.de>
To: Jan Chaloupka <jchaloup <at> redhat.com>
Cc: 18140 <at> debbugs.gnu.org
Subject: Re: bug#18140: [PATCH] macros.c: CHECK_VECTOR_OR_STRING invokes
 wrong_type_argument for Qnil instead of return 0
Date: Tue, 29 Jul 2014 12:05:58 +0200

Jan Chaloupka <jchaloup <at> redhat.com> writes:

> emacs_abort then has to be called after Fsignal finished.

Fsignal never returns.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Previous Next

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