* [PATCH] fix null pointer in mtrace
@ 2019-11-12 9:22 Liusirui
2019-11-12 9:39 ` liqingqing
0 siblings, 1 reply; 6+ messages in thread
From: Liusirui @ 2019-11-12 9:22 UTC (permalink / raw)
To: libc-alpha; +Cc: carlos, siddhesh, dj, hushiyuan, liqingqing3
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fix null pointer in mtrace
2019-11-12 9:22 Liusirui
@ 2019-11-12 9:39 ` liqingqing
2019-11-12 10:41 ` Florian Weimer
0 siblings, 1 reply; 6+ messages in thread
From: liqingqing @ 2019-11-12 9:39 UTC (permalink / raw)
To: Liusirui, libc-alpha; +Cc: carlos, siddhesh, dj, hushiyuan
On 2019/11/12 17:22, 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.
>
> ---
> 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)
> +
I had tested this scenario, it seems like that the fprintf and other file operation function do not check the invalid argument like the null pointer.
does any one knows why fprintf do not check the input? thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix null pointer in mtrace
2019-11-12 9:39 ` liqingqing
@ 2019-11-12 10:41 ` Florian Weimer
2019-11-13 1:28 ` liqingqing
0 siblings, 1 reply; 6+ messages in thread
From: Florian Weimer @ 2019-11-12 10:41 UTC (permalink / raw)
To: liqingqing; +Cc: Liusirui, libc-alpha, carlos, siddhesh, dj, hushiyuan
* liqingqing:
> I had tested this scenario, it seems like that the fprintf and other
> file operation function do not check the invalid argument like the
> null pointer. does any one knows why fprintf do not check the input?
It's not required by the standard because violating preconditions
results in undefined behavior. Sloppily written code with such bugs
tends to not check error returns from functions, either. This means if
we added a null check for the stream argument to fprintf (e.g., failing
with EINVAL in that case), it would only obscure the problem and make
diagnosis even more difficult.
Thanks,
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix null pointer in mtrace
2019-11-12 10:41 ` Florian Weimer
@ 2019-11-13 1:28 ` liqingqing
0 siblings, 0 replies; 6+ messages in thread
From: liqingqing @ 2019-11-13 1:28 UTC (permalink / raw)
To: Florian Weimer
Cc: liusirui, libc-alpha@sourceware.org, carlos@redhat.com,
siddhesh@gotplt.org, dj@redhat.com, Hushiyuan
On 2019/11/12 18:41, Florian Weimer wrote:
> * liqingqing:
>
>> I had tested this scenario, it seems like that the fprintf and other
>> file operation function do not check the invalid argument like the
>> null pointer. does any one knows why fprintf do not check the input?
>
> It's not required by the standard because violating preconditions
> results in undefined behavior. Sloppily written code with such bugs
> tends to not check error returns from functions, either. This means if
> we added a null check for the stream argument to fprintf (e.g., failing
> with EINVAL in that case), it would only obscure the problem and make
> diagnosis even more difficult.
>
> Thanks,
> Florian
>
clear, thank you Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] fix null pointer in mtrace
@ 2019-11-13 9:36 Liusirui
2019-11-13 12:59 ` Carlos O'Donell
0 siblings, 1 reply; 6+ messages in thread
From: Liusirui @ 2019-11-13 9:36 UTC (permalink / raw)
To: libc-alpha; +Cc: carlos, siddhesh, dj, hushiyuan, liqingqing3
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fix null pointer in mtrace
2019-11-13 9:36 [PATCH] fix null pointer in mtrace Liusirui
@ 2019-11-13 12:59 ` Carlos O'Donell
0 siblings, 0 replies; 6+ messages in thread
From: Carlos O'Donell @ 2019-11-13 12:59 UTC (permalink / raw)
To: Liusirui, libc-alpha; +Cc: siddhesh, dj, hushiyuan, liqingqing3
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.
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-13 13:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-13 9:36 [PATCH] fix null pointer in mtrace Liusirui
2019-11-13 12:59 ` Carlos O'Donell
-- strict thread matches above, loose matches on Subject: below --
2019-11-12 9:22 Liusirui
2019-11-12 9:39 ` liqingqing
2019-11-12 10:41 ` Florian Weimer
2019-11-13 1:28 ` liqingqing
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).