Subject: bug#77046: 31.0.50; Emacs sometimes crashes

From: Eli Zaretskii <[email-masked]>
Date: Sat, 22 Mar 2025 17:55:41 +0200

> Date: Sat, 22 Mar 2025 12:20:16 +0000
> From: Pip Cet <[email-masked]>
> Cc: Eli Zaretskii <[email-masked]>, [email-masked], [email-masked]
> 
> > #6 0x000055555566e969 in tty_lookup_color (f=f@entry=0x555555aeff60,
> > color=0x555555a008e4, tty_color=tty_color@entry=0x7fffffffc720,
> > std_color=std_color@entry=0x0) at xfaces.c:1090
> > #7 0x0000555555675d8c in tty_defined_color (f=0x555555aeff60,
> > color_name=0x555555ae1270 "unspecified-bg", color_def=0x7fffffffc720,
> > alloc=<optimized out>, _makeIndex=<optimized out>) at xfaces.c:1153
> > #8 0x000055555566e426 in load_color2 (f=0x555555aeff60,
> > face=0x555555a6d370, name=0x555555a00634,
> > target_index=LFACE_BACKGROUND_INDEX, color=0x7fffffffc720) at
> > xfaces.c:1300
> 
> That line is:
> 
>   if (!FRAME_TERMINAL (f)->defined_color_hook
>       (f, SSDATA (name), color, true, true))
> 
> which looks very unsafe to me: the SSDATA pointer will become invalid
> when we compact strings, which we might do since tty_lookup_color calls
> into Lisp.  The color_name pointer is used after that point in
> tty_defined_color, possibly causing false negatives for the strcmp in
> lines 1157/1159.
> 
> However, that doesn't really explain this crash, it's just another
> (possibly latent) SDATA bug.  At first glance, there seem to be more of
> hose in xfaces.c, though, so it's possible something further up the call
> chain results in a corrupt SDATA pointer.

Does the below look OK?  I think it solves several SSDATA issues,
since tty_defined_color is called from several other places that pass
it SSDATA of some Lisp string.  But if you see some other place where
similar problems could happen and are not solved by the patch below,
please point them out.

diff --git a/src/xfaces.c b/src/xfaces.c
index fbbaffb..7a4571c 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -1150,14 +1150,18 @@ tty_defined_color (struct frame *f, const char *color_name,
   color_def->green = 0;
 
   if (*color_name)
-    status = tty_lookup_color (f, build_string (color_name), color_def, NULL);
-
-  if (color_def->pixel == FACE_TTY_DEFAULT_COLOR && *color_name)
     {
-      if (strcmp (color_name, "unspecified-fg") == 0)
-	color_def->pixel = FACE_TTY_DEFAULT_FG_COLOR;
-      else if (strcmp (color_name, "unspecified-bg") == 0)
-	color_def->pixel = FACE_TTY_DEFAULT_BG_COLOR;
+      Lisp_Object lcolor = build_string (color_name);
+      status = tty_lookup_color (f, lcolor, color_def, NULL);
+
+      if (color_def->pixel == FACE_TTY_DEFAULT_COLOR)
+	{
+	  color_name = SSDATA (lcolor);
+	  if (strcmp (color_name, "unspecified-fg") == 0)
+	    color_def->pixel = FACE_TTY_DEFAULT_FG_COLOR;
+	  else if (strcmp (color_name, "unspecified-bg") == 0)
+	    color_def->pixel = FACE_TTY_DEFAULT_BG_COLOR;
+	}
     }
 
   if (color_def->pixel != FACE_TTY_DEFAULT_COLOR)