[X2Go-Commits] [nx-libs] 51/429: NXdixfonts.c: fix memory leak

[git-admin at x2go.org]

This is an automated email from the git hooks/post-receive script.

x2go pushed a commit to branch 3.6.x
in repository nx-libs.

commit db45683a6317a8712bbdbff0615d1b8e39b5d57c
Author: Ulrich Sibiller <uli42 at gmx.de>
Date:   Sun Oct 18 01:48:34 2020 +0200

    NXdixfonts.c: fix memory leak
    
    ==15332== 2,500 (96 direct, 2,404 indirect) bytes in 6 blocks are definitely lost in loss record 324 of 342
    ==15332==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==15332==    by 0x5748B9E: FontFileStartListFonts (in /usr/lib/x86_64-linux-gnu/libXfont.so.1.4.1)
    ==15332==    by 0x5748C4A: FontFileStartListFontsAndAliases (in /usr/lib/x86_64-linux-gnu/libXfont.so.1.4.1)
    ==15332==    by 0x42859A: nxdoListFontsAndAliases (NXdixfonts.c:1163)
    ==15332==    by 0x42C0E0: nxOpenFont (NXdixfonts.c:1541)
    ==15332==    by 0x43392E: ProcOpenFont (NXdispatch.c:902)
    ==15332==    by 0x434585: Dispatch (NXdispatch.c:482)
    ==15332==    by 0x40EF77: main (main.c:355)
    
    FontFileStartListFonts[AndAliases]() allocates some private data. This
    data is used by subsequent calls of FontFileListNextFontOrAlias() in a
    loop. (Only) the last call to that function will free() the private
    data and return with BadFontName.  FontFileListNextFontOrAlias() is
    the only libXfont function that free()s the private data.
    
    In nxagent the loop is exited as soon as a font exists both locally
    and remote. Therefore the private data would never be free()d.
    
    Solution: do not break the loop but store the first matching result
    and let the loop run to the end, ignoring all following results.
    
    Disadvantage: this can mean hundreds of extra iterations for
    nothing. I have done no investigation of the time penalty this might
    cause.
    
    Unfortunately this is the only clean way I have found so far.
    
    An unclean solution has also been implemented. It can be activated by
    defining BREAK_XFONT_LOOP. In that case the private data is handled in
    nxagent by taking assumptions about its structure (taken from the
    libXfont source). That will break if libXfont changes its internal
    handling of the private. Therefore it is discouraged.
    
    An third alternative would be to drop using libXfont from the
    system. Instead fork libXfont to the nx-libs tree, add some patches
    link to that library statically.
    
    Fixes ArcticaProject/nx-libs#586
---
 nx-X11/programs/Xserver/hw/nxagent/NXdixfonts.c | 62 ++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 6 deletions(-)

diff --git a/nx-X11/programs/Xserver/hw/nxagent/NXdixfonts.c b/nx-X11/programs/Xserver/hw/nxagent/NXdixfonts.c
index 6bb4ea306..516b7eb02 100644
--- a/nx-X11/programs/Xserver/hw/nxagent/NXdixfonts.c
+++ b/nx-X11/programs/Xserver/hw/nxagent/NXdixfonts.c
@@ -924,6 +924,9 @@ doListFontsWithInfo(ClientPtr client, LFWIclosurePtr c)
 	  ContBadFontName: ;
 	    c->current.list_started = FALSE;
 	    c->current.current_fpe++;
+#ifdef NXAGENT_SERVER
+	    c->current.private = 0;   /* BadFontName -> private has been freed */
+#endif
 	    err = Successful;
 	    if (c->haveSaved)
  	    {
@@ -1070,6 +1073,7 @@ nxdoListFontsAndAliases(ClientPtr client, nxFsPtr fss)
     int         i;
     int		aliascount = 0;
     char        tmp[256];
+
     tmp[0] = 0;
     if (client->clientGone)
     {
@@ -1195,6 +1199,13 @@ nxdoListFontsAndAliases(ClientPtr client, nxFsPtr fss)
 
 	    if (err == Successful)
 	    {
+#ifndef BREAK_XFONT_LOOP
+	        if (tmp[0] != 0)
+	        {
+		  continue;
+	        }
+#endif
+
 		if (c->haveSaved)
 		{
 		    if (c->savedName)
@@ -1202,8 +1213,14 @@ nxdoListFontsAndAliases(ClientPtr client, nxFsPtr fss)
 		       memcpy(tmp, c->savedName, c->savedNameLen > 255 ? 255 : c->savedNameLen);
 		       tmp[c->savedNameLen >255 ? 255 : c->savedNameLen] = 0;
 		       if (nxagentFontLookUp(tmp))
-		          break;
-			else tmp[0] = 0;
+		       {
+#ifdef BREAK_XFONT_LOOP
+			 break;
+#else
+			 continue;
+#endif
+		       }
+		       else tmp[0] = 0;
 		    }
 		}
 		else
@@ -1211,7 +1228,13 @@ nxdoListFontsAndAliases(ClientPtr client, nxFsPtr fss)
 		   memcpy(tmp, name, namelen > 255 ? 255 : namelen);
 		   tmp[namelen > 255 ? 255 : namelen] = 0;
 		   if (nxagentFontLookUp(tmp))
-		      break;
+		   {
+#ifdef BREAK_XFONT_LOOP
+		     break;
+#else
+		     continue;
+#endif
+		   }
 		   else tmp[0] = 0;
 		}
 	    }
@@ -1282,6 +1305,10 @@ nxdoListFontsAndAliases(ClientPtr client, nxFsPtr fss)
 	  ContBadFontName: ;
 	    c->current.list_started = FALSE;
 	    c->current.current_fpe++;
+#ifdef NXAGENT_SERVER
+	    /* clearing a freed pointer helps for debugging */
+	    c->current.private = 0;  /* BadFontName means private has been freed */
+#endif
 	    err = Successful;
 	    if (c->haveSaved)
 	    {
@@ -1298,11 +1325,34 @@ nxdoListFontsAndAliases(ClientPtr client, nxFsPtr fss)
 	}
     }
 
-    /*
-     * send the reply
-     */
 bail:
 finish:
+#ifdef BREAK_XFONT_LOOP
+    /* if we allow above loop to be exited via break
+       we need to free the private xfont data somehow. */
+    if (c->current.list_started)
+    {
+      /* WARNING: this codes makes assumptions about an internal
+	 private structure of libXfont and can therefore break with
+	 ANY libXfont update! */
+      typedef struct _LFWIData {
+	FontNamesPtr    names;
+	int                   current;
+      } LFWIDataRec, *LFWIDataPtr;
+
+      LFWIDataPtr data = c->current.private;
+      if (data)
+      {
+#ifdef HAS_XFONT2
+        xfont2_free_font_names(data->names);
+#else
+        FreeFontName(data->names);
+#endif
+        free(data);
+      }
+    }
+#endif
+
     if (strlen(tmp))
     {
 #ifdef NXAGENT_FONTMATCH_DEBUG

--
Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/nx-libs.git