bug#58327: 29.0.50; Missing staticpro for old_selected_window in window.c

bug#58327: 29.0.50; Missing staticpro for old_selected_window in window.c

[Gerd Möllmann]

This was found while investigating bug#58024.

Since this is not directly related to bug#58024, and to make sure it's
not forgotten, I'm submitting this report separately.

Below is a copy a mail describing the problem.

From: Po Lu <luangruo@yahoo.com>
Subject: bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal
To: Gerd Möllmann <gerd.moellmann@gmail.com>
Cc: Eli Zaretskii <eliz@gnu.org>, 58042@debbugs.gnu.org, Alan Third
 <alan@idiocy.org>
Date: Wed, 05 Oct 2022 20:48:25 +0800 (16 hours, 20 minutes, 2 seconds ago)
Resent-From: Po Lu <luangruo@yahoo.com>

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> I don't get an abort, but the ASAN error again

Interesting.

> ==67682==ERROR: AddressSanitizer: heap-use-after-free on address 0x000107130d00 at pc 0x0001002a481c bp 0x00016fdcc3c0 sp 0x00016fdcc3b8
> READ of size 8 at 0x000107130d00 thread T0
>     #0 0x1002a4818 in PSEUDOVECTORP lisp.h:1110
>     #1 0x1002a4888 in SYMBOL_WITH_POS_P lisp.h:1122
>     #2 0x10025a338 in EQ lisp.h:1342
>     #3 0x100280eb0 in run_window_change_functions window.c:3964
>     #4 0x1000f18c4 in redisplay_internal xdisp.c:16600
>     #5 0x100107bf8 in redisplay xdisp.c:16111
>     #6 0x10089364c in -[EmacsView layoutSublayersOfLayer:] nsterm.m:8661
>     #7 0x1900a9624 in CA::Layer::layout_if_needed(CA::Transaction*)+0x224 (QuartzCore:arm64e+0x20624)
>     #8 0x1901f661c in CA::Context::commit_transaction(CA::Transaction*,
>     double, double*)+0x1c0 (QuartzCore:arm6
>
> frame #8: 0x0000000100280eb4 emacs`run_window_change_functions at window.c:3964:7
>    3961		     (de-)selected as its frame's or the globally selected
>    3962		     window.  */
>    3963		  if (((frame_selected_change
> -> 3964			&& (EQ (window, old_selected_window)
>    3965			    || EQ (window, selected_window)))
>    3966		       || (frame_selected_window_change
>    3967			   && (EQ (window, FRAME_OLD_SELECTED_WINDOW (f))
>
> (lldb) p window
> (Lisp_Object) $18 = 0x00000001071c2935 (struct window *) $23 = 0x00000001071c2930
> (lldb) p old_selected_window
> (Lisp_Object) $24 = 0x0000000107130d05 (struct Lisp_Vector *) $28 = 0x0000000107130d00
>
> old_selected_window looks strange.  It's a global that is not
> staticpro'd

Isn't old_selected_window supposed to be kept in sync with
FRAME_OLD_SELECTED_WINDOW in old_selected_frame, with the latter being
removed once it is deleted?

Would someone who knows the window code well please take a look at this?


The proposed fix was

From: Gerd Möllmann <gerd.moellmann@gmail.com>
Subject: bug#58042: 29.0.50; ASAN use-after-free in re_match_2_internal 
To: Po Lu <luangruo@yahoo.com>
Cc: Eli Zaretskii <eliz@gnu.org>, 58042@debbugs.gnu.org, Alan Third
 <alan@idiocy.org>
Date: Wed, 05 Oct 2022 14:38:24 +0200 (16 hours, 37 minutes, 19 seconds ago)
Resent-From: Gerd Möllmann <gerd.moellmann@gmail.com>

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> old_selected_window looks strange.  It's a global that is not
> staticpro'd

And with this it works again:

diff --git a/src/window.c b/src/window.c
index 12a212a85a..da80fabe33 100644
--- a/src/window.c
+++ b/src/window.c
@@ -8213,6 +8213,8 @@ init_window_once (void)
 
   minibuf_selected_window = Qnil;
   staticpro (&minibuf_selected_window);
+  old_selected_window = Qnil;
+  staticpro (&old_selected_window);
 
   pdumper_do_now_and_after_late_load (init_window_once_for_pdumper);
 }

bug#58327: 29.0.50; Missing staticpro for old_selected_window in window.c

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Thu, 06 Oct 2022 07:16:29 +0200
> 
> This was found while investigating bug#58024.
> 
> Since this is not directly related to bug#58024, and to make sure it's
> not forgotten, I'm submitting this report separately.

Thanks.

Martin, any comments?

bug#58327: 29.0.50; Missing staticpro for old_selected_window in window.c

[martin rudalics]

 >> This was found while investigating bug#58024.
 >>
 >> Since this is not directly related to bug#58024, and to make sure it's
 >> not forgotten, I'm submitting this report separately.
 >
 > Thanks.
 >
 > Martin, any comments?

Gerd is right.

martin

bug#58327: 29.0.50; Missing staticpro for old_selected_window in window.c

[Eli Zaretskii]

> Date: Thu, 6 Oct 2022 09:56:01 +0200
> Cc: 58327@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  >> This was found while investigating bug#58024.
>  >>
>  >> Since this is not directly related to bug#58024, and to make sure it's
>  >> not forgotten, I'm submitting this report separately.
>  >
>  > Thanks.
>  >
>  > Martin, any comments?
> 
> Gerd is right.

Thanks, then let's install that staticpro.

bug#58327: 29.0.50; Missing staticpro for old_selected_window in window.c

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, then let's install that staticpro.

Done in master.  And closing.