From 19dae293f9fecea07c91440f0a33076c54d39b49 Mon Sep 17 00:00:00 2001 From: Dmitry Antipov Date: Mon, 3 Mar 2014 12:27:58 +0400 Subject: [PATCH] Avoid crashes when X fonts are erroneously freed on reused X 'Display *' connection data (Bug#16069). Note that X font resources still may be leaked, but currently there is no way to completely avoid it. * xterm.h (struct x_display_info): New member x_id. Add comments. * xterm.c (x_display_id): New variable. (x_term_init): Assign identifier to each opened X connection. * xfont.c (struct xfont): New member x_display_id. (xfont_open): Initialize it with frame's display id. (xfont_close): Check whether font's display id matches the one recorded for the given display. Adjust comment. * xftfont.c (struct xftfont_info): (xftfont_open, xftfont_close): Exactly as above with xfont stuff. --- src/ChangeLog | 14 ++++++++++++++ src/xfont.c | 14 ++++++++++++-- src/xftfont.c | 6 +++++- src/xterm.c | 5 +++++ src/xterm.h | 4 ++++ 5 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 280077c3844..7bc69b2894e 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -6,6 +6,20 @@ (Fframe_font_cache) [FONT_DEBUG]: New function. (syms_of_font) [FONT_DEBUG]: Defsubr it. + Avoid crashes when X fonts are erroneously freed on reused X + 'Display *' connection data (Bug#16069). Note that X font + resources still may be leaked, but currently there is no way + to completely avoid it. + * xterm.h (struct x_display_info): New member x_id. Add comments. + * xterm.c (x_display_id): New variable. + (x_term_init): Assign identifier to each opened X connection. + * xfont.c (struct xfont): New member x_display_id. + (xfont_open): Initialize it with frame's display id. + (xfont_close): Check whether font's display id matches the one + recorded for the given display. Adjust comment. + * xftfont.c (struct xftfont_info): + (xftfont_open, xftfont_close): Exactly as above with xfont stuff. + 2014-03-01 Martin Rudalics Consider Vother_window_scroll_buffer valid iff it's a live buffer. diff --git a/src/xfont.c b/src/xfont.c index 3295a62bbb0..525f00031c5 100644 --- a/src/xfont.c +++ b/src/xfont.c @@ -42,6 +42,7 @@ struct xfont_info struct font font; Display *display; XFontStruct *xfont; + unsigned x_display_id; }; /* Prototypes of support functions. */ @@ -808,6 +809,7 @@ xfont_open (struct frame *f, Lisp_Object entity, int pixel_size) font = XFONT_OBJECT (font_object); ((struct xfont_info *) font)->xfont = xfont; ((struct xfont_info *) font)->display = FRAME_X_DISPLAY (f); + ((struct xfont_info *) font)->x_display_id = FRAME_DISPLAY_INFO (f)->x_id; font->pixel_size = pixel_size; font->driver = &xfont_driver; font->encoding_charset = encoding->id; @@ -892,12 +894,20 @@ xfont_open (struct frame *f, Lisp_Object entity, int pixel_size) static void xfont_close (struct font *font) { + struct x_display_info *xdi; struct xfont_info *xfi = (struct xfont_info *) font; /* This function may be called from GC when X connection is gone (Bug#16093), and an attempt to free font resources on invalid - display may lead to X protocol errors or segfaults. */ - if (xfi->xfont && x_display_info_for_display (xfi->display)) + display may lead to X protocol errors or segfaults. Moreover, + the memory referenced by 'Display *' pointer may be reused for + the logically different X connection after the previous display + connection was closed. That's why we also check whether font's + ID matches the one recorded in x_display_info for this display. + See http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16069. */ + if (xfi->xfont + && ((xdi = x_display_info_for_display (xfi->display)) + && xfi->x_display_id == xdi->x_id)) { block_input (); XFreeFont (xfi->display, xfi->xfont); diff --git a/src/xftfont.c b/src/xftfont.c index 32816b97f23..18c180f906a 100644 --- a/src/xftfont.c +++ b/src/xftfont.c @@ -59,6 +59,7 @@ struct xftfont_info FT_Matrix matrix; Display *display; XftFont *xftfont; + unsigned x_display_id; }; /* Structure pointed by (struct face *)->extra */ @@ -372,6 +373,7 @@ xftfont_open (struct frame *f, Lisp_Object entity, int pixel_size) xftfont_info = (struct xftfont_info *) font; xftfont_info->display = display; xftfont_info->xftfont = xftfont; + xftfont_info->x_display_id = FRAME_DISPLAY_INFO (f)->x_id; /* This means that there's no need of transformation. */ xftfont_info->matrix.xx = 0; if (FcPatternGetMatrix (xftfont->pattern, FC_MATRIX, 0, &matrix) @@ -481,6 +483,7 @@ xftfont_open (struct frame *f, Lisp_Object entity, int pixel_size) static void xftfont_close (struct font *font) { + struct x_display_info *xdi; struct xftfont_info *xftfont_info = (struct xftfont_info *) font; #ifdef HAVE_LIBOTF @@ -493,7 +496,8 @@ xftfont_close (struct font *font) /* See comment in xfont_close. */ if (xftfont_info->xftfont - && x_display_info_for_display (xftfont_info->display)) + && ((xdi = x_display_info_for_display (xftfont_info->display)) + && xftfont_info->x_display_id == xdi->x_id)) { block_input (); XftUnlockFace (xftfont_info->xftfont); diff --git a/src/xterm.c b/src/xterm.c index e1873127276..f06225002a7 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -9679,6 +9679,10 @@ my_log_handler (const gchar *log_domain, GLogLevelFlags log_level, } #endif +/* Current X display connection identifier. Incremented for each next + connection established. */ +static unsigned x_display_id; + /* Open a connection to X display DISPLAY_NAME, and return the structure that describes the open display. If we cannot contact the display, return null. */ @@ -9896,6 +9900,7 @@ x_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name) lim = min (PTRDIFF_MAX, SIZE_MAX) - sizeof "@"; if (lim - SBYTES (Vinvocation_name) < SBYTES (Vsystem_name)) memory_full (SIZE_MAX); + dpyinfo->x_id = ++x_display_id; dpyinfo->x_id_name = xmalloc (SBYTES (Vinvocation_name) + SBYTES (Vsystem_name) + 2); strcat (strcat (strcpy (dpyinfo->x_id_name, SSDATA (Vinvocation_name)), "@"), diff --git a/src/xterm.h b/src/xterm.h index 3e79d46702c..50df88cb592 100644 --- a/src/xterm.h +++ b/src/xterm.h @@ -198,6 +198,10 @@ struct x_display_info mouse-face. */ Mouse_HLInfo mouse_highlight; + /* Logical identifier of this display. */ + unsigned x_id; + + /* Default name for all frames on this display. */ char *x_id_name; /* The number of fonts opened for this display. */ -- 2.39.2