[PATCH] Fix memory leak in sysinfo

[PATCH] Fix memory leak in sysinfo

[Simon Marchi]

I am trying to build the binutils-gdb repo with address sanitizer, but
the build fails because sysinfo (executed during the build) leaks, which
fails its execution and interrupts the Makefile.

Direct leak of 7122 byte(s) in 755 object(s) allocated from:
    #0 0x7f050664e602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0x403bca in yylex /home/emaisin/src/binutils-gdb/binutils/syslex.l:51
    #2 0x4016f9 in yyparse /home/emaisin/build/binutils-gdb/binutils/sysinfo.c:1179
    #3 0x4034b2 in main /home/emaisin/src/binutils-gdb/binutils/sysinfo.y:420
    #4 0x7f050620d82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: 7122 byte(s) leaked in 755 allocation(s).

One solution would have been to find a way to not pass
-fsanitize=address when building sysinfo, but it's not clear how to omit
the flag for this program only.  It turns out that it was easier to fix
the leak.

binutils/ChangeLog:

	* sysinfo.y: Free "it" variable.
---
 binutils/sysinfo.y | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/binutils/sysinfo.y b/binutils/sysinfo.y
index 62758de..6c4a23d 100644
--- a/binutils/sysinfo.y
+++ b/binutils/sysinfo.y
@@ -159,6 +159,8 @@ it:
   case 'c':
     printf("}\n");
   }
+
+  free (it);
 }
 ;
 
-- 
2.7.4

Re: [PATCH] Fix memory leak in sysinfo

[Simon Marchi]

On 2017-06-01 10:51 PM, Simon Marchi wrote:
> I am trying to build the binutils-gdb repo with address sanitizer, but
> the build fails because sysinfo (executed during the build) leaks, which
> fails its execution and interrupts the Makefile.
> 
> Direct leak of 7122 byte(s) in 755 object(s) allocated from:
>     #0 0x7f050664e602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
>     #1 0x403bca in yylex /home/emaisin/src/binutils-gdb/binutils/syslex.l:51
>     #2 0x4016f9 in yyparse /home/emaisin/build/binutils-gdb/binutils/sysinfo.c:1179
>     #3 0x4034b2 in main /home/emaisin/src/binutils-gdb/binutils/sysinfo.y:420
>     #4 0x7f050620d82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
> 
> SUMMARY: AddressSanitizer: 7122 byte(s) leaked in 755 allocation(s).
> 
> One solution would have been to find a way to not pass
> -fsanitize=address when building sysinfo, but it's not clear how to omit
> the flag for this program only.  It turns out that it was easier to fix
> the leak.
> 
> binutils/ChangeLog:
> 
> 	* sysinfo.y: Free "it" variable.
> ---
>  binutils/sysinfo.y | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/binutils/sysinfo.y b/binutils/sysinfo.y
> index 62758de..6c4a23d 100644
> --- a/binutils/sysinfo.y
> +++ b/binutils/sysinfo.y
> @@ -159,6 +159,8 @@ it:
>    case 'c':
>      printf("}\n");
>    }
> +
> +  free (it);
>  }
>  ;
>  
> 

Ah, nevermind, I was actually not rebuilding sysinfo properly: make'ing directly in binutils/
doesn't honor the CFLAGS_FOR_BUILD Makefile variable set in the top-level Makefile.  I was
therefore rebuilding without ASan, leading me to think the leak was fixed.  But it's not,
NAME is being used at a few other places, and it's not always obvious where it should be freed.

Simon

[PATCH v2] Fix memory leak in sysinfo

[Simon Marchi]

New in v2: try to fix all the leaks.  I made sure that sysinfo was actually
built with Asan while testing.

I am trying to build the binutils-gdb repo with address sanitizer, but
the build fails because sysinfo (executed during the build) leaks, which
fails its execution and interrupts the Makefile.

Direct leak of 7122 byte(s) in 755 object(s) allocated from:
    #0 0x7f050664e602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0x403bca in yylex /home/emaisin/src/binutils-gdb/binutils/syslex.l:51
    #2 0x4016f9 in yyparse /home/emaisin/build/binutils-gdb/binutils/sysinfo.c:1179
    #3 0x4034b2 in main /home/emaisin/src/binutils-gdb/binutils/sysinfo.y:420
    #4 0x7f050620d82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: 7122 byte(s) leaked in 755 allocation(s).

One solution would have been to find a way to not pass
-fsanitize=address when building sysinfo, but it's not clear how to omit
the flag for this program only.

To fix the leaks, we need to free the memory allocated by the token NAME
whenever we are done with it.

binutils/ChangeLog:

	* sysinfo.y: Free memory allocated by token NAME.
---
 binutils/sysinfo.y | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/binutils/sysinfo.y b/binutils/sysinfo.y
index 62758de..1c8f1ff 100644
--- a/binutils/sysinfo.y
+++ b/binutils/sysinfo.y
@@ -21,6 +21,7 @@
 %{
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 
 static char writecode;
 static char *it;
@@ -159,6 +160,8 @@ it:
   case 'c':
     printf("}\n");
   }
+
+  free (it);
 }
 ;
 
@@ -204,6 +207,8 @@ repeat_it_field: '(' REPEAT NAME
 	 it_field_list ')'
 
 	{
+	  free (repeat);
+
 	  repeat = oldrepeat;
 	  oldrepeat =0;
 	  rdepth--;
@@ -230,6 +235,8 @@ cond_it_field: '(' COND NAME
 	      printf("\tif (%s) {\n", $3);
 	      break;
 	    }
+
+	  free ($3);
 	}
 
 	 it_field_list ')'
@@ -348,6 +355,9 @@ char *ptr = pnames[rdepth];
 	      else abort();
 		  break;
 		}
+
+	  free (desc);
+	  free (id);
 	}
 
 	;
@@ -371,7 +381,7 @@ attr_size:
 
 attr_id:
 		'(' NAME ')'	{ $$ = $2; }
-	|	{ $$ = "dummy";}
+	|	{ $$ = strdup ("dummy");}
 	;
 
 enums:
@@ -388,6 +398,9 @@ enum_list:
 	    case 'c':
 		printf("if (ptr->%s%s == %s) { tabout(); printf(\"%s\\n\");}\n", name, names[rdepth],$4,$3);
 	    }
+
+	  free ($3);
+	  free ($4);
 	}
 
 	;
-- 
2.7.4

Re: [PATCH v2] Fix memory leak in sysinfo

[Nick Clifton]

Hi Simon,

> binutils/ChangeLog:
> 
> 	* sysinfo.y: Free memory allocated by token NAME.

Approved and applied - thanks!

Cheers
  Nick