patches/26.defcpdtable_leak.patch

Change 542666043 by sungyc@sungyc:fig-export-icing-153-change-361:5566:citc on 2023/06/22 14:19:55

	[hunspell] Fix defcpdtable memory leak error in AffixMgr
	
	## Test plan
	```
	sso_client -location 'https://clusterfuzz.corp.google.com/testcase-detail/download-testcase?id=4667620284301312' > /tmp/testcase-4667620284301312 && \
	blaze --blazerc=/dev/null test --config=asan-fuzzer --test_strategy=local --test_sharding_strategy=disabled \
	  --test_env=ENABLE_BLAZE_TEST_FUZZING=1 --test_arg=-rss_limit_mb=2560 --test_arg=-timeout=90 --test_arg=-runs=100 --test_arg=/tmp/testcase-4667620284301312 \
	  //third_party/hunspell/fuzzers:dict_fuzzer
	```
	
	## Description
	The memory leak is caused by:
	- Some previous round allocated `defcpdtable[j]` successfully.
	- If later fails, then we free the entire `defcpdtable` and early return. But the original free memory code didn't correctly handle 2nd level of memory (`defcpdtable[j]`) allocated previously.
	
	This CL creates a new private method `free_defcpdtable` to handle all these cases.
	
	PRESUBMIT=passed
	BUG=280277178
	R=tjbarron
	CC=mghiware
	APPROVED=tjbarron
	REQUIRED_REVIEW=1
	DELTA=37 (25 added, 8 deleted, 4 changed)
	DELTA_BY_EXTENSION=cxx=27,hxx=2
	OCL=540728508
	FIG_CHANGESET=a345f0b0f502b10b471cfd2c7a008bb06724aad8
	FIG_WORKSPACE=sungyc/153:icing
	MARKDOWN=true

Affected files ...

... //depot//src/hunspell/affixmgr.cxx#13 edit
... //depot//src/hunspell/affixmgr.hxx#3 edit

==== //depot//src/hunspell/affixmgr.cxx#12 - /google/src/files/542666043/depot//src/hunspell/affixmgr.cxx ====
--- /google/src/files/537915601/depot//src/hunspell/affixmgr.cxx	2023-06-05 13:29:15.000000000 -0400
+++ /google/src/files/542666043/depot//src/hunspell/affixmgr.cxx	2023-06-22 17:19:55.000000000 -0400
@@ -197,14 +197,8 @@
      phone = NULL;
   }
 
-  if (defcpdtable) {  
-     for (int j=0; j < numdefcpd; j++) {
-        free(defcpdtable[j].def);
-        defcpdtable[j].def = NULL;
-     }
-     free(defcpdtable);  
-     defcpdtable = NULL;
-  }
+  free_defcpdtable();
+
   numrep = 0;
   if (checkcpdtable) {  
      for (int j=0; j < numcheckcpd; j++) {
@@ -3948,11 +3942,15 @@
              case 1: { 
                        numdefcpd = atoi(piece);
                        if (numdefcpd < 1) {
+                          free_defcpdtable();
                           HUNSPELL_WARNING(stderr, "error: line %d: bad entry number\n", af->getlinenum());
                           return 1;
                        }
                        defcpdtable = (flagentry *) malloc(numdefcpd * sizeof(flagentry));
-                       if (!defcpdtable) return 1;
+                       if (!defcpdtable) {
+                         free_defcpdtable();
+                         return 1;
+                       }
                        np++;
                        break;
                      }
@@ -3963,6 +3961,7 @@
        piece = mystrsep(&tp, 0);
    }
    if (np != 2) {
+      free_defcpdtable();
       HUNSPELL_WARNING(stderr, "error: line %d: missing data\n", af->getlinenum());
       return 1;
    } 
@@ -3970,7 +3969,10 @@
    /* now parse the numdefcpd lines to read in the remainder of the table */
    char * nl;
    for (int j=0; j < numdefcpd; j++) {
-        if (!(nl = af->getline())) return 1;
+        if (!(nl = af->getline())) {
+          free_defcpdtable();
+          return 1;
+        }
         mychomp(nl);
         tp = nl;
         i = 0;
@@ -3981,8 +3983,8 @@
                switch(i) {
                   case 0: {
                              if (strncmp(piece, "COMPOUNDRULE", 12) != 0) {
+                                 free_defcpdtable();
                                  HUNSPELL_WARNING(stderr, "error: line %d: table is corrupt\n", af->getlinenum());
-                                 numdefcpd = 0;
                                  return 1;
                              }
                              break;
@@ -4019,8 +4021,8 @@
            piece = mystrsep(&tp, 0);
         }
         if (!defcpdtable[j].len) {
+             free_defcpdtable();
              HUNSPELL_WARNING(stderr, "error: line %d: table is corrupt\n", af->getlinenum());
-             numdefcpd = 0;
              return 1;
         }
    }
@@ -4582,3 +4584,16 @@
   }
   return 0;
 }
+
+void AffixMgr::free_defcpdtable() {
+  if (defcpdtable) {
+     for (int j=0; j < numdefcpd; j++) {
+        free(defcpdtable[j].def);
+        defcpdtable[j].def = NULL;
+     }
+     free(defcpdtable);
+     defcpdtable = NULL;
+  }
+  numdefcpd = 0;
+}
+
==== //depot//src/hunspell/affixmgr.hxx#2 - /google/src/files/542666043/depot//src/hunspell/affixmgr.hxx ====
--- /google/src/files/49864191/depot//src/hunspell/affixmgr.hxx	2013-07-24 23:12:12.000000000 -0400
+++ /google/src/files/542666043/depot//src/hunspell/affixmgr.hxx	2023-06-22 17:19:55.000000000 -0400
@@ -244,6 +244,8 @@
   int process_sfx_tree_to_list();
   int redundant_condition(char, char * strip, int stripl,
       const char * cond, int);
+
+  void free_defcpdtable();
 };
 
 #endif