diff mbox series

fix null pointer in mtrace

Message ID 1573637811-7639-1-git-send-email-liusirui@huawei.com
State New
Headers show
Series fix null pointer in mtrace | expand

Commit Message

liusirui Nov. 13, 2019, 9:36 a.m. UTC 
In a multi-threaded program, some threads request or free memory and try to write trace info
into file which "mallstream" points to. At the same time, another thread calls "muntrace" and
set "mallstream" to NULL. This may cause a segmentation fault.

The comment in malloc/mtrace.c says "We could be printing a NULL here; that's OK.". Although
the functions mtrace/muntrace are used for debugging, program isn't expected to crash while using
these functions.

---
 malloc/mtrace.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

Comments

Carlos O'Donell Nov. 13, 2019, 12:59 p.m. UTC | #1 
On 11/13/19 4:36 AM, Liusirui wrote:
> In a multi-threaded program, some threads request or free memory and try to write trace info
> into file which "mallstream" points to. At the same time, another thread calls "muntrace" and
> set "mallstream" to NULL. This may cause a segmentation fault.
> 
> The comment in malloc/mtrace.c says "We could be printing a NULL here; that's OK.". Although
> the functions mtrace/muntrace are used for debugging, program isn't expected to crash while using
> these functions.
Just a reminder that we're still waiting on the completion of a corporate assignment from Huawei.

We can't accept these patches until that is complete.
diff mbox series

Patch

diff --git a/malloc/mtrace.c b/malloc/mtrace.c
index 707f998..33f01b4 100644
--- a/malloc/mtrace.c
+++ b/malloc/mtrace.c
@@ -44,6 +44,10 @@ 
 
 #define TRACE_BUFFER_SIZE 512
 
+#define mtrace_print(file, format, ...) do { \
+if (file != NULL) mtrace_print(file, format,##__VA_ARGS__); \
+} while(0)
+
 static FILE *mallstream;
 static const char mallenv[] = "MALLOC_TRACE";
 static char *malloc_trace_buffer;
@@ -99,12 +103,12 @@  tr_where (const void *caller, Dl_info *info)
                         ")");
             }
 
-          fprintf (mallstream, "@ %s%s%s[%p] ",
+          mtrace_print (mallstream, "@ %s%s%s[%p] ",
                    info->dli_fname ? : "", info->dli_fname ? ":" : "",
                    buf, caller);
         }
       else
-        fprintf (mallstream, "@ [%p] ", caller);
+        mtrace_print (mallstream, "@ [%p] ", caller);
     }
 }
 
@@ -166,7 +170,7 @@  tr_freehook (void *ptr, const void *caller)
   Dl_info *info = lock_and_info (caller, &mem);
   tr_where (caller, info);
   /* Be sure to print it first.  */
-  fprintf (mallstream, "- %p\n", ptr);
+  mtrace_print (mallstream, "- %p\n", ptr);
   if (ptr == mallwatch)
     {
       __libc_lock_unlock (lock);
@@ -198,8 +202,7 @@  tr_mallochook (size_t size, const void *caller)
   set_trace_hooks ();
 
   tr_where (caller, info);
-  /* We could be printing a NULL here; that's OK.  */
-  fprintf (mallstream, "+ %p %#lx\n", hdr, (unsigned long int) size);
+  mtrace_print (mallstream, "+ %p %#lx\n", hdr, (unsigned long int) size);
 
   __libc_lock_unlock (lock);
 
@@ -232,17 +235,17 @@  tr_reallochook (void *ptr, size_t size, const void *caller)
     {
       if (size != 0)
         /* Failed realloc.  */
-        fprintf (mallstream, "! %p %#lx\n", ptr, (unsigned long int) size);
+        mtrace_print (mallstream, "! %p %#lx\n", ptr, (unsigned long int) size);
       else
-        fprintf (mallstream, "- %p\n", ptr);
+        mtrace_print (mallstream, "- %p\n", ptr);
     }
   else if (ptr == NULL)
-    fprintf (mallstream, "+ %p %#lx\n", hdr, (unsigned long int) size);
+    mtrace_print (mallstream, "+ %p %#lx\n", hdr, (unsigned long int) size);
   else
     {
-      fprintf (mallstream, "< %p\n", ptr);
+      mtrace_print (mallstream, "< %p\n", ptr);
       tr_where (caller, info);
-      fprintf (mallstream, "> %p %#lx\n", hdr, (unsigned long int) size);
+      mtrace_print (mallstream, "> %p %#lx\n", hdr, (unsigned long int) size);
     }
 
   __libc_lock_unlock (lock);
@@ -270,7 +273,7 @@  tr_memalignhook (size_t alignment, size_t size, const void *caller)
 
   tr_where (caller, info);
   /* We could be printing a NULL here; that's OK.  */
-  fprintf (mallstream, "+ %p %#lx\n", hdr, (unsigned long int) size);
+  mtrace_print (mallstream, "+ %p %#lx\n", hdr, (unsigned long int) size);
 
   __libc_lock_unlock (lock);
 
@@ -333,7 +336,7 @@  mtrace (void)
           /* Be sure it doesn't malloc its buffer!  */
           malloc_trace_buffer = mtb;
           setvbuf (mallstream, malloc_trace_buffer, _IOFBF, TRACE_BUFFER_SIZE);
-          fprintf (mallstream, "= Start\n");
+          mtrace_print (mallstream, "= Start\n");
 	  save_default_hooks ();
 	  set_trace_hooks ();
 #ifdef _LIBC
@@ -363,6 +366,6 @@  muntrace (void)
   mallstream = NULL;
   set_default_hooks ();
 
-  fprintf (f, "= End\n");
+  mtrace_print (f, "= End\n");
   fclose (f);
 }