Blob Blame History Raw
From 77b8fc885e582a21c0f3f00f9120c948d4fa424b Mon Sep 17 00:00:00 2001
From: Mamoru TASAKA <mtasaka@fedoraproject.org>
Date: Tue, 24 Mar 2020 14:26:43 +0900
Subject: [PATCH] free_gibson: fix order of freeing memory

gcc10 -fsanitize=address detects use-after-free on free_gibsonn:
=================================================================
==49579==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d000021c8c at pc 0x00000040da99 bp 0x7ffdb9dfea00 sp 0x7ffdb9dfe9f0
READ of size 4 at 0x61d000021c8c thread T0
    #0 0x40da98 in free_gibson ../../../hacks/glx/gibson.c:1323
    #1 0x4256af in xlockmore_free ../../hacks/xlockmore.c:724
    #2 0x40aaeb in run_screenhack_table ../../hacks/screenhack.c:614
    #3 0x40aaeb in main ../../hacks/screenhack.c:991
    #4 0x7f6aab9b81a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
    #5 0x40c09d in _start (/home/mtasaka/rpmbuild/fedora-specific/xscreensaver/master/xscreensaver-5.44/x86_64-pc-linux-gnu/hacks/glx/gibson+0x40c09d)

0x61d000021c8c is located 12 bytes inside of 2304-byte region [0x61d000021c80,0x61d000022580)
freed by thread T0 here:
    #0 0x7f6aacc4a357 in __interceptor_free (/lib64/libasan.so.6+0xb0357)
    #1 0x40d21a in free_gibson ../../../hacks/glx/gibson.c:1309

previously allocated by thread T0 here:
    #0 0x7f6aacc4a887 in __interceptor_calloc (/lib64/libasan.so.6+0xb0887)
    #1 0x41321c in init_gibson ../../../hacks/glx/gibson.c:1028

This patch fixes order of freeing memory.
---
 hacks/glx/gibson.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/hacks/glx/gibson.c b/hacks/glx/gibson.c
index 8f99040..05b16a7 100644
--- a/hacks/glx/gibson.c
+++ b/hacks/glx/gibson.c
@@ -1306,7 +1306,6 @@ free_gibson (ModeInfo *mi)
   if (!bp->glx_context) return;
   glXMakeCurrent(MI_DISPLAY(mi), MI_WINDOW(mi), *bp->glx_context);
 
-  if (bp->towers) free (bp->towers);
   if (bp->rot)  free_rotator (bp->rot);
   if (bp->rot2) free_rotator (bp->rot2);
   if (glIsList(bp->ground_dlist)) glDeleteLists(bp->ground_dlist, 1);
@@ -1316,15 +1315,19 @@ free_gibson (ModeInfo *mi)
       if (bp->text[i].font_data) free_texture_font (bp->text[i].font_data);
       if (bp->text[i].text) free (bp->text[i].text);
     }
-  for (i = 0; i < bp->ntowers; i++)
+  if (bp->towers)
     {
-      for (j = 0; j < countof(bp->towers[i].fg_dlists); j++)
+      for (i = 0; i < bp->ntowers; i++)
         {
-          if (glIsList(bp->towers[i].fg_dlists[j]))
-            glDeleteLists(bp->towers[i].fg_dlists[j], 1);
-          if (glIsList(bp->towers[i].bg_dlists[j]))
-            glDeleteLists(bp->towers[i].bg_dlists[j], 1);
+          for (j = 0; j < countof(bp->towers[i].fg_dlists); j++)
+            {
+              if (glIsList(bp->towers[i].fg_dlists[j]))
+                glDeleteLists(bp->towers[i].fg_dlists[j], 1);
+              if (glIsList(bp->towers[i].bg_dlists[j]))
+                glDeleteLists(bp->towers[i].bg_dlists[j], 1);
+            }
         }
+      free (bp->towers);
     }
 }
 
-- 
2.25.2