unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Liusirui <liusirui@huawei.com>
To: <libc-alpha@sourceware.org>
Cc: <carlos@redhat.com>, <siddhesh@gotplt.org>, <dj@redhat.com>,
	<hushiyuan@huawei.com>, <liqingqing3@huawei.com>
Subject: [PATCH] fix null pointer in mtrace
Date: Tue, 12 Nov 2019 17:22:19 +0800	[thread overview]
Message-ID: <1573550539-34259-1-git-send-email-liusirui@huawei.com> (raw)

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(-)

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);
 }
-- 
2.7.4


             reply	other threads:[~2019-11-12  9:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-12  9:22 Liusirui [this message]
2019-11-12  9:39 ` [PATCH] fix null pointer in mtrace liqingqing
2019-11-12 10:41   ` Florian Weimer
2019-11-13  1:28     ` liqingqing
  -- strict thread matches above, loose matches on Subject: below --
2019-11-13  9:36 Liusirui
2019-11-13 12:59 ` Carlos O'Donell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1573550539-34259-1-git-send-email-liusirui@huawei.com \
    --to=liusirui@huawei.com \
    --cc=carlos@redhat.com \
    --cc=dj@redhat.com \
    --cc=hushiyuan@huawei.com \
    --cc=libc-alpha@sourceware.org \
    --cc=liqingqing3@huawei.com \
    --cc=siddhesh@gotplt.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).